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

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

[Mohammed Sadiq]

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

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

[Yuan Fu]

Mohammed Sadiq <sadiq@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

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

[Mohammed Sadiq]

On 2023-01-03 03:59, Yuan Fu wrote:
> Mohammed Sadiq <sadiq@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 ();
   }
}

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

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Mohammed Sadiq <sadiq@sadiqpk.org> writes:

> On 2023-01-03 03:59, Yuan Fu wrote:
>> Mohammed Sadiq <sadiq@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

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

[Yuan Fu]

Yuan Fu <casouri@gmail.com> writes:

>> 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?

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