GNU bug report logs -
#60983
29.0.60; Tree-sitter user-level control
Previous Next
To reply to this bug, email your comments to 60983 AT debbugs.gnu.org.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 21 Jan 2023 11:12:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Eli Zaretskii <eliz <at> gnu.org>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Sat, 21 Jan 2023 11:12:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
I started looking into providing user-level documentation for
tree-sitter based modes, and bumped into some issues:
. How does one use treesit-font-lock-level?
- It is not a customizable user option (unlike
font-lock-maximum-decoration), so it cannot be set via
customize-variable. Is there a reason not to make it a
defcustom?
- It automatically becomes buffer-local when set, and OTOH setting
it in a buffer does not produce fontifications according to the
level, and neither does setting it in a mode hook. So the only
way to change its value is by using setq-default, which I don't
think is the intent?
- Should we make the variable a defcustom?
- Should it be possible to customize it separately for each mode?
- Should we allow to change the level and then call some function
to re-fontify the current buffer according to the new level?
. How does one change the indentation style in c-ts-mode?
- There is a defcustom c-ts-mode-indent-style, but I don't think I
see any difference in indentation of new code when I change the
value. What am I missing?
. What commands are affected by treesit-defun-tactic?
In GNU Emacs 29.0.60 (build 238, i686-pc-mingw32) of 2023-01-20 built on
HOME-C4E4A596F7
Repository revision: 472f142598566fbaeedcacaf9a9c757a1281c0c5
Repository branch: emacs-29
Windowing system distributor 'Microsoft Corp.', version 5.1.2600
System Description: Microsoft Windows XP Service Pack 3 (v5.1.0.2600)
Configured using:
'configure -C --prefix=/d/usr --with-wide-int
--enable-checking=yes,glyphs 'CFLAGS=-O0 -gdwarf-4 -g3''
Configured features:
ACL GIF GMP GNUTLS HARFBUZZ JPEG JSON LCMS2 LIBXML2 MODULES NOTIFY
W32NOTIFY PDUMPER PNG RSVG SOUND SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XPM ZLIB
Important settings:
value of $LANG: ENU
locale-coding-system: cp1255
Major mode: C
Minor modes in effect:
tooltip-mode: t
global-eldoc-mode: t
show-paren-mode: t
electric-indent-mode: t
mouse-wheel-mode: t
tool-bar-mode: t
menu-bar-mode: t
file-name-shadow-mode: t
global-font-lock-mode: t
font-lock-mode: t
blink-cursor-mode: t
line-number-mode: t
indent-tabs-mode: t
transient-mark-mode: t
auto-composition-mode: t
auto-encryption-mode: t
auto-compression-mode: t
Load-path shadows:
None found.
Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util time-date subr-x mm-decode mm-bodies mm-encode
mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mail-utils shortdoc
text-property-search misearch multi-isearch cl-extra cl-print thingatpt
help-fns radix-tree pp wid-edit descr-text help-mode c-ts-mode treesit
cl-seq vc-git diff-mode easy-mmode vc vc-dispatcher bug-reference
byte-opt gv bytecomp byte-compile cc-mode cc-fonts cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs cl-loaddefs cl-lib
rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel dos-w32 ls-lisp disp-table
term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads w32notify w32 lcms2 multi-tty
make-network-process emacs)
Memory information:
((conses 16 107446 10610)
(symbols 48 10355 0)
(strings 16 33788 3445)
(string-bytes 1 1014069)
(vectors 16 17439)
(vector-slots 8 223440 13934)
(floats 8 91 195)
(intervals 40 3548 140)
(buffers 888 14))
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 21 Jan 2023 11:50:02 GMT)
Full text and
rfc822 format available.
Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
> I started looking into providing user-level documentation for
> tree-sitter based modes, and bumped into some issues:
>
> . How does one use treesit-font-lock-level?
>
> - It is not a customizable user option (unlike
> font-lock-maximum-decoration), so it cannot be set via
> customize-variable. Is there a reason not to make it a
> defcustom?
> - It automatically becomes buffer-local when set, and OTOH setting
> it in a buffer does not produce fontifications according to the
> level, and neither does setting it in a mode hook. So the only
> way to change its value is by using setq-default, which I don't
> think is the intent?
> - Should we make the variable a defcustom?
> - Should it be possible to customize it separately for each mode?
> - Should we allow to change the level and then call some function
> to re-fontify the current buffer according to the new level?
I struggled with this too. I ended up setting it with setq-default,
assuming I was just missing something very simple. I'm in favor for
either a defcustom or honoring the font-lock-maximum-decoration values,
specifically these settings:
```
If t, use the maximum decoration available.
If a number, use that level of decoration (or if not available the maximum).
```
>
> . How does one change the indentation style in c-ts-mode?
>
> - There is a defcustom c-ts-mode-indent-style, but I don't think I
> see any difference in indentation of new code when I change the
> value. What am I missing?
>
(setq c-ts-mode-indent-style 'bsd) then revert-buffer fixes it for me.
It seems you need to reload the file to enable the new style. Should I
add a command that can be set explicitly as in c-mode?
'c-ts-mode-set-style'?
However, going over it I see there are lots of regressions after the new
bracket-counting code added recently, effectively making the indent
styles pretty broken right now...
For example with bsd style:
Previously:
int
main()
{
if (x)
{
}
else
{
}
}
now:
int
main()
{
if (x)
{
}
else
{
}
}
> . What commands are affected by treesit-defun-tactic?
'treesit--navigate-thing' uses it, so 'beginning-of-defun',
'forward-sentence' etc uses it through 'treesit-beginning-of-thing'.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 21 Jan 2023 12:37:02 GMT)
Full text and
rfc822 format available.
Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: Yuan Fu <casouri <at> gmail.com>
> Date: Sat, 21 Jan 2023 12:48:58 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > . How does one change the indentation style in c-ts-mode?
> >
> > - There is a defcustom c-ts-mode-indent-style, but I don't think I
> > see any difference in indentation of new code when I change the
> > value. What am I missing?
> >
>
> (setq c-ts-mode-indent-style 'bsd) then revert-buffer fixes it for me.
> It seems you need to reload the file to enable the new style. Should I
> add a command that can be set explicitly as in c-mode?
> 'c-ts-mode-set-style'?
I think we need both a command and a :set function for the defcustom.
> However, going over it I see there are lots of regressions after the new
> bracket-counting code added recently, effectively making the indent
> styles pretty broken right now...
That's a separate issue, and I'm sure it will be fixed. And adding
tests to the test suite will prevent us from breaking it too easily in
the future.
> > . What commands are affected by treesit-defun-tactic?
>
> 'treesit--navigate-thing' uses it, so 'beginning-of-defun',
> 'forward-sentence' etc uses it through 'treesit-beginning-of-thing'.
What commands except those that call treesit--navigate-thing call
beginning-of-defun and treesit-beginning-of-thing?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 21 Jan 2023 12:41:02 GMT)
Full text and
rfc822 format available.
Message #14 received at submit <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: Yuan Fu <casouri <at> gmail.com>
>> Date: Sat, 21 Jan 2023 12:48:58 +0100
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> > . How does one change the indentation style in c-ts-mode?
>> >
>> > - There is a defcustom c-ts-mode-indent-style, but I don't think I
>> > see any difference in indentation of new code when I change the
>> > value. What am I missing?
>> >
>>
>> (setq c-ts-mode-indent-style 'bsd) then revert-buffer fixes it for me.
>> It seems you need to reload the file to enable the new style. Should I
>> add a command that can be set explicitly as in c-mode?
>> 'c-ts-mode-set-style'?
>
> I think we need both a command and a :set function for the defcustom.
>
Ok, I'll make a bugreport for this.
>> However, going over it I see there are lots of regressions after the new
>> bracket-counting code added recently, effectively making the indent
>> styles pretty broken right now...
>
> That's a separate issue, and I'm sure it will be fixed. And adding
> tests to the test suite will prevent us from breaking it too easily in
> the future.
>
And this.
>> > . What commands are affected by treesit-defun-tactic?
>>
>> 'treesit--navigate-thing' uses it, so 'beginning-of-defun',
>> 'forward-sentence' etc uses it through 'treesit-beginning-of-thing'.
>
> What commands except those that call treesit--navigate-thing call
> beginning-of-defun and treesit-beginning-of-thing?
I'll defer to Yuan for this :)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 23 Jan 2023 16:53:01 GMT)
Full text and
rfc822 format available.
Message #17 received at submit <at> debbugs.gnu.org (full text, mbox):
Ping! Yuan, any comments? I'd like to finish this job some time
soon, and I need your feedback and/or code changes before I can
proceed.
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: Yuan Fu <casouri <at> gmail.com>
> Date: Sat, 21 Jan 2023 12:48:58 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > I started looking into providing user-level documentation for
> > tree-sitter based modes, and bumped into some issues:
> >
> > . How does one use treesit-font-lock-level?
> >
> > - It is not a customizable user option (unlike
> > font-lock-maximum-decoration), so it cannot be set via
> > customize-variable. Is there a reason not to make it a
> > defcustom?
> > - It automatically becomes buffer-local when set, and OTOH setting
> > it in a buffer does not produce fontifications according to the
> > level, and neither does setting it in a mode hook. So the only
> > way to change its value is by using setq-default, which I don't
> > think is the intent?
> > - Should we make the variable a defcustom?
> > - Should it be possible to customize it separately for each mode?
> > - Should we allow to change the level and then call some function
> > to re-fontify the current buffer according to the new level?
>
> I struggled with this too. I ended up setting it with setq-default,
> assuming I was just missing something very simple. I'm in favor for
> either a defcustom or honoring the font-lock-maximum-decoration values,
> specifically these settings:
>
> ```
> If t, use the maximum decoration available.
> If a number, use that level of decoration (or if not available the maximum).
> ```
>
> >
> > . How does one change the indentation style in c-ts-mode?
> >
> > - There is a defcustom c-ts-mode-indent-style, but I don't think I
> > see any difference in indentation of new code when I change the
> > value. What am I missing?
> >
>
> (setq c-ts-mode-indent-style 'bsd) then revert-buffer fixes it for me.
> It seems you need to reload the file to enable the new style. Should I
> add a command that can be set explicitly as in c-mode?
> 'c-ts-mode-set-style'?
>
> However, going over it I see there are lots of regressions after the new
> bracket-counting code added recently, effectively making the indent
> styles pretty broken right now...
>
> For example with bsd style:
>
> Previously:
>
> int
> main()
> {
> if (x)
> {
>
> }
> else
> {
>
> }
> }
>
> now:
>
> int
> main()
> {
> if (x)
> {
>
> }
> else
> {
>
> }
> }
>
>
>
> > . What commands are affected by treesit-defun-tactic?
>
> 'treesit--navigate-thing' uses it, so 'beginning-of-defun',
> 'forward-sentence' etc uses it through 'treesit-beginning-of-thing'.
>
> Theo
>
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 23 Jan 2023 19:38:02 GMT)
Full text and
rfc822 format available.
Message #20 received at submit <at> debbugs.gnu.org (full text, mbox):
Sorry for the delay, overall I agree with your changes.
>
> . How does one use treesit-font-lock-level?
>
> - It is not a customizable user option (unlike
> font-lock-maximum-decoration), so it cannot be set via
> customize-variable. Is there a reason not to make it a
> defcustom?
> - It automatically becomes buffer-local when set, and OTOH setting
> it in a buffer does not produce fontifications according to the
> level, and neither does setting it in a mode hook. So the only
> way to change its value is by using setq-default, which I don't
> think is the intent?
> - Should we make the variable a defcustom?
Yeah it should be a defcustom.
> - Should it be possible to customize it separately for each mode?
> - Should we allow to change the level and then call some function
> to re-fontify the current buffer according to the new level?
You can set this variable and call treesit-font-lock-recompute-features with no argument, which enables/disables features according to the current level.
> Ok, I'll make a bugreport for this.
>
>>> However, going over it I see there are lots of regressions after the new
>>> bracket-counting code added recently, effectively making the indent
>>> styles pretty broken right now...
Sorry about that :-(
>
>
>>>> . What commands are affected by treesit-defun-tactic?
>>>
>>> 'treesit--navigate-thing' uses it, so 'beginning-of-defun',
>>> 'forward-sentence' etc uses it through 'treesit-beginning-of-thing'.
>>
>> What commands except those that call treesit--navigate-thing call
>> beginning-of-defun and treesit-beginning-of-thing?
>
> I'll defer to Yuan for this :)
treesit-beginning/end-of-defun are called by beginning/end-of-defun, so other functions like mark-defund are also affected by treesit-defun-tactic. Treesit-beginning-of-thing is on master, not on emacs-29.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 23 Jan 2023 20:00:02 GMT)
Full text and
rfc822 format available.
Message #23 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 23 Jan 2023 11:37:24 -0800
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>
> Sorry for the delay, overall I agree with your changes.
Thanks. What about the questions I asked regarding indentation
features, and specifically about c-ts-mode-indent-style?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 23 Jan 2023 21:10:02 GMT)
Full text and
rfc822 format available.
Message #26 received at submit <at> debbugs.gnu.org (full text, mbox):
On 23 January 2023 20:59:14 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 23 Jan 2023 11:37:24 -0800
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>>
>> Sorry for the delay, overall I agree with your changes.
>
>Thanks. What about the questions I asked regarding indentation
>features, and specifically about c-ts-mode-indent-style?
I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 23 Jan 2023 23:56:02 GMT)
Full text and
rfc822 format available.
Message #29 received at submit <at> debbugs.gnu.org (full text, mbox):
> On Jan 23, 2023, at 1:08 PM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>
>
>
> On 23 January 2023 20:59:14 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>>> From: Yuan Fu <casouri <at> gmail.com>
>>> Date: Mon, 23 Jan 2023 11:37:24 -0800
>>> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>>> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>>>
>>> Sorry for the delay, overall I agree with your changes.
>>
>> Thanks. What about the questions I asked regarding indentation
>> features, and specifically about c-ts-mode-indent-style?
>
> I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
>
> I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
One common approach is to iterate over all live buffer and reset the variable (in this case treesit-simple-indent-rules) on applicate buffers (in this case c/c++-ts-mode buffers).
It would be nice to also have a command c-ts-mode-set-style (like c-set-style) that takes a style symbol and sets treesit-simple-indent-rules accordingly. And in major-mode setup, ie, c-ts-mode’s body, you call it with c-ts-mode-indent-style.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Tue, 24 Jan 2023 03:27:01 GMT)
Full text and
rfc822 format available.
Message #32 received at submit <at> debbugs.gnu.org (full text, mbox):
> Date: Mon, 23 Jan 2023 22:08:27 +0100
> From: Theodor Thornhill <theo <at> thornhill.no>
> CC: bug-gnu-emacs <at> gnu.org
>
>
>
> On 23 January 2023 20:59:14 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
> >> From: Yuan Fu <casouri <at> gmail.com>
> >> Date: Mon, 23 Jan 2023 11:37:24 -0800
> >> Cc: Eli Zaretskii <eliz <at> gnu.org>,
> >> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
> >>
> >> Sorry for the delay, overall I agree with your changes.
> >
> >Thanks. What about the questions I asked regarding indentation
> >features, and specifically about c-ts-mode-indent-style?
>
> I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
>
> I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
Thanks, but can you add some details of what you are trying to do and
what are the difficulties?
Adding Stefan in case he has some advice.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Wed, 25 Jan 2023 20:14:01 GMT)
Full text and
rfc822 format available.
Message #35 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Date: Mon, 23 Jan 2023 22:08:27 +0100
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> CC: bug-gnu-emacs <at> gnu.org
>>
>>
>>
>> On 23 January 2023 20:59:14 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> >> From: Yuan Fu <casouri <at> gmail.com>
>> >> Date: Mon, 23 Jan 2023 11:37:24 -0800
>> >> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>> >> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>> >>
>> >> Sorry for the delay, overall I agree with your changes.
>> >
>> >Thanks. What about the questions I asked regarding indentation
>> >features, and specifically about c-ts-mode-indent-style?
>>
>> I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
>>
>> I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
>
> Thanks, but can you add some details of what you are trying to do and
> what are the difficulties?
>
> Adding Stefan in case he has some advice.
Ok, I added a patch below.
try:
1. open some c buffer and make some edits, for example
```
void
main()
{
if (x)
{
}
}
```
This should be the expected output when using the gnu style.
2. M-x c-ts-mode-set-style "bsd" RET
3. C-h o c-ts-mode-indent-style RET
Observe variable has changed, but indenting the code does not.
4 C-h o treesit-simple-indent-rules RET
Observe variable keeps old value
5. C-x x g
Now the bsd style takes effect, and the treesit-simple-indent-rules
variable has changed.
I'm sure the fix is easy, but I don't see it. I purposely kept the
functions simple until I know what approach is best:)
Thanks,
Theo
[0001-Initial-c-ts-mode-set-style-attempt.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Wed, 25 Jan 2023 21:17:02 GMT)
Full text and
rfc822 format available.
Message #38 received at submit <at> debbugs.gnu.org (full text, mbox):
> -(defcustom c-ts-mode-indent-style 'gnu
> +(defcustom c-ts-mode-indent-style "gnu"
> "Style used for indentation.
>
> The selected style could be one of GNU, K&R, LINUX or BSD. If
> @@ -100,13 +100,33 @@ c-ts-mode-indent-style
> set instead. This function is expected return a list that
> follows the form of `treesit-simple-indent-rules'."
> :version "29.1"
> - :type '(choice (symbol :tag "Gnu" 'gnu)
> - (symbol :tag "K&R" 'k&r)
> - (symbol :tag "Linux" 'linux)
> - (symbol :tag "BSD" 'bsd)
> + :type '(choice (string :tag "Gnu" "gnu")
> + (string :tag "K&R" "k&r")
> + (string :tag "Linux" "linux")
> + (string :tag "BSD" "bsd")
> (function :tag "A function for user customized style" ignore))
> + :set #'c-ts-mode--indent-style
> :group 'c)
Why change to strings?
BTW the previous code seems wrong: instead of
(symbol :tag "FOO" 'foo)
it should be
(symbol :tag "FOO" foo)
since `'foo` is not a symbol but a list (of two symbols).
> +(defun c-ts-mode--indent-style (sym val)
> + "Custom setter for `c-ts-mode-indent-style'."
> + (set-default sym val))
Hmm... why bother use a `:set`ter if it doesn't do anything more than
the default does?
Shouldn't it call `treesit--indent-rules-optimize` to (re)set
`treesit-simple-indent-rules`?
[ Presumably in all relevant buffers, since the defcustom setting is
global. ]
> +(defun c-ts-mode-set-style ()
> + (interactive)
> + (or (eq major-mode 'c-ts-mode) (eq major-mode 'c++-ts-mode)
> + (error "Buffer %s is not a c-ts-mode (c-ts-mode-set-style)"
> + (buffer-name)))
> + (if-let ((mode (cond ((eq major-mode 'c-ts-mode) 'c)
> + ((eq major-mode 'c++-ts-mode) 'cpp)
> + (t nil)))
> + (choice (completing-read "Select style: " '("gnu" "k&r" "linux" "bsd"))))
Here, we probably want to specify `must-match` to `completing-read`
(which makes it unnecessary to check `if-let`, I think) and we should
provide a default.
Also we should probably use the (c-ts-mode--indent-styles mode) alist
rather than hardcode the set of styles.
> + (c-ts-mode--indent-style 'c-ts-mode-indent-style choice)
> + (kill-local-variable 'treesit-simple-indent-rules)
> + (setq-local treesit-simple-indent-rules
> + (treesit--indent-rules-optimize
> + (c-ts-mode--set-indent-style mode)))))
Here we presumably want to do the (setq-local
treesit-simple-indent-rules ...) every time (and set
`c-ts-mode-indent-style` buffer locally rather than via
`c-ts-mode--indent-style`, or otherwise provide an additional arg to
`c-ts-mode--indent-style` to say whether it applies globally or only to
the current buffer).
BTW, the naming of `c-ts-mode--indent-style` and
`c-ts-mode--set-indent-style` is confusing.
Also: why `kill-local-variable` just before the `setq-local`?
> - (pcase c-ts-mode-indent-style
> - ('gnu (alist-get 'gnu (c-ts-mode--indent-styles mode)))
> - ('k&r (alist-get 'k&r (c-ts-mode--indent-styles mode)))
> - ('bsd (alist-get 'bsd (c-ts-mode--indent-styles mode)))
> - ('linux (alist-get 'linux (c-ts-mode--indent-styles mode)))))))
> + (alist-get c-ts-mode-indent-style
> + (c-ts-mode--indent-styles mode) nil nil #'equal))))
Thanks :-)
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 06:09:02 GMT)
Full text and
rfc822 format available.
Message #41 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: casouri <at> gmail.com, bug-gnu-emacs <at> gnu.org
> Date: Wed, 25 Jan 2023 21:12:53 +0100
>
> > Adding Stefan in case he has some advice.
>
> Ok, I added a patch below.
>
>
> try:
>
> 1. open some c buffer and make some edits, for example
> ```
> void
> main()
> {
> if (x)
> {
> }
> }
> ```
>
> This should be the expected output when using the gnu style.
>
> 2. M-x c-ts-mode-set-style "bsd" RET
>
> 3. C-h o c-ts-mode-indent-style RET
> Observe variable has changed, but indenting the code does not.
>
> 4 C-h o treesit-simple-indent-rules RET
> Observe variable keeps old value
>
> 5. C-x x g
>
> Now the bsd style takes effect, and the treesit-simple-indent-rules
> variable has changed.
>
> I'm sure the fix is easy, but I don't see it. I purposely kept the
> functions simple until I know what approach is best:)
Thanks, I'll wait until you adapt the changes to Stefan's comments,
and try the new code then. If that doesn't fix the above problem, ask
the question again at that time.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 06:26:02 GMT)
Full text and
rfc822 format available.
Message #44 received at submit <at> debbugs.gnu.org (full text, mbox):
On 26 January 2023 07:08:33 CET, Eli Zaretskii <eliz <at> gnu.org> wrote:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: casouri <at> gmail.com, bug-gnu-emacs <at> gnu.org
>> Date: Wed, 25 Jan 2023 21:12:53 +0100
>>
>> > Adding Stefan in case he has some advice.
>>
>> Ok, I added a patch below.
>>
>>
>> try:
>>
>> 1. open some c buffer and make some edits, for example
>> ```
>> void
>> main()
>> {
>> if (x)
>> {
>> }
>> }
>> ```
>>
>> This should be the expected output when using the gnu style.
>>
>> 2. M-x c-ts-mode-set-style "bsd" RET
>>
>> 3. C-h o c-ts-mode-indent-style RET
>> Observe variable has changed, but indenting the code does not.
>>
>> 4 C-h o treesit-simple-indent-rules RET
>> Observe variable keeps old value
>>
>> 5. C-x x g
>>
>> Now the bsd style takes effect, and the treesit-simple-indent-rules
>> variable has changed.
>>
>> I'm sure the fix is easy, but I don't see it. I purposely kept the
>> functions simple until I know what approach is best:)
>
>Thanks, I'll wait until you adapt the changes to Stefan's comments,
>and try the new code then. If that doesn't fix the above problem, ask
>the question again at that time.
Yeah, thanks both :)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 07:28:02 GMT)
Full text and
rfc822 format available.
Message #47 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> Cc: 60983 <at> debbugs.gnu.org, theo <at> thornhill.no
> Date: Mon, 23 Jan 2023 18:52:33 +0200
> From: Eli Zaretskii <eliz <at> gnu.org>
>
> > From: Theodor Thornhill <theo <at> thornhill.no>
> > Cc: Yuan Fu <casouri <at> gmail.com>
> > Date: Sat, 21 Jan 2023 12:48:58 +0100
> >
> > Eli Zaretskii <eliz <at> gnu.org> writes:
> >
> > > I started looking into providing user-level documentation for
> > > tree-sitter based modes, and bumped into some issues:
> > >
> > > . How does one use treesit-font-lock-level?
> > >
> > > - It is not a customizable user option (unlike
> > > font-lock-maximum-decoration), so it cannot be set via
> > > customize-variable. Is there a reason not to make it a
> > > defcustom?
> > > - It automatically becomes buffer-local when set, and OTOH setting
> > > it in a buffer does not produce fontifications according to the
> > > level, and neither does setting it in a mode hook. So the only
> > > way to change its value is by using setq-default, which I don't
> > > think is the intent?
> > > - Should we make the variable a defcustom?
> > > - Should it be possible to customize it separately for each mode?
> > > - Should we allow to change the level and then call some function
> > > to re-fontify the current buffer according to the new level?
> >
> > I struggled with this too. I ended up setting it with setq-default,
> > assuming I was just missing something very simple. I'm in favor for
> > either a defcustom or honoring the font-lock-maximum-decoration values,
> > specifically these settings:
> >
> > ```
> > If t, use the maximum decoration available.
> > If a number, use that level of decoration (or if not available the maximum).
> > ```
Let's just make it a defcustom for now, with the values it has today,
including the default.
The problems with honoring the value of font-lock-maximum-decoration
are that (a) its default value is t in most (all?) modes, whereas we
decided not to use 4 as the default value of treesit-font-lock-level;
and (b) if changing treesit-font-lock-level's value doesn't require to
kill the buffer and revisit the file (as I hope we will make it work),
the instructions regarding changing the value of
font-lock-maximum-decoration will depend on whether the mode does or
doesn't use tree-sitter, which will make the instructions confusingly
complex.
Yuan or Theo, would one of you please make the change of making
treesit-font-lock-level a defcustom, with a proper :set functions to
avoid the need to revisit the file? My hands are too full ATM, and
this issue is basically the only one which prevents me from updating
the Emacs user manual with the tree-sitter info, which in turn is the
only issue that blocks the move to releasing the 29.0.90 pretest
tarball.
TIA
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 07:38:02 GMT)
Full text and
rfc822 format available.
Message #50 received at 60983 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: 60983 <at> debbugs.gnu.org, theo <at> thornhill.no
>> Date: Mon, 23 Jan 2023 18:52:33 +0200
>> From: Eli Zaretskii <eliz <at> gnu.org>
>>
>> > From: Theodor Thornhill <theo <at> thornhill.no>
>> > Cc: Yuan Fu <casouri <at> gmail.com>
>> > Date: Sat, 21 Jan 2023 12:48:58 +0100
>> >
>> > Eli Zaretskii <eliz <at> gnu.org> writes:
>> >
>> > > I started looking into providing user-level documentation for
>> > > tree-sitter based modes, and bumped into some issues:
>> > >
>> > > . How does one use treesit-font-lock-level?
>> > >
>> > > - It is not a customizable user option (unlike
>> > > font-lock-maximum-decoration), so it cannot be set via
>> > > customize-variable. Is there a reason not to make it a
>> > > defcustom?
>> > > - It automatically becomes buffer-local when set, and OTOH setting
>> > > it in a buffer does not produce fontifications according to the
>> > > level, and neither does setting it in a mode hook. So the only
>> > > way to change its value is by using setq-default, which I don't
>> > > think is the intent?
>> > > - Should we make the variable a defcustom?
>> > > - Should it be possible to customize it separately for each mode?
>> > > - Should we allow to change the level and then call some function
>> > > to re-fontify the current buffer according to the new level?
>> >
>> > I struggled with this too. I ended up setting it with setq-default,
>> > assuming I was just missing something very simple. I'm in favor for
>> > either a defcustom or honoring the font-lock-maximum-decoration values,
>> > specifically these settings:
>> >
>> > ```
>> > If t, use the maximum decoration available.
>> > If a number, use that level of decoration (or if not available the maximum).
>> > ```
>
> Let's just make it a defcustom for now, with the values it has today,
> including the default.
>
> The problems with honoring the value of font-lock-maximum-decoration
> are that (a) its default value is t in most (all?) modes, whereas we
> decided not to use 4 as the default value of treesit-font-lock-level;
> and (b) if changing treesit-font-lock-level's value doesn't require to
> kill the buffer and revisit the file (as I hope we will make it work),
> the instructions regarding changing the value of
> font-lock-maximum-decoration will depend on whether the mode does or
> doesn't use tree-sitter, which will make the instructions confusingly
> complex.
>
> Yuan or Theo, would one of you please make the change of making
> treesit-font-lock-level a defcustom, with a proper :set functions to
> avoid the need to revisit the file? My hands are too full ATM, and
> this issue is basically the only one which prevents me from updating
> the Emacs user manual with the tree-sitter info, which in turn is the
> only issue that blocks the move to releasing the 29.0.90 pretest
> tarball.
>
> TIA
I can take a stab at it :)
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 07:57:02 GMT)
Full text and
rfc822 format available.
Message #53 received at 60983 <at> debbugs.gnu.org (full text, mbox):
Theodor Thornhill <theo <at> thornhill.no> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> Cc: 60983 <at> debbugs.gnu.org, theo <at> thornhill.no
>>> Date: Mon, 23 Jan 2023 18:52:33 +0200
>>> From: Eli Zaretskii <eliz <at> gnu.org>
>>>
>>> > From: Theodor Thornhill <theo <at> thornhill.no>
>>> > Cc: Yuan Fu <casouri <at> gmail.com>
>>> > Date: Sat, 21 Jan 2023 12:48:58 +0100
>>> >
>>> > Eli Zaretskii <eliz <at> gnu.org> writes:
>>> >
>>> > > I started looking into providing user-level documentation for
>>> > > tree-sitter based modes, and bumped into some issues:
>>> > >
>>> > > . How does one use treesit-font-lock-level?
>>> > >
>>> > > - It is not a customizable user option (unlike
>>> > > font-lock-maximum-decoration), so it cannot be set via
>>> > > customize-variable. Is there a reason not to make it a
>>> > > defcustom?
>>> > > - It automatically becomes buffer-local when set, and OTOH setting
>>> > > it in a buffer does not produce fontifications according to the
>>> > > level, and neither does setting it in a mode hook. So the only
>>> > > way to change its value is by using setq-default, which I don't
>>> > > think is the intent?
>>> > > - Should we make the variable a defcustom?
>>> > > - Should it be possible to customize it separately for each mode?
>>> > > - Should we allow to change the level and then call some function
>>> > > to re-fontify the current buffer according to the new level?
>>> >
>>> > I struggled with this too. I ended up setting it with setq-default,
>>> > assuming I was just missing something very simple. I'm in favor for
>>> > either a defcustom or honoring the font-lock-maximum-decoration values,
>>> > specifically these settings:
>>> >
>>> > ```
>>> > If t, use the maximum decoration available.
>>> > If a number, use that level of decoration (or if not available the maximum).
>>> > ```
>>
>> Let's just make it a defcustom for now, with the values it has today,
>> including the default.
>>
>> The problems with honoring the value of font-lock-maximum-decoration
>> are that (a) its default value is t in most (all?) modes, whereas we
>> decided not to use 4 as the default value of treesit-font-lock-level;
>> and (b) if changing treesit-font-lock-level's value doesn't require to
>> kill the buffer and revisit the file (as I hope we will make it work),
>> the instructions regarding changing the value of
>> font-lock-maximum-decoration will depend on whether the mode does or
>> doesn't use tree-sitter, which will make the instructions confusingly
>> complex.
>>
>> Yuan or Theo, would one of you please make the change of making
>> treesit-font-lock-level a defcustom, with a proper :set functions to
>> avoid the need to revisit the file? My hands are too full ATM, and
>> this issue is basically the only one which prevents me from updating
>> the Emacs user manual with the tree-sitter info, which in turn is the
>> only issue that blocks the move to releasing the 29.0.90 pretest
>> tarball.
>>
>> TIA
>
>
> I can take a stab at it :)
>
> Theo
Thanks! I don’t have a ton of free time right now either.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 08:28:01 GMT)
Full text and
rfc822 format available.
Message #56 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> -(defcustom c-ts-mode-indent-style 'gnu
>> +(defcustom c-ts-mode-indent-style "gnu"
>> "Style used for indentation.
>>
>> The selected style could be one of GNU, K&R, LINUX or BSD. If
>> @@ -100,13 +100,33 @@ c-ts-mode-indent-style
>> set instead. This function is expected return a list that
>> follows the form of `treesit-simple-indent-rules'."
>> :version "29.1"
>> - :type '(choice (symbol :tag "Gnu" 'gnu)
>> - (symbol :tag "K&R" 'k&r)
>> - (symbol :tag "Linux" 'linux)
>> - (symbol :tag "BSD" 'bsd)
>> + :type '(choice (string :tag "Gnu" "gnu")
>> + (string :tag "K&R" "k&r")
>> + (string :tag "Linux" "linux")
>> + (string :tag "BSD" "bsd")
>> (function :tag "A function for user customized style" ignore))
>> + :set #'c-ts-mode--indent-style
>> :group 'c)
>
> Why change to strings?
> BTW the previous code seems wrong: instead of
>
> (symbol :tag "FOO" 'foo)
>
> it should be
>
> (symbol :tag "FOO" foo)
>
> since `'foo` is not a symbol but a list (of two symbols).
>
Thanks, fixed.
>> +(defun c-ts-mode--indent-style (sym val)
>> + "Custom setter for `c-ts-mode-indent-style'."
>> + (set-default sym val))
>
> Hmm... why bother use a `:set`ter if it doesn't do anything more than
> the default does?
> Shouldn't it call `treesit--indent-rules-optimize` to (re)set
> `treesit-simple-indent-rules`?
> [ Presumably in all relevant buffers, since the defcustom setting is
> global. ]
>
Thanks, done.
>> +(defun c-ts-mode-set-style ()
>> + (interactive)
>> + (or (eq major-mode 'c-ts-mode) (eq major-mode 'c++-ts-mode)
>> + (error "Buffer %s is not a c-ts-mode (c-ts-mode-set-style)"
>> + (buffer-name)))
>> + (if-let ((mode (cond ((eq major-mode 'c-ts-mode) 'c)
>> + ((eq major-mode 'c++-ts-mode) 'cpp)
>> + (t nil)))
>> + (choice (completing-read "Select style: " '("gnu" "k&r" "linux" "bsd"))))
>
> Here, we probably want to specify `must-match` to `completing-read`
> (which makes it unnecessary to check `if-let`, I think) and we should
> provide a default.
> Also we should probably use the (c-ts-mode--indent-styles mode) alist
> rather than hardcode the set of styles.
>
>> + (c-ts-mode--indent-style 'c-ts-mode-indent-style choice)
>> + (kill-local-variable 'treesit-simple-indent-rules)
>> + (setq-local treesit-simple-indent-rules
>> + (treesit--indent-rules-optimize
>> + (c-ts-mode--set-indent-style mode)))))
>
> Here we presumably want to do the (setq-local
> treesit-simple-indent-rules ...) every time (and set
> `c-ts-mode-indent-style` buffer locally rather than via
> `c-ts-mode--indent-style`, or otherwise provide an additional arg to
> `c-ts-mode--indent-style` to say whether it applies globally or only to
> the current buffer).
>
> BTW, the naming of `c-ts-mode--indent-style` and
> `c-ts-mode--set-indent-style` is confusing.
>
> Also: why `kill-local-variable` just before the `setq-local`?
>
>> - (pcase c-ts-mode-indent-style
>> - ('gnu (alist-get 'gnu (c-ts-mode--indent-styles mode)))
>> - ('k&r (alist-get 'k&r (c-ts-mode--indent-styles mode)))
>> - ('bsd (alist-get 'bsd (c-ts-mode--indent-styles mode)))
>> - ('linux (alist-get 'linux (c-ts-mode--indent-styles mode)))))))
>> + (alist-get c-ts-mode-indent-style
>> + (c-ts-mode--indent-styles mode) nil nil #'equal))))
>
> Thanks :-)
>
>
> Stefan
Does this patch look better? It also works, now :)
Theo
[0001-Initial-c-ts-mode-set-style-attempt.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Thu, 26 Jan 2023 09:08:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 60983 <at> debbugs.gnu.org
> Date: Thu, 26 Jan 2023 08:37:42 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > Yuan or Theo, would one of you please make the change of making
> > treesit-font-lock-level a defcustom, with a proper :set functions to
> > avoid the need to revisit the file? My hands are too full ATM, and
> > this issue is basically the only one which prevents me from updating
> > the Emacs user manual with the tree-sitter info, which in turn is the
> > only issue that blocks the move to releasing the 29.0.90 pretest
> > tarball.
> >
> > TIA
>
>
> I can take a stab at it :)
Thanks!
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 28 Jan 2023 13:14:01 GMT)
Full text and
rfc822 format available.
Message #62 received at 60983 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: 60983 <at> debbugs.gnu.org, theo <at> thornhill.no
>> Date: Mon, 23 Jan 2023 18:52:33 +0200
>> From: Eli Zaretskii <eliz <at> gnu.org>
>>
>> > From: Theodor Thornhill <theo <at> thornhill.no>
>> > Cc: Yuan Fu <casouri <at> gmail.com>
>> > Date: Sat, 21 Jan 2023 12:48:58 +0100
>> >
>> > Eli Zaretskii <eliz <at> gnu.org> writes:
>> >
>> > > I started looking into providing user-level documentation for
>> > > tree-sitter based modes, and bumped into some issues:
>> > >
>> > > . How does one use treesit-font-lock-level?
>> > >
>> > > - It is not a customizable user option (unlike
>> > > font-lock-maximum-decoration), so it cannot be set via
>> > > customize-variable. Is there a reason not to make it a
>> > > defcustom?
>> > > - It automatically becomes buffer-local when set, and OTOH setting
>> > > it in a buffer does not produce fontifications according to the
>> > > level, and neither does setting it in a mode hook. So the only
>> > > way to change its value is by using setq-default, which I don't
>> > > think is the intent?
>> > > - Should we make the variable a defcustom?
>> > > - Should it be possible to customize it separately for each mode?
>> > > - Should we allow to change the level and then call some function
>> > > to re-fontify the current buffer according to the new level?
>> >
>> > I struggled with this too. I ended up setting it with setq-default,
>> > assuming I was just missing something very simple. I'm in favor for
>> > either a defcustom or honoring the font-lock-maximum-decoration values,
>> > specifically these settings:
>> >
>> > ```
>> > If t, use the maximum decoration available.
>> > If a number, use that level of decoration (or if not available the maximum).
>> > ```
>
> Let's just make it a defcustom for now, with the values it has today,
> including the default.
>
> The problems with honoring the value of font-lock-maximum-decoration
> are that (a) its default value is t in most (all?) modes, whereas we
> decided not to use 4 as the default value of treesit-font-lock-level;
> and (b) if changing treesit-font-lock-level's value doesn't require to
> kill the buffer and revisit the file (as I hope we will make it work),
> the instructions regarding changing the value of
> font-lock-maximum-decoration will depend on whether the mode does or
> doesn't use tree-sitter, which will make the instructions confusingly
> complex.
>
> Yuan or Theo, would one of you please make the change of making
> treesit-font-lock-level a defcustom, with a proper :set functions to
> avoid the need to revisit the file? My hands are too full ATM, and
> this issue is basically the only one which prevents me from updating
> the Emacs user manual with the tree-sitter info, which in turn is the
> only issue that blocks the move to releasing the 29.0.90 pretest
> tarball.
>
> TIA
Are you okay with these patches, Eli? If so, then I can clean them up a
little and install on emacs-29. In both cases using
(customize-set-variable 'the-variable val) will take effect in the
buffer without having to reload anything.
Let me know if there are any other changes needed. I'll have some time
this evening to do some hacking if needed :)
Thanks,
Theo
[0001-Initial-c-ts-mode-set-style-attempt.patch (text/x-patch, attachment)]
[0001-Make-treesit-font-lock-level-a-defcustom.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 28 Jan 2023 13:27:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 60983 <at> debbugs.gnu.org
> Date: Sat, 28 Jan 2023 14:12:52 +0100
>
> > Yuan or Theo, would one of you please make the change of making
> > treesit-font-lock-level a defcustom, with a proper :set functions to
> > avoid the need to revisit the file? My hands are too full ATM, and
> > this issue is basically the only one which prevents me from updating
> > the Emacs user manual with the tree-sitter info, which in turn is the
> > only issue that blocks the move to releasing the 29.0.90 pretest
> > tarball.
> >
> > TIA
>
>
> Are you okay with these patches, Eli? If so, then I can clean them up a
> little and install on emacs-29. In both cases using
> (customize-set-variable 'the-variable val) will take effect in the
> buffer without having to reload anything.
LGTM, please install, and thanks.
> Let me know if there are any other changes needed. I'll have some time
> this evening to do some hacking if needed :)
Will do.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 28 Jan 2023 18:42:02 GMT)
Full text and
rfc822 format available.
Message #68 received at 60983 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Theodor Thornhill <theo <at> thornhill.no>
>> Cc: 60983 <at> debbugs.gnu.org
>> Date: Sat, 28 Jan 2023 14:12:52 +0100
>>
>> > Yuan or Theo, would one of you please make the change of making
>> > treesit-font-lock-level a defcustom, with a proper :set functions to
>> > avoid the need to revisit the file? My hands are too full ATM, and
>> > this issue is basically the only one which prevents me from updating
>> > the Emacs user manual with the tree-sitter info, which in turn is the
>> > only issue that blocks the move to releasing the 29.0.90 pretest
>> > tarball.
>> >
>> > TIA
>>
>>
>> Are you okay with these patches, Eli? If so, then I can clean them up a
>> little and install on emacs-29. In both cases using
>> (customize-set-variable 'the-variable val) will take effect in the
>> buffer without having to reload anything.
>
> LGTM, please install, and thanks.
>
>> Let me know if there are any other changes needed. I'll have some time
>> this evening to do some hacking if needed :)
>
> Will do.
Installed.
Theo
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 29 Jan 2023 13:26:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: casouri <at> gmail.com, 60983 <at> debbugs.gnu.org
> Date: Sat, 28 Jan 2023 19:41:26 +0100
>
> >> Let me know if there are any other changes needed. I'll have some time
> >> this evening to do some hacking if needed :)
> >
> > Will do.
>
> Installed.
Thanks, I've now added to the Emacs manual the user-level
documentation of tree-sitter features. Please take a look and tell if
anything is missing/incorrect.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 29 Jan 2023 13:34:02 GMT)
Full text and
rfc822 format available.
Message #74 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Mon, 23 Jan 2023 15:55:30 -0800
> Cc: Eli Zaretskii <eliz <at> gnu.org>,
> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>
> >> Thanks. What about the questions I asked regarding indentation
> >> features, and specifically about c-ts-mode-indent-style?
> >
> > I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
> >
> > I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
>
> One common approach is to iterate over all live buffer and reset the variable (in this case treesit-simple-indent-rules) on applicate buffers (in this case c/c++-ts-mode buffers).
>
> It would be nice to also have a command c-ts-mode-set-style (like c-set-style) that takes a style symbol and sets treesit-simple-indent-rules accordingly. And in major-mode setup, ie, c-ts-mode’s body, you call it with c-ts-mode-indent-style.
This command now exists, courtesy of Theo, but I see some strange
misbehavior with it in c++-ts-mode, related to keymap inheritance:
emacs -Q
M-x c-ts-mode RET
C-h c C-c C-q
=> C-c C-q runs the command c-ts-mode-indent-defun
But
M-x c++-ts-mode RET
C-h c C-c C-q
=> C-c C-q is undefined
This is strange, since the binding is defined in c-ts-mode-map, which
is used in c-ts-base-mode:
(defvar-keymap c-ts-mode-map
:doc "Keymap for the C language with tree-sitter"
:parent prog-mode-map
"C-c C-q" #'c-ts-mode-indent-defun
"C-c ." #'c-ts-mode-set-style)
;;;###autoload
(define-derived-mode c-ts-base-mode prog-mode "C"
"Major mode for editing C, powered by tree-sitter.
\\{c-ts-mode-map}"
:syntax-table c-ts-mode--syntax-table
and both c-ts-mode and c++-ts-mode derive from c-ts-base-mode:
(define-derived-mode c-ts-mode c-ts-base-mode "C"
"Major mode for editing C, powered by tree-sitter.
(define-derived-mode c++-ts-mode c-ts-base-mode "C++"
"Major mode for editing C++, powered by tree-sitter.
What's missing here? Stefan, any advice?
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 29 Jan 2023 19:13:02 GMT)
Full text and
rfc822 format available.
Message #77 received at 60983 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Mon, 23 Jan 2023 15:55:30 -0800
>> Cc: Eli Zaretskii <eliz <at> gnu.org>,
>> Bug Report Emacs <bug-gnu-emacs <at> gnu.org>
>>
>> >> Thanks. What about the questions I asked regarding indentation
>> >> features, and specifically about c-ts-mode-indent-style?
>> >
>> > I am working on that, but I hit some issues where I cannot make treesit recognize the new settings before the whole treesit-major-mode-setup reruns. Just setting the symbol doesn't work, and reenabling the mode inside of the :set function isn't the best idea maybe?
>> >
>> > I'd love some pointers to how other modes do similar stuff, but I didn't really find anything.
>>
>> One common approach is to iterate over all live buffer and reset the variable (in this case treesit-simple-indent-rules) on applicate buffers (in this case c/c++-ts-mode buffers).
>>
>> It would be nice to also have a command c-ts-mode-set-style (like c-set-style) that takes a style symbol and sets treesit-simple-indent-rules accordingly. And in major-mode setup, ie, c-ts-mode’s body, you call it with c-ts-mode-indent-style.
>
> This command now exists, courtesy of Theo, but I see some strange
> misbehavior with it in c++-ts-mode, related to keymap inheritance:
>
> emacs -Q
> M-x c-ts-mode RET
> C-h c C-c C-q
> => C-c C-q runs the command c-ts-mode-indent-defun
>
> But
>
> M-x c++-ts-mode RET
> C-h c C-c C-q
> => C-c C-q is undefined
>
> This is strange, since the binding is defined in c-ts-mode-map, which
> is used in c-ts-base-mode:
>
> (defvar-keymap c-ts-mode-map
> :doc "Keymap for the C language with tree-sitter"
> :parent prog-mode-map
> "C-c C-q" #'c-ts-mode-indent-defun
> "C-c ." #'c-ts-mode-set-style)
>
> ;;;###autoload
> (define-derived-mode c-ts-base-mode prog-mode "C"
> "Major mode for editing C, powered by tree-sitter.
>
> \\{c-ts-mode-map}"
> :syntax-table c-ts-mode--syntax-table
>
> and both c-ts-mode and c++-ts-mode derive from c-ts-base-mode:
>
> (define-derived-mode c-ts-mode c-ts-base-mode "C"
> "Major mode for editing C, powered by tree-sitter.
>
> (define-derived-mode c++-ts-mode c-ts-base-mode "C++"
> "Major mode for editing C++, powered by tree-sitter.
>
> What's missing here? Stefan, any advice?
I'm sure you know this, but adding the below patch "fixes" it. It seems
like the inheritance isn't registered somehow without a defined
mode-map?
Theo
diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 612c41bf07..e9f9eea69c 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -703,6 +703,10 @@ c-ts-mode-map
"C-c C-q" #'c-ts-mode-indent-defun
"C-c ." #'c-ts-mode-set-style)
+(defvar-keymap c++-ts-mode-map
+ :doc "Keymap for the C++ language with tree-sitter"
+ :parent c-ts-mode-map)
+
;;;###autoload
(define-derived-mode c-ts-base-mode prog-mode "C"
"Major mode for editing C, powered by tree-sitter.
@@ -810,7 +814,9 @@ c++-ts-mode
(add-to-list \\='major-mode-remap-alist
\\='(c-or-c++-mode . c-or-c++-ts-mode))
-in your configuration."
+in your configuration.
+
+\\{c++-ts-mode-map}"
:group 'c++
(when (treesit-ready-p 'cpp)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 29 Jan 2023 19:42:02 GMT)
Full text and
rfc822 format available.
Message #80 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Theodor Thornhill <theo <at> thornhill.no>
> Cc: 60983 <at> debbugs.gnu.org
> Date: Sun, 29 Jan 2023 20:12:39 +0100
>
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> > What's missing here? Stefan, any advice?
>
>
> I'm sure you know this, but adding the below patch "fixes" it. It seems
> like the inheritance isn't registered somehow without a defined
> mode-map?
I asked Stefan to help precisely because I don't understand why we
need anything beyond what we have already. I thought mode inheritance
should have taken care of it, or at least the documentation of
define-derived-mode seems to imply that.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 30 Jan 2023 02:29:01 GMT)
Full text and
rfc822 format available.
Message #83 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> I asked Stefan to help precisely because I don't understand why we
> need anything beyond what we have already. I thought mode inheritance
> should have taken care of it, or at least the documentation of
> define-derived-mode seems to imply that.
But the `define-derived-mode` uses `prog-mode` as parent, so while the
keymap does inherit from its parent mode's keymap, it's just
`prog-mode-map` rather than `c-ts-mode-map`.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Mon, 30 Jan 2023 13:46:02 GMT)
Full text and
rfc822 format available.
Message #86 received at 60983 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Theodor Thornhill <theo <at> thornhill.no>, casouri <at> gmail.com,
> 60983 <at> debbugs.gnu.org
> Date: Sun, 29 Jan 2023 21:28:29 -0500
>
> > I asked Stefan to help precisely because I don't understand why we
> > need anything beyond what we have already. I thought mode inheritance
> > should have taken care of it, or at least the documentation of
> > define-derived-mode seems to imply that.
>
> But the `define-derived-mode` uses `prog-mode` as parent, so while the
> keymap does inherit from its parent mode's keymap, it's just
> `prog-mode-map` rather than `c-ts-mode-map`.
Ah, so it's this bit of define-derived-mode's documentation:
• The new mode has its own sparse keymap, named ‘VARIANT-map’.
‘define-derived-mode’ makes the parent mode’s keymap the
parent of the new map, unless ‘VARIANT-map’ is already set and
already has a parent. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
^^^^^^^^^^^^^^^^^^^^
IOW, the problem is that c-ts-base-mode defined c-ts-mode-map as its
keymap, so when c-ts-mode is defined, its keymap variable "is already
set and already has a parent".
Thanks, this is now fixed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Fri, 03 Feb 2023 03:08:02 GMT)
Full text and
rfc822 format available.
Message #89 received at submit <at> debbugs.gnu.org (full text, mbox):
I just read the new manual sections, thanks for the work! Do we want to mention treesit-font-lock-recompute-features in the user manual? That’s the only way for someone to add/remove specific features (as opposed to changing the decoration level). (We might also want to mention that changing treesit-font-lock-features directly doesn’t have any effect, similar to treesit-font-lock-level.)
treesit-font-lock-recompute-features is intended to be used in major mode hooks, like
(add-hook 'c-ts-mode-hook #'c-ts-setup)
(defun c-ts-mode-setup ()
(treesit-font-lock-recompute-features
'(emacs-devel)
'(property bracket delimiter operator variable function)))
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Fri, 03 Feb 2023 07:48:01 GMT)
Full text and
rfc822 format available.
Message #92 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Thu, 2 Feb 2023 19:07:07 -0800
> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
> Theodor Thornhill <theo <at> thornhill.no>
>
> I just read the new manual sections, thanks for the work! Do we want to mention treesit-font-lock-recompute-features in the user manual? That’s the only way for someone to add/remove specific features (as opposed to changing the decoration level). (We might also want to mention that changing treesit-font-lock-features directly doesn’t have any effect, similar to treesit-font-lock-level.)
treesit-font-lock-recompute-features is a non-interactive function, so
mentioning it in the user manual is generally inappropriate. Users
are unlikely to add features that aren't already defined in the mode's
font-lock setup.
This function and its use in these situations are described in the
ELisp manual, where I think it belongs.
As for the fact that changing treesit-font-lock-feature-list directly
doesn't have any effect, that is already in the doc string. I'm not
opposed to mentioning that in the manual as well, but I see no problem
with what we have.
> treesit-font-lock-recompute-features is intended to be used in major mode hooks, like
>
> (add-hook 'c-ts-mode-hook #'c-ts-setup)
>
> (defun c-ts-mode-setup ()
> (treesit-font-lock-recompute-features
> '(emacs-devel)
> '(property bracket delimiter operator variable function)))
This belongs to the ELisp manual, IMO.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sat, 04 Feb 2023 23:39:01 GMT)
Full text and
rfc822 format available.
Message #95 received at submit <at> debbugs.gnu.org (full text, mbox):
> On Feb 2, 2023, at 11:47 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Thu, 2 Feb 2023 19:07:07 -0800
>> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
>> Theodor Thornhill <theo <at> thornhill.no>
>>
>> I just read the new manual sections, thanks for the work! Do we want to mention treesit-font-lock-recompute-features in the user manual? That’s the only way for someone to add/remove specific features (as opposed to changing the decoration level). (We might also want to mention that changing treesit-font-lock-features directly doesn’t have any effect, similar to treesit-font-lock-level.)
>
> treesit-font-lock-recompute-features is a non-interactive function, so
> mentioning it in the user manual is generally inappropriate. Users
> are unlikely to add features that aren't already defined in the mode's
> font-lock setup.
>
> This function and its use in these situations are described in the
> ELisp manual, where I think it belongs.
>
> As for the fact that changing treesit-font-lock-feature-list directly
> doesn't have any effect, that is already in the doc string. I'm not
> opposed to mentioning that in the manual as well, but I see no problem
> with what we have.
I see. Sounds good to me. I meant enabling/disabling features when I say “adding/removing” features. Does that make anything different?
>
>> treesit-font-lock-recompute-features is intended to be used in major mode hooks, like
>>
>> (add-hook 'c-ts-mode-hook #'c-ts-setup)
>>
>> (defun c-ts-mode-setup ()
>> (treesit-font-lock-recompute-features
>> '(emacs-devel)
>> '(property bracket delimiter operator variable function)))
>
> This belongs to the ELisp manual, IMO.
Ok, then I think there’s nothing needed to change in the Emacs manual.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 05 Feb 2023 06:02:02 GMT)
Full text and
rfc822 format available.
Message #98 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Sat, 4 Feb 2023 15:38:17 -0800
> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
> theo <at> thornhill.no
>
> > treesit-font-lock-recompute-features is a non-interactive function, so
> > mentioning it in the user manual is generally inappropriate. Users
> > are unlikely to add features that aren't already defined in the mode's
> > font-lock setup.
> >
> > This function and its use in these situations are described in the
> > ELisp manual, where I think it belongs.
> >
> > As for the fact that changing treesit-font-lock-feature-list directly
> > doesn't have any effect, that is already in the doc string. I'm not
> > opposed to mentioning that in the manual as well, but I see no problem
> > with what we have.
>
> I see. Sounds good to me. I meant enabling/disabling features when I say “adding/removing” features. Does that make anything different?
Yes, that's how I understood what you were saying: changing the list
of features enabled/disabled by specific levels. This is not a
user-level thing, so describing it in the ELisp manual is good enough,
I think. (If it turns out users want to do this kind of thing too
often, it probably means our design of the user-facing features is
sub-optimal and should be improved.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 05 Feb 2023 07:56:01 GMT)
Full text and
rfc822 format available.
Message #101 received at submit <at> debbugs.gnu.org (full text, mbox):
> On Feb 4, 2023, at 10:01 PM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Sat, 4 Feb 2023 15:38:17 -0800
>> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
>> theo <at> thornhill.no
>>
>>> treesit-font-lock-recompute-features is a non-interactive function, so
>>> mentioning it in the user manual is generally inappropriate. Users
>>> are unlikely to add features that aren't already defined in the mode's
>>> font-lock setup.
>>>
>>> This function and its use in these situations are described in the
>>> ELisp manual, where I think it belongs.
>>>
>>> As for the fact that changing treesit-font-lock-feature-list directly
>>> doesn't have any effect, that is already in the doc string. I'm not
>>> opposed to mentioning that in the manual as well, but I see no problem
>>> with what we have.
>>
>> I see. Sounds good to me. I meant enabling/disabling features when I say “adding/removing” features. Does that make anything different?
>
> Yes, that's how I understood what you were saying: changing the list
> of features enabled/disabled by specific levels. This is not a
> user-level thing, so describing it in the ELisp manual is good enough,
> I think. (If it turns out users want to do this kind of thing too
> often, it probably means our design of the user-facing features is
> sub-optimal and should be improved.)
I see, my description and the documentation is still not clear enough, I’m afraid. treesit-font-lock-recompute-feature does not add/remove features that belongs to a level. The design is that, the user uses decoration level to set the rough level, which enables a set of features, then use treesit-font-lock-recompute-feature to do more fine-grained control by additionally enabling/disabling features.
For example in c-ts-mode, if I set the decoration level to 2, I’d have these features: comment, definition, keyword, preprocessor, string, type. If I also want the assignment features, which is in level 3, but don’t want other features in level 3, I would use treesit-font-lock-recompute-feature to enable that feature. Similarly, I can use treesit-font-lock-recompute-feature to disable the preprocessor which is at level 2, without affecting other features status.
Yuan
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 05 Feb 2023 09:24:01 GMT)
Full text and
rfc822 format available.
Message #104 received at submit <at> debbugs.gnu.org (full text, mbox):
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Sat, 4 Feb 2023 23:54:56 -0800
> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
> theo <at> thornhill.no
>
> > Yes, that's how I understood what you were saying: changing the list
> > of features enabled/disabled by specific levels. This is not a
> > user-level thing, so describing it in the ELisp manual is good enough,
> > I think. (If it turns out users want to do this kind of thing too
> > often, it probably means our design of the user-facing features is
> > sub-optimal and should be improved.)
>
> I see, my description and the documentation is still not clear enough, I’m afraid. treesit-font-lock-recompute-feature does not add/remove features that belongs to a level. The design is that, the user uses decoration level to set the rough level, which enables a set of features, then use treesit-font-lock-recompute-feature to do more fine-grained control by additionally enabling/disabling features.
>
> For example in c-ts-mode, if I set the decoration level to 2, I’d have these features: comment, definition, keyword, preprocessor, string, type. If I also want the assignment features, which is in level 3, but don’t want other features in level 3, I would use treesit-font-lock-recompute-feature to enable that feature. Similarly, I can use treesit-font-lock-recompute-feature to disable the preprocessor which is at level 2, without affecting other features status.
That's exactly what I understood, and that was what I responded to. I
don't think it's a user-level feature to tweak the list of features
that are enabled/disables by a certain decoration level. It is on the
level of Lisp programming, and therefore should be described in the
ELisp manual.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#60983
; Package
emacs
.
(Sun, 05 Feb 2023 09:44:02 GMT)
Full text and
rfc822 format available.
Message #107 received at submit <at> debbugs.gnu.org (full text, mbox):
> On Feb 5, 2023, at 1:23 AM, Eli Zaretskii <eliz <at> gnu.org> wrote:
>
>> From: Yuan Fu <casouri <at> gmail.com>
>> Date: Sat, 4 Feb 2023 23:54:56 -0800
>> Cc: Bug Report Emacs <bug-gnu-emacs <at> gnu.org>,
>> theo <at> thornhill.no
>>
>>> Yes, that's how I understood what you were saying: changing the list
>>> of features enabled/disabled by specific levels. This is not a
>>> user-level thing, so describing it in the ELisp manual is good enough,
>>> I think. (If it turns out users want to do this kind of thing too
>>> often, it probably means our design of the user-facing features is
>>> sub-optimal and should be improved.)
>>
>> I see, my description and the documentation is still not clear enough, I’m afraid. treesit-font-lock-recompute-feature does not add/remove features that belongs to a level. The design is that, the user uses decoration level to set the rough level, which enables a set of features, then use treesit-font-lock-recompute-feature to do more fine-grained control by additionally enabling/disabling features.
>>
>> For example in c-ts-mode, if I set the decoration level to 2, I’d have these features: comment, definition, keyword, preprocessor, string, type. If I also want the assignment features, which is in level 3, but don’t want other features in level 3, I would use treesit-font-lock-recompute-feature to enable that feature. Similarly, I can use treesit-font-lock-recompute-feature to disable the preprocessor which is at level 2, without affecting other features status.
>
> That's exactly what I understood, and that was what I responded to. I
> don't think it's a user-level feature to tweak the list of features
> that are enabled/disables by a certain decoration level. It is on the
> level of Lisp programming, and therefore should be described in the
> ELisp manual.
Oh! All is well, then :-)
Yuan
This bug report was last modified 1 year and 295 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.