GNU bug report logs - #60496
29.0.60; c-ts-mode: Broken indentation with linux style conditionals

Previous Next

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

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

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

From: Mohammed Sadiq <sadiq <at> sadiqpk.org>
To: bug-gnu-emacs <at> gnu.org
Subject: 29.0.60; c-ts-mode: Broken indentation with linux style conditionals
Date: Mon, 02 Jan 2023 21:27:25 +0530

Indentation of the following C code with linux style is broken:

int main (void)
{
  if (a) {
    func_a ();
  } else if (b) {
	   func_b ();
	 } else {
	   func_c ();
	 }
}



In GNU Emacs 29.0.60 (build 5, x86_64-pc-linux-gnu, GTK+ Version
 3.24.35, cairo version 1.16.0) of 2023-01-02 built on purism
Repository revision: 2569ede9c496bb060e0b88428cb541088aaba1f9
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 
11.0.12101005
System Description: Debian GNU/Linux bookworm/sid

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

From: Yuan Fu <casouri <at> gmail.com>
To: Mohammed Sadiq <sadiq <at> sadiqpk.org>
Cc: 60496 <at> debbugs.gnu.org
Subject: Re: bug#60496: 29.0.60; c-ts-mode: Broken indentation with linux 
 style conditionals
Date: Mon, 2 Jan 2023 14:29:02 -0800

Mohammed Sadiq <sadiq <at> sadiqpk.org> writes:

> Indentation of the following C code with linux style is broken:
>
> int main (void)
> {
>   if (a) {
>     func_a ();
>   } else if (b) {
> 	   func_b ();
> 	 } else {
> 	   func_c ();
> 	 }
> }
>

Thanks. I’m not familiar with the linux style, is the problem with the
closing brackets? Could you give me a correct example so I know exactly
what’s wrong?

Yuan

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

From: Mohammed Sadiq <sadiq <at> sadiqpk.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: 60496 <at> debbugs.gnu.org
Subject: Re: bug#60496: 29.0.60; c-ts-mode: Broken indentation with linux
 style conditionals
Date: Tue, 03 Jan 2023 14:39:52 +0530

On 2023-01-03 03:59, Yuan Fu wrote:
> Mohammed Sadiq <sadiq <at> sadiqpk.org> writes:
> 
>> Indentation of the following C code with linux style is broken:
>> 
>> int main (void)
>> {
>>   if (a) {
>>     func_a ();
>>   } else if (b) {
>> 	   func_b ();
>> 	 } else {
>> 	   func_c ();
>> 	 }
>> }
>> 
> 
> Thanks. I’m not familiar with the linux style, is the problem with the
> closing brackets? Could you give me a correct example so I know exactly
> what’s wrong?
> 
> Yuan


The expected indentation style for the given code (this one is provided
by c-mode), with the default indentation values:

int
main (void)
{
  if (a) {
    func_a ();
  } else if (b) {
    func_b ();
  } else {
    func_c ();
  }
}

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

From: Theodor Thornhill <theo <at> thornhill.no>
To: Mohammed Sadiq <sadiq <at> sadiqpk.org>
Cc: Yuan Fu <casouri <at> gmail.com>, 60496 <at> debbugs.gnu.org
Subject: Re: bug#60496: 29.0.60; c-ts-mode: Broken indentation with linux
 style conditionals
Date: Sat, 07 Jan 2023 13:18:38 +0100

Mohammed Sadiq <sadiq <at> sadiqpk.org> writes:

> On 2023-01-03 03:59, Yuan Fu wrote:
>> Mohammed Sadiq <sadiq <at> sadiqpk.org> writes:
>> 
>>> Indentation of the following C code with linux style is broken:
>>> int main (void)
>>> {
>>>   if (a) {
>>>     func_a ();
>>>   } else if (b) {
>>> 	   func_b ();
>>> 	 } else {
>>> 	   func_c ();
>>> 	 }
>>> }
>>> 
>> Thanks. I’m not familiar with the linux style, is the problem with the
>> closing brackets? Could you give me a correct example so I know exactly
>> what’s wrong?
>> Yuan
>
>
> The expected indentation style for the given code (this one is provided
> by c-mode), with the default indentation values:
>
> int
> main (void)
> {
>   if (a) {
>     func_a ();
>   } else if (b) {
>     func_b ();
>   } else {
>     func_c ();
>   }
> }

Yuan, this is a regression after the
c-ts-mode--bracket-children-anchor. IIRC this was one of the cases that
gave me headeaches.  IIUC now the grand-parent cannot help because we
could have infinite else ifs, right?  And the c grammar nests these
"alternative:" nodes deeper and deeper.

Reverting to

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index dec866f762f..6c8c671550a 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -118,7 +118,7 @@ c-ts-mode--indent-styles
          `(((parent-is "translation_unit") parent-bol 0)
            ((node-is ")") parent 1)
            ((node-is "]") parent-bol 0)
-           ((node-is "}") c-ts-mode--bracket-children-anchor 0)
+           ((node-is "}") parent-bol 0)
            ((node-is "else") parent-bol 0)
            ((node-is "case") parent-bol 0)
            ((node-is "preproc_arg") no-indent)
@@ -134,7 +134,7 @@ c-ts-mode--indent-styles
            ((match "preproc_function_def" "compound_statement") point-min 0)
            ((match "preproc_call" "compound_statement") point-min 0)
            ((parent-is "compound_statement")
-            c-ts-mode--bracket-children-anchor c-ts-mode-indent-offset)
+            parent-bol c-ts-mode-indent-offset)
            ((parent-is "function_definition") parent-bol 0)
            ((parent-is "conditional_expression") first-sibling 0)
            ((parent-is "assignment_expression") parent-bol c-ts-mode-indent-offset)
@@ -155,7 +155,7 @@ c-ts-mode--indent-styles
                '(((node-is "access_specifier") parent-bol 0)))
            ((parent-is "field_declaration_list") parent-bol c-ts-mode-indent-offset)
            ((parent-is "initializer_list") parent-bol c-ts-mode-indent-offset)
-           ((parent-is "if_statement") parent-bol c-ts-mode-indent-offset)
+           ((parent-is "if_statement") c-ts-mode--bracket-children-anchor c-ts-mode-indent-offset)
            ((parent-is "for_statement") parent-bol c-ts-mode-indent-offset)
            ((parent-is "while_statement") parent-bol c-ts-mode-indent-offset)
            ((parent-is "switch_statement") parent-bol c-ts-mode-indent-offset)

Fixes this issue, but then we again have the problem with code such as:

```
int
main (void)
{
  for(;
    ;) {
      // wrong...
    }
}
```

Theo

Message #21 received at 60496 <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

Previous Next

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