* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong @ 2022-11-29 18:41 Bruce Stephens 2022-12-02 5:13 ` Yuan Fu 2022-12-03 10:48 ` Yuan Fu 0 siblings, 2 replies; 10+ messages in thread From: Bruce Stephens @ 2022-11-29 18:41 UTC (permalink / raw) To: 59686 With the following file I would expect foo(i) to be indented to just two indents (like the first one), but they are not. Similarly for many other compound statements where the opening spans more than one line. // -*- c-ts -*- int main() { for (int i=0; i<10; ++i) { foo(i); } for (int i=0; i<10; ++i) { foo(i); } } In GNU Emacs 30.0.50 (build 1, x86_64-apple-darwin21.6.0, NS appkit-2113.60 Version 12.6 (Build 21G115)) of 2022-11-29 built on Bruces-MacBook-Pro.local Repository revision: e90c21e6fb35d1c844c49d96f0b62c5fe55bb203 Repository branch: master Windowing system distributor 'Apple', version 10.3.2113 System Description: macOS 12.6 Configured using: 'configure --with-native-compilation=aot --with-tree-sitter' Configured features: ACL GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB Important settings: value of $LANG: en_GB.UTF-8 locale-coding-system: utf-8-unix 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 text-property-search time-date mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr mail-utils comp comp-cstr warnings icons subr-x rx cl-macs gv cl-extra help-mode bytecomp byte-compile c-ts-mode treesit cl-seq cl-loaddefs cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/ns-win ns-win ucs-normalize mule-util 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 kqueue cocoa ns lcms2 multi-tty make-network-process native-compile emacs) Memory information: ((conses 16 81423 9034) (symbols 48 7273 0) (strings 32 20423 1742) (string-bytes 1 634204) (vectors 16 17092) (vector-slots 8 341908 8957) (floats 8 32 58) (intervals 56 242 0) (buffers 992 12)) ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-11-29 18:41 bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong Bruce Stephens @ 2022-12-02 5:13 ` Yuan Fu 2022-12-02 5:42 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-03 10:48 ` Yuan Fu 1 sibling, 1 reply; 10+ messages in thread From: Yuan Fu @ 2022-12-02 5:13 UTC (permalink / raw) To: bruce.stephens; +Cc: Theodor Thornhill, 59686 Bruce Stephens <bruce.stephens@isode.com> writes: > With the following file I would expect foo(i) to be indented to just two > indents (like the first one), but they are not. Similarly for many other > compound statements where the opening spans more than one line. > > // -*- c-ts -*- > int main() { > for (int i=0; i<10; ++i) { > foo(i); > } > > for (int i=0; > i<10; > ++i) { > foo(i); > } > } I see, that’s because the indent rule finds the BOL of the line where the "{" is on, and indents from there. Theo, WDYT? Does indent style fix this, or we should change the indent rules? Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-02 5:13 ` Yuan Fu @ 2022-12-02 5:42 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-02 8:39 ` Eli Zaretskii 0 siblings, 1 reply; 10+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 5:42 UTC (permalink / raw) To: Yuan Fu, bruce.stephens; +Cc: 59686 On 2 December 2022 06:13:42 CET, Yuan Fu <casouri@gmail.com> wrote: > >Bruce Stephens <bruce.stephens@isode.com> writes: > >> With the following file I would expect foo(i) to be indented to just two >> indents (like the first one), but they are not. Similarly for many other >> compound statements where the opening spans more than one line. >> >> // -*- c-ts -*- >> int main() { >> for (int i=0; i<10; ++i) { >> foo(i); >> } >> >> for (int i=0; >> i<10; >> ++i) { >> foo(i); >> } >> } > >I see, that’s because the indent rule finds the BOL of the line where >the "{" is on, and indents from there. Theo, WDYT? Does indent style >fix this, or we should change the indent rules? > >Yuan I've seen this issue myself, and have tried several combinations to fix it. It is trivial to fix this particular case, but because compound_statement is used everywhere problems will pop up other places. The fix here would be some grand-parent-bol function, but if my memory serves that would mess up the else in an if else. I'm happy to see this fixed, but just remember that it's not _super_ trivial. Other languages exhibit similar issues, but with other constructs. Lastly, I _did_ fix most of these, but that required using query as the matcher in simple-indent-rules, which made indenting slower, and I needed one rule for almost every construct in a language. I'm sure she fix can be simple, but I'm a little blind to it right now. My eyes are bleeding tree-sitter nodes at the moment, so I might not see clearly :P ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-02 5:42 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 8:39 ` Eli Zaretskii 2022-12-02 9:20 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-02 10:46 ` Bruce Stephens 0 siblings, 2 replies; 10+ messages in thread From: Eli Zaretskii @ 2022-12-02 8:39 UTC (permalink / raw) To: Theodor Thornhill; +Cc: bruce.stephens, casouri, 59686 > Cc: 59686@debbugs.gnu.org > Date: Fri, 02 Dec 2022 06:42:03 +0100 > From: Theodor Thornhill via "Bug reports for GNU Emacs, > the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> > > >> // -*- c-ts -*- > >> int main() { > >> for (int i=0; i<10; ++i) { > >> foo(i); > >> } > >> > >> for (int i=0; > >> i<10; > >> ++i) { > >> foo(i); > >> } > >> } > > > >I see, that’s because the indent rule finds the BOL of the line where > >the "{" is on, and indents from there. Theo, WDYT? Does indent style > >fix this, or we should change the indent rules? > > > >Yuan > > I've seen this issue myself, and have tried several combinations to fix it. It is trivial to fix this particular case, but because compound_statement is used everywhere problems will pop up other places. The fix here would be some grand-parent-bol function, but if my memory serves that would mess up the else in an if else. I'm happy to see this fixed, but just remember that it's not _super_ trivial. > > Other languages exhibit similar issues, but with other constructs. FWIW, this is an unusual style, so I see no catastrophe if it is not 110% according to expectations. Users can easily fix that by tweaking their BOLs where important. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-02 8:39 ` Eli Zaretskii @ 2022-12-02 9:20 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-02 10:46 ` Bruce Stephens 1 sibling, 0 replies; 10+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 9:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: bruce.stephens, casouri, 59686 Eli Zaretskii <eliz@gnu.org> writes: >> Cc: 59686@debbugs.gnu.org >> Date: Fri, 02 Dec 2022 06:42:03 +0100 >> From: Theodor Thornhill via "Bug reports for GNU Emacs, >> the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> >> >> >> // -*- c-ts -*- >> >> int main() { >> >> for (int i=0; i<10; ++i) { >> >> foo(i); >> >> } >> >> >> >> for (int i=0; >> >> i<10; >> >> ++i) { >> >> foo(i); >> >> } >> >> } >> > >> >I see, that’s because the indent rule finds the BOL of the line where >> >the "{" is on, and indents from there. Theo, WDYT? Does indent style >> >fix this, or we should change the indent rules? >> > >> >Yuan >> >> I've seen this issue myself, and have tried several combinations to fix it. It is trivial to fix this particular case, but because compound_statement is used everywhere problems will pop up other places. The fix here would be some grand-parent-bol function, but if my memory serves that would mess up the else in an if else. I'm happy to see this fixed, but just remember that it's not _super_ trivial. >> >> Other languages exhibit similar issues, but with other constructs. > > FWIW, this is an unusual style, so I see no catastrophe if it is not 110% > according to expectations. Users can easily fix that by tweaking their BOLs > where important. Yeah, that was my reasoning as well. I think we should come up with some preset that can do this, but maybe when we know even more about how to deal with similar edge cases? ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-02 8:39 ` Eli Zaretskii 2022-12-02 9:20 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 10:46 ` Bruce Stephens 2022-12-02 11:51 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 10+ messages in thread From: Bruce Stephens @ 2022-12-02 10:46 UTC (permalink / raw) To: Eli Zaretskii, Theodor Thornhill; +Cc: casouri, 59686 On 02/12/2022 08:39, Eli Zaretskii wrote: > FWIW, this is an unusual style, so I see no catastrophe if it is not 110% > according to expectations. Users can easily fix that by tweaking their BOLs > where important. The example I gave would be unusual, I think, but I'd argue that the situations where I saw the problem are quite natural. For example, } else if ( MYSTRCMP (attname, SOME_PREFIX_X400ADDRESS) || MYSTRCMP (attname, SOME_PREFIX_X400) ) { FOO_ptr orp = foo_std2foo (val); or a function declaration with several arguments with types that are rather long. I agree it's not a critical bug but if there's no appropriate general fix it would be helpful to have some guidance for users to resolve our specific cases. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-02 10:46 ` Bruce Stephens @ 2022-12-02 11:51 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 0 siblings, 0 replies; 10+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-02 11:51 UTC (permalink / raw) To: Bruce Stephens, Eli Zaretskii; +Cc: casouri, 59686 Bruce Stephens <bruce.stephens@isode.com> writes: > On 02/12/2022 08:39, Eli Zaretskii wrote: > >> FWIW, this is an unusual style, so I see no catastrophe if it is not 110% >> according to expectations. Users can easily fix that by tweaking their BOLs >> where important. > > > The example I gave would be unusual, I think, but I'd argue that the > situations where I saw the problem are quite natural. > > For example, > > } else if ( MYSTRCMP (attname, SOME_PREFIX_X400ADDRESS) || > MYSTRCMP (attname, SOME_PREFIX_X400) ) { > FOO_ptr orp = foo_std2foo (val); > > or a function declaration with several arguments with types that are > rather long. > > I agree it's not a critical bug but if there's no appropriate general > fix it would be helpful to have some guidance for users to resolve our > specific cases. This is the case I was thinking of. In the for-loop a grand-parent-bol on compound_statement rule would match the 'for' keyword, so the indentation will be correct, but this one will not, IIRC. I plan to dig into this some more soon, but motivation left me a little on that issue. Maybe we could make a preset like: ``` (seq (parent-is "compound_statement") parent (parent-is "for_statement") bol) ``` In other words, make other presets execute sequentially, move point, check again, and if all are true, pick indent offset. Or allow multiple captures, like so: ``` (for_statement @offset-anchor body: (compound_statement (_) @to-indent)) ``` Here the @to-indent capture would get the new indent level based on treesit-node-start of for_statement. What do you think, Yuan? Theo ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-11-29 18:41 bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong Bruce Stephens 2022-12-02 5:13 ` Yuan Fu @ 2022-12-03 10:48 ` Yuan Fu 2022-12-03 11:08 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 1 sibling, 1 reply; 10+ messages in thread From: Yuan Fu @ 2022-12-03 10:48 UTC (permalink / raw) To: Theodor Thornhill; +Cc: bruce.stephens, Eli Zaretskii, 59686 Theodor Thornhill <theo@thornhill.no> writes: > Bruce Stephens <bruce.stephens@isode.com> writes: > >> On 02/12/2022 08:39, Eli Zaretskii wrote: >> >>> FWIW, this is an unusual style, so I see no catastrophe if it is not 110% >>> according to expectations. Users can easily fix that by tweaking their BOLs >>> where important. >> >> >> The example I gave would be unusual, I think, but I'd argue that the >> situations where I saw the problem are quite natural. >> >> For example, >> >> } else if ( MYSTRCMP (attname, SOME_PREFIX_X400ADDRESS) || >> MYSTRCMP (attname, SOME_PREFIX_X400) ) { >> FOO_ptr orp = foo_std2foo (val); >> >> or a function declaration with several arguments with types that are >> rather long. >> >> I agree it's not a critical bug but if there's no appropriate general >> fix it would be helpful to have some guidance for users to resolve our >> specific cases. > > This is the case I was thinking of. In the for-loop a grand-parent-bol > on compound_statement rule would match the 'for' keyword, so the > indentation will be correct, but this one will not, IIRC. I plan to dig > into this some more soon, but motivation left me a little on that issue. > Maybe we could make a preset like: > > ``` > (seq > (parent-is "compound_statement") parent (parent-is "for_statement") bol) > ``` > > > In other words, make other presets execute sequentially, move point, > check again, and if all are true, pick indent offset. Or allow multiple > captures, like so: > > ``` > (for_statement @offset-anchor > body: (compound_statement (_) @to-indent)) > ``` > > Here the @to-indent capture would get the new indent level based on > treesit-node-start of for_statement. > > What do you think, Yuan? I think we can just test for the grandparent, there is an (undocumented) matcher n-p-gp which matches parent and grandparent. Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-03 10:48 ` Yuan Fu @ 2022-12-03 11:08 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-03 11:19 ` Yuan Fu 0 siblings, 1 reply; 10+ messages in thread From: Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-03 11:08 UTC (permalink / raw) To: Yuan Fu; +Cc: bruce.stephens, Eli Zaretskii, 59686 On 3 December 2022 11:48:34 CET, Yuan Fu <casouri@gmail.com> wrote: > >Theodor Thornhill <theo@thornhill.no> writes: > >> Bruce Stephens <bruce.stephens@isode.com> writes: >> >>> On 02/12/2022 08:39, Eli Zaretskii wrote: >>> >>>> FWIW, this is an unusual style, so I see no catastrophe if it is not 110% >>>> according to expectations. Users can easily fix that by tweaking their BOLs >>>> where important. >>> >>> >>> The example I gave would be unusual, I think, but I'd argue that the >>> situations where I saw the problem are quite natural. >>> >>> For example, >>> >>> } else if ( MYSTRCMP (attname, SOME_PREFIX_X400ADDRESS) || >>> MYSTRCMP (attname, SOME_PREFIX_X400) ) { >>> FOO_ptr orp = foo_std2foo (val); >>> >>> or a function declaration with several arguments with types that are >>> rather long. >>> >>> I agree it's not a critical bug but if there's no appropriate general >>> fix it would be helpful to have some guidance for users to resolve our >>> specific cases. >> >> This is the case I was thinking of. In the for-loop a grand-parent-bol >> on compound_statement rule would match the 'for' keyword, so the >> indentation will be correct, but this one will not, IIRC. I plan to dig >> into this some more soon, but motivation left me a little on that issue. >> Maybe we could make a preset like: >> >> ``` >> (seq >> (parent-is "compound_statement") parent (parent-is "for_statement") bol) >> ``` >> >> >> In other words, make other presets execute sequentially, move point, >> check again, and if all are true, pick indent offset. Or allow multiple >> captures, like so: >> >> ``` >> (for_statement @offset-anchor >> body: (compound_statement (_) @to-indent)) >> ``` >> >> Here the @to-indent capture would get the new indent level based on >> treesit-node-start of for_statement. >> >> What do you think, Yuan? > >I think we can just test for the grandparent, there is an >(undocumented) matcher n-p-gp which matches parent and grandparent. > >Yuan Yeah I know, but that doesn't work in every case we see this behavior. ^ permalink raw reply [flat|nested] 10+ messages in thread
* bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong 2022-12-03 11:08 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2022-12-03 11:19 ` Yuan Fu 0 siblings, 0 replies; 10+ messages in thread From: Yuan Fu @ 2022-12-03 11:19 UTC (permalink / raw) To: Theodor Thornhill; +Cc: bruce.stephens, Eli Zaretskii, 59686 > On Dec 3, 2022, at 3:08 AM, Theodor Thornhill <theo@thornhill.no> wrote: > > > > On 3 December 2022 11:48:34 CET, Yuan Fu <casouri@gmail.com> wrote: >> >> Theodor Thornhill <theo@thornhill.no> writes: >> >>> Bruce Stephens <bruce.stephens@isode.com> writes: >>> >>>> On 02/12/2022 08:39, Eli Zaretskii wrote: >>>> >>>>> FWIW, this is an unusual style, so I see no catastrophe if it is not 110% >>>>> according to expectations. Users can easily fix that by tweaking their BOLs >>>>> where important. >>>> >>>> >>>> The example I gave would be unusual, I think, but I'd argue that the >>>> situations where I saw the problem are quite natural. >>>> >>>> For example, >>>> >>>> } else if ( MYSTRCMP (attname, SOME_PREFIX_X400ADDRESS) || >>>> MYSTRCMP (attname, SOME_PREFIX_X400) ) { >>>> FOO_ptr orp = foo_std2foo (val); >>>> >>>> or a function declaration with several arguments with types that are >>>> rather long. >>>> >>>> I agree it's not a critical bug but if there's no appropriate general >>>> fix it would be helpful to have some guidance for users to resolve our >>>> specific cases. >>> >>> This is the case I was thinking of. In the for-loop a grand-parent-bol >>> on compound_statement rule would match the 'for' keyword, so the >>> indentation will be correct, but this one will not, IIRC. I plan to dig >>> into this some more soon, but motivation left me a little on that issue. >>> Maybe we could make a preset like: >>> >>> ``` >>> (seq >>> (parent-is "compound_statement") parent (parent-is "for_statement") bol) >>> ``` >>> >>> >>> In other words, make other presets execute sequentially, move point, >>> check again, and if all are true, pick indent offset. Or allow multiple >>> captures, like so: >>> >>> ``` >>> (for_statement @offset-anchor >>> body: (compound_statement (_) @to-indent)) >>> ``` >>> >>> Here the @to-indent capture would get the new indent level based on >>> treesit-node-start of for_statement. >>> >>> What do you think, Yuan? >> >> I think we can just test for the grandparent, there is an >> (undocumented) matcher n-p-gp which matches parent and grandparent. >> >> Yuan > > Yeah I know, but that doesn't work in every case we see this behavior. I see, but at least it fixes common cases that I can think of right now, namely if, for, while. What are some other cases? Yuan ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-12-03 11:19 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-11-29 18:41 bug#59686: 30.0.50; tree-sitter indentation in some loops and conditional statements is wrong Bruce Stephens 2022-12-02 5:13 ` Yuan Fu 2022-12-02 5:42 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-02 8:39 ` Eli Zaretskii 2022-12-02 9:20 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-02 10:46 ` Bruce Stephens 2022-12-02 11:51 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-03 10:48 ` Yuan Fu 2022-12-03 11:08 ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors 2022-12-03 11:19 ` Yuan Fu
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).