bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Mohammed Sadiq]

The following code has broken indentation with c-ts-mode:

   while (true)
     if (true) {
     puts ("Hello");
   }

   if (true)
     if (true)
       {
       puts ("World");
     }

For which, I expect to get the following indentation (with default 
config):

   while (true)
     if (true) {
       puts ("Hello");
     }

   if (true)
     if (true)
       {
         puts ("World");
       }



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

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

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

Mohammed Sadiq <sadiq@sadiqpk.org> writes:

> The following code has broken indentation with c-ts-mode:
>
>   while (true)
>     if (true) {
>     puts ("Hello");
>   }
>
>   if (true)
>     if (true)
>       {
>       puts ("World");
>     }
>
> For which, I expect to get the following indentation (with default config):
>
>   while (true)
>     if (true) {
>       puts ("Hello");
>     }
>
>   if (true)
>     if (true)
>       {
>         puts ("World");
>       }
>
>

Thanks, Mohammed!

Yuan, I'm not sure the new brace counting method is a good enough
improvement just yet considering its complexity and the number of issues
we are seeing.  Do you see any simple fixes for these issues?

I can try to debug this this weekend, but not sure I really have time to
fix it.  All of these issues work fine in js-ts-mode or java-ts-mode, if
I'm not mistaken.

Theo

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Yuan Fu]

Theodor Thornhill <theo@thornhill.no> writes:

> Mohammed Sadiq <sadiq@sadiqpk.org> writes:
>
>> The following code has broken indentation with c-ts-mode:
>>
>>   while (true)
>>     if (true) {
>>     puts ("Hello");
>>   }
>>
>>   if (true)
>>     if (true)
>>       {
>>       puts ("World");
>>     }
>>
>> For which, I expect to get the following indentation (with default config):
>>
>>   while (true)
>>     if (true) {
>>       puts ("Hello");
>>     }
>>
>>   if (true)
>>     if (true)
>>       {
>>         puts ("World");
>>       }
>>
>>
>
> Thanks, Mohammed!
>
> Yuan, I'm not sure the new brace counting method is a good enough
> improvement just yet considering its complexity and the number of issues
> we are seeing.  Do you see any simple fixes for these issues?

I fixed it.

> I can try to debug this this weekend, but not sure I really have time to
> fix it.  All of these issues work fine in js-ts-mode or java-ts-mode, if
> I'm not mistaken.

Yes, because js and java doesn’t allow omitting brackets. So we only
need to add a special-case for C/C++, so it isn’t too bad.

Yuan

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

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

Yuan Fu <casouri@gmail.com> writes:

> Theodor Thornhill <theo@thornhill.no> writes:
>
>> Mohammed Sadiq <sadiq@sadiqpk.org> writes:
>>
>>> The following code has broken indentation with c-ts-mode:
>>>
>>>   while (true)
>>>     if (true) {
>>>     puts ("Hello");
>>>   }
>>>
>>>   if (true)
>>>     if (true)
>>>       {
>>>       puts ("World");
>>>     }
>>>
>>> For which, I expect to get the following indentation (with default config):
>>>
>>>   while (true)
>>>     if (true) {
>>>       puts ("Hello");
>>>     }
>>>
>>>   if (true)
>>>     if (true)
>>>       {
>>>         puts ("World");
>>>       }
>>>
>>>
>>
>> Thanks, Mohammed!
>>
>> Yuan, I'm not sure the new brace counting method is a good enough
>> improvement just yet considering its complexity and the number of issues
>> we are seeing.  Do you see any simple fixes for these issues?
>
> I fixed it.
>
>> I can try to debug this this weekend, but not sure I really have time to
>> fix it.  All of these issues work fine in js-ts-mode or java-ts-mode, if
>> I'm not mistaken.
>
> Yes, because js and java doesn’t allow omitting brackets. So we only
> need to add a special-case for C/C++, so it isn’t too bad.
>
> Yuan

They allow that just fine, don't they? 

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Yuan Fu]

Theodor Thornhill <theo@thornhill.no> writes:

> Yuan Fu <casouri@gmail.com> writes:
>
>> Theodor Thornhill <theo@thornhill.no> writes:
>>
>>> Mohammed Sadiq <sadiq@sadiqpk.org> writes:
>>>
>>>> The following code has broken indentation with c-ts-mode:
>>>>
>>>>   while (true)
>>>>     if (true) {
>>>>     puts ("Hello");
>>>>   }
>>>>
>>>>   if (true)
>>>>     if (true)
>>>>       {
>>>>       puts ("World");
>>>>     }
>>>>
>>>> For which, I expect to get the following indentation (with default config):
>>>>
>>>>   while (true)
>>>>     if (true) {
>>>>       puts ("Hello");
>>>>     }
>>>>
>>>>   if (true)
>>>>     if (true)
>>>>       {
>>>>         puts ("World");
>>>>       }
>>>>
>>>>
>>>
>>> Thanks, Mohammed!
>>>
>>> Yuan, I'm not sure the new brace counting method is a good enough
>>> improvement just yet considering its complexity and the number of issues
>>> we are seeing.  Do you see any simple fixes for these issues?
>>
>> I fixed it.
>>
>>> I can try to debug this this weekend, but not sure I really have time to
>>> fix it.  All of these issues work fine in js-ts-mode or java-ts-mode, if
>>> I'm not mistaken.
>>
>> Yes, because js and java doesn’t allow omitting brackets. So we only
>> need to add a special-case for C/C++, so it isn’t too bad.
>>
>> Yuan
>
> They allow that just fine, don't they? 

You are right. I made the fix general and moved everything to
c-ts-common.el. The next step is to make other modes use it.

Yuan

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

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

On 29 January 2023 09:22:26 CET, Yuan Fu <casouri@gmail.com> wrote:
>
>Theodor Thornhill <theo@thornhill.no> writes:
>
>> Yuan Fu <casouri@gmail.com> writes:
>>
>>> Theodor Thornhill <theo@thornhill.no> writes:
>>>
>>>> Mohammed Sadiq <sadiq@sadiqpk.org> writes:
>>>>
>>>>> The following code has broken indentation with c-ts-mode:
>>>>>
>>>>>   while (true)
>>>>>     if (true) {
>>>>>     puts ("Hello");
>>>>>   }
>>>>>
>>>>>   if (true)
>>>>>     if (true)
>>>>>       {
>>>>>       puts ("World");
>>>>>     }
>>>>>
>>>>> For which, I expect to get the following indentation (with default config):
>>>>>
>>>>>   while (true)
>>>>>     if (true) {
>>>>>       puts ("Hello");
>>>>>     }
>>>>>
>>>>>   if (true)
>>>>>     if (true)
>>>>>       {
>>>>>         puts ("World");
>>>>>       }
>>>>>
>>>>>
>>>>
>>>> Thanks, Mohammed!
>>>>
>>>> Yuan, I'm not sure the new brace counting method is a good enough
>>>> improvement just yet considering its complexity and the number of issues
>>>> we are seeing.  Do you see any simple fixes for these issues?
>>>
>>> I fixed it.
>>>
>>>> I can try to debug this this weekend, but not sure I really have time to
>>>> fix it.  All of these issues work fine in js-ts-mode or java-ts-mode, if
>>>> I'm not mistaken.
>>>
>>> Yes, because js and java doesn’t allow omitting brackets. So we only
>>> need to add a special-case for C/C++, so it isn’t too bad.
>>>
>>> Yuan
>>
>> They allow that just fine, don't they? 
>
>You are right. I made the fix general and moved everything to
>c-ts-common.el. The next step is to make other modes use it.
>
>Yuan

Nice!

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Knut Anders Hatlen]

Hi,

I'm still seeing odd indentation for conditions without braces. For
example, invoking c-ts-mode-indent-defun (C-c C-q) with the point inside
this function ...

int f(int x) {
if (x > 1)
return x;
else
return x + 1;
}

... results in the following indentation with the default GNU style:

int f(int x) {
  if (x > 1)
return x;
  else
return x + 1;
}

I would have expected this:

int f(int x) {
  if (x > 1)
    return x;
  else
    return x + 1;
}

Is something more needed for this bug, or is the problem I'm seeing a
different bug?

-- 
Knut Anders

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Yuan Fu]

Knut Anders Hatlen <kahatlen@gmail.com> writes:

> Hi,
>
> I'm still seeing odd indentation for conditions without braces. For
> example, invoking c-ts-mode-indent-defun (C-c C-q) with the point inside
> this function ...
>
> int f(int x) {
> if (x > 1)
> return x;
> else
> return x + 1;
> }
>
> ... results in the following indentation with the default GNU style:
>
> int f(int x) {
>   if (x > 1)
> return x;
>   else
> return x + 1;
> }
>
> I would have expected this:
>
> int f(int x) {
>   if (x > 1)
>     return x;
>   else
>     return x + 1;
> }
>
> Is something more needed for this bug, or is the problem I'm seeing a
> different bug?

My bad, I made some changes and didn’t push them, and I’m sitting here
wondering why everyone is seeing problems... Everything should indent
fine now.

Yuan

bug#61026: 29.0.60; c-ts-mode: Broken indentation for loops and conditions without brace

[Knut Anders Hatlen]

[-- Attachment #1: Type: text/plain, Size: 1338 bytes --]

tir. 7. feb. 2023 kl. 03:30 skrev Yuan Fu <casouri@gmail.com>:

>
> Knut Anders Hatlen <kahatlen@gmail.com> writes:
>
> > Hi,
> >
> > I'm still seeing odd indentation for conditions without braces. For
> > example, invoking c-ts-mode-indent-defun (C-c C-q) with the point inside
> > this function ...
> >
> > int f(int x) {
> > if (x > 1)
> > return x;
> > else
> > return x + 1;
> > }
> >
> > ... results in the following indentation with the default GNU style:
> >
> > int f(int x) {
> >   if (x > 1)
> > return x;
> >   else
> > return x + 1;
> > }
> >
> > I would have expected this:
> >
> > int f(int x) {
> >   if (x > 1)
> >     return x;
> >   else
> >     return x + 1;
> > }
> >
> > Is something more needed for this bug, or is the problem I'm seeing a
> > different bug?
>
> My bad, I made some changes and didn’t push them, and I’m sitting here
> wondering why everyone is seeing problems... Everything should indent
> fine now.
>
> Yuan
>

Thanks, Yuan!

It now seems to work fine with the GNU style, K&R style and Linux style.
With BSD style, however, it ends up like this:

int f(int x) {
  if (x > 1)
  return x;
  else
  return x + 1;
}

I assume the return statements should have been indented one more level
here too?

Best regards,

-- 
Knut Anders

[-- Attachment #2: Type: text/html, Size: 2083 bytes --]