GNU bug report logs - #59686
30.0.50; tree-sitter indentation in some loops and conditional statements is wrong

Previous Next

Package: emacs;

Reported by: Bruce Stephens <bruce.stephens <at> isode.com>

Date: Tue, 29 Nov 2022 18:42:01 UTC

Severity: normal

Merged with 60398, 60496

Found in versions 29.0.60, 30.0.50

To reply to this bug, email your comments to 59686 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Tue, 29 Nov 2022 18:42:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Bruce Stephens <bruce.stephens <at> isode.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 29 Nov 2022 18:42:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Bruce Stephens <bruce.stephens <at> isode.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; tree-sitter indentation in some loops and conditional
 statements is wrong
Date: Tue, 29 Nov 2022 18:41:27 +0000
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))





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 05:14:02 GMT) Full text and rfc822 format available.

Message #8 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: bruce.stephens <at> isode.com
Cc: Theodor Thornhill <theo <at> thornhill.no>, 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and 
 conditional statements is wrong
Date: Thu, 1 Dec 2022 21:13:42 -0800
Bruce Stephens <bruce.stephens <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 05:43:02 GMT) Full text and rfc822 format available.

Message #11 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>, bruce.stephens <at> isode.com
Cc: 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and  conditional statements is wrong
Date: Fri, 02 Dec 2022 06:42:03 +0100

On 2 December 2022 06:13:42 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>
>Bruce Stephens <bruce.stephens <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 08:40:01 GMT) Full text and rfc822 format available.

Message #14 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: bruce.stephens <at> isode.com, casouri <at> gmail.com, 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50;
 tree-sitter indentation in some loops and conditional statements is
 wrong
Date: Fri, 02 Dec 2022 10:39:21 +0200
> Cc: 59686 <at> 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 <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 09:21:02 GMT) Full text and rfc822 format available.

Message #17 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: bruce.stephens <at> isode.com, casouri <at> gmail.com, 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and
 conditional statements is wrong
Date: Fri, 02 Dec 2022 10:20:39 +0100
Eli Zaretskii <eliz <at> gnu.org> writes:

>> Cc: 59686 <at> 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 <at> 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?




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 10:47:01 GMT) Full text and rfc822 format available.

Message #20 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Bruce Stephens <bruce.stephens <at> isode.com>
To: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>
Cc: casouri <at> gmail.com, 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and
 conditional statements is wrong
Date: Fri, 2 Dec 2022 10:46:34 +0000
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.






Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Fri, 02 Dec 2022 11:52:01 GMT) Full text and rfc822 format available.

Message #23 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Bruce Stephens <bruce.stephens <at> isode.com>, Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and
 conditional statements is wrong
Date: Fri, 02 Dec 2022 12:51:14 +0100
Bruce Stephens <bruce.stephens <at> 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





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Sat, 03 Dec 2022 10:49:01 GMT) Full text and rfc822 format available.

Message #26 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: bruce.stephens <at> isode.com, Eli Zaretskii <eliz <at> gnu.org>,
 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and 
 conditional statements is wrong
Date: Sat, 3 Dec 2022 02:48:34 -0800
Theodor Thornhill <theo <at> thornhill.no> writes:

> Bruce Stephens <bruce.stephens <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Sat, 03 Dec 2022 11:10:01 GMT) Full text and rfc822 format available.

Message #29 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Theodor Thornhill <theo <at> thornhill.no>
To: Yuan Fu <casouri <at> gmail.com>
Cc: bruce.stephens <at> isode.com, Eli Zaretskii <eliz <at> gnu.org>,
 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and  conditional statements is wrong
Date: Sat, 03 Dec 2022 12:08:59 +0100

On 3 December 2022 11:48:34 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>
>Theodor Thornhill <theo <at> thornhill.no> writes:
>
>> Bruce Stephens <bruce.stephens <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Sat, 03 Dec 2022 11:20:02 GMT) Full text and rfc822 format available.

Message #32 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: bruce.stephens <at> isode.com, Eli Zaretskii <eliz <at> gnu.org>,
 59686 <at> debbugs.gnu.org
Subject: Re: bug#59686: 30.0.50; tree-sitter indentation in some loops and 
 conditional statements is wrong
Date: Sat, 3 Dec 2022 03:19:45 -0800

> On Dec 3, 2022, at 3:08 AM, Theodor Thornhill <theo <at> thornhill.no> wrote:
> 
> 
> 
> On 3 December 2022 11:48:34 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>> 
>> Theodor Thornhill <theo <at> thornhill.no> writes:
>> 
>>> Bruce Stephens <bruce.stephens <at> 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





Merged 59686 60496. Request was from Yuan Fu <casouri <at> gmail.com> to control <at> debbugs.gnu.org. (Sat, 07 Jan 2023 23:06:02 GMT) Full text and rfc822 format available.

Merged 59686 60398 60496. Request was from Yuan Fu <casouri <at> gmail.com> to control <at> debbugs.gnu.org. (Sun, 08 Jan 2023 00:16:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#59686; Package emacs. (Sun, 15 Jan 2023 09:24:02 GMT) Full text and rfc822 format available.

Message #39 received at 59686 <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Theodor Thornhill <theo <at> thornhill.no>
Cc: bruce.stephens <at> isode.com, eliz <at> gnu.org, 60496 <at> debbugs.gnu.org,
 59686 <at> debbugs.gnu.org
Subject: Re: bug#60496: 29.0.60; c-ts-mode: Broken indentation with linux 
 style conditionals
Date: Sun, 15 Jan 2023 01:23:45 -0800
Yuan Fu <casouri <at> gmail.com> writes:

>> On Dec 3, 2022, at 3:08 AM, Theodor Thornhill <theo <at> thornhill.no> wrote:
>> 
>> 
>> 
>> On 3 December 2022 11:48:34 CET, Yuan Fu <casouri <at> gmail.com> wrote:
>>> 
>>> Theodor Thornhill <theo <at> thornhill.no> writes:
>>> 
>>>> Bruce Stephens <bruce.stephens <at> 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?

I just pushed a change (189d976dbae) that I think fixes this kind of
problems. Instead of trying to figure out the right anchor, we simply
count the number of {} blocks between the node at point and the root
node, and use that number (multiplied by c-ts-mode-indent-offset) as the
indentation.

If you think about it, both

for (a;b;c)
{
  |
}

and

for (a;
     b;
     c)
{
  |     
}

are one block-level deep.  So multi-line conditions is not an issue
anymore.

And

int main()
{
  if ()
    {
      |
    }
}

is 2 block-level deep, etc.

Yuan




This bug report was last modified 2 years and 207 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.