GNU bug report logs - #74507
[PATCH] Indent compounds c-ts-mode when { is not BOL

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 74507 in the body.
You can then email your comments to 74507 AT debbugs.gnu.org in the normal way.

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

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 24 Nov 2024 10:15:12 +0100

[Message part 1 (text/plain, inline)]

Tags: patch

Tags: patch


I found that the coumpounded statements are anchored to the wrong object
when 1. the compound is not at beginning-of-line and 2. it is not
preceeded by a construct such as fndecl/for/if/while/etc., which differs
from how c-mode indents it.

Non-BOL compound statements is actually quite common with macros and
testing frameworks (see the test case). For example, you want this to
indent:

TEST_CASE(1) {
    assert (...);
}

The heuristic is quite course - it simply checks if the grandparent is
function-definition or not, which is really if this node is a top-level
sibling in the function body.  If that is the case, the old rules should
apply and standalone-parent should be the guide; otherwise, it should be
the parent-BOL. This feels a bit shaky but does seem to work well for
the test cases, and can be refined in the future.

[0001-Indent-compounds-c-ts-mode-when-is-not-BOL.patch (text/patch, attachment)]

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

From: Yuan Fu <casouri <at> gmail.com>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Thu, 28 Nov 2024 20:56:26 -0800

> On Nov 24, 2024, at 1:15 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
> 
> Tags: patch
> 
> Tags: patch
> 
> 
> I found that the coumpounded statements are anchored to the wrong object
> when 1. the compound is not at beginning-of-line and 2. it is not
> preceeded by a construct such as fndecl/for/if/while/etc., which differs
> from how c-mode indents it.
> 
> Non-BOL compound statements is actually quite common with macros and
> testing frameworks (see the test case). For example, you want this to
> indent:
> 
> TEST_CASE(1) {
>    assert (...);
> }
> 
> The heuristic is quite course - it simply checks if the grandparent is
> function-definition or not, which is really if this node is a top-level
> sibling in the function body.  If that is the case, the old rules should
> apply and standalone-parent should be the guide; otherwise, it should be
> the parent-BOL. This feels a bit shaky but does seem to work well for
> the test cases, and can be refined in the future.
> 
> <0001-Indent-compounds-c-ts-mode-when-is-not-BOL.patch>

Thank you very much! Especially for the test case ;-) What’s the progress of your copyright assignment? Has it completed?

I’m working on refactoring the indentation rules for c-ts-mode. Let me see if I can integrate this situation into the new rules I’m writing.

Yuan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Yuan Fu <casouri <at> gmail.com>
Cc: theo <at> thornhill.no, j <at> lambda.is, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Fri, 29 Nov 2024 09:54:56 +0200

> Cc: theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
> From: Yuan Fu <casouri <at> gmail.com>
> Date: Thu, 28 Nov 2024 20:56:26 -0800
> 
> Thank you very much! Especially for the test case ;-) What’s the progress of your copyright assignment? Has it completed?

Jørgen's assignment is on file, so we are good in that department.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: casouri <at> gmail.com, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Fri, 29 Nov 2024 09:57:03 +0200

> Cc: theo <at> thornhill.no,casouri <at> gmail.com
> From: Jørgen Kvalsvik <j <at> lambda.is>
> Date: Sun, 24 Nov 2024 10:15:12 +0100
> 
> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
> New function.
> (c-ts-mode--indent-styles): Use it.
> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
> statement test.

Please make sure the lines here are not too long (see CONTRIBUTE for
details).

> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
> +  "Matches when PARENT is not the top level compound statement,
> +the {} that immediately follows the signature."

The first line of a doc string should be a single complete sentence.

Thanks.

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: Yuan Fu <casouri <at> gmail.com>
Cc: theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Fri, 29 Nov 2024 10:04:36 +0100

On 11/29/24 05:56, Yuan Fu wrote:
> 
> 
>> On Nov 24, 2024, at 1:15 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>
>> Tags: patch
>>
>> Tags: patch
>>
>>
>> I found that the coumpounded statements are anchored to the wrong object
>> when 1. the compound is not at beginning-of-line and 2. it is not
>> preceeded by a construct such as fndecl/for/if/while/etc., which differs
>> from how c-mode indents it.
>>
>> Non-BOL compound statements is actually quite common with macros and
>> testing frameworks (see the test case). For example, you want this to
>> indent:
>>
>> TEST_CASE(1) {
>>     assert (...);
>> }
>>
>> The heuristic is quite course - it simply checks if the grandparent is
>> function-definition or not, which is really if this node is a top-level
>> sibling in the function body.  If that is the case, the old rules should
>> apply and standalone-parent should be the guide; otherwise, it should be
>> the parent-BOL. This feels a bit shaky but does seem to work well for
>> the test cases, and can be refined in the future.
>>
>> <0001-Indent-compounds-c-ts-mode-when-is-not-BOL.patch>
> 
> Thank you very much! Especially for the test case ;-) What’s the progress of your copyright assignment? Has it completed?

As Eli says, I have submitted the paperwork.

> 
> I’m working on refactoring the indentation rules for c-ts-mode. Let me see if I can integrate this situation into the new rules I’m writing.
> 
> Yuan

Splendid, thanks.

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Fri, 29 Nov 2024 10:05:11 +0100

On 11/29/24 08:57, Eli Zaretskii wrote:
>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>> From: Jørgen Kvalsvik <j <at> lambda.is>
>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>
>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>> New function.
>> (c-ts-mode--indent-styles): Use it.
>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>> statement test.
> 
> Please make sure the lines here are not too long (see CONTRIBUTE for
> details).
> 
>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>> +  "Matches when PARENT is not the top level compound statement,
>> +the {} that immediately follows the signature."
> 
> The first line of a doc string should be a single complete sentence.
> 
> Thanks.

Ok. Yuan, would you like me to submit a revision?

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

From: Yuan Fu <casouri <at> gmail.com>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Fri, 29 Nov 2024 16:16:02 -0800

> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
> 
> On 11/29/24 08:57, Eli Zaretskii wrote:
>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>> 
>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>> New function.
>>> (c-ts-mode--indent-styles): Use it.
>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>> statement test.
>> Please make sure the lines here are not too long (see CONTRIBUTE for
>> details).
>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>> +  "Matches when PARENT is not the top level compound statement,
>>> +the {} that immediately follows the signature."
>> The first line of a doc string should be a single complete sentence.
>> Thanks.
> 
> Ok. Yuan, would you like me to submit a revision?

Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!

Yuan

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: 74507 <at> debbugs.gnu.org
Subject: [PATCH] Indent compounds in c-ts-mode when { is not BOL
Date: Sat, 30 Nov 2024 16:39:48 +0100

[Message part 1 (text/plain, inline)]

[0001-Indent-compounds-in-c-ts-mode-when-is-not-BOL.patch (text/patch, attachment)]

[Message part 3 (text/plain, inline)]

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sat, 30 Nov 2024 21:49:47 +0100

On 11/30/24 01:16, Yuan Fu wrote:
> 
> 
>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>
>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>
>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>> New function.
>>>> (c-ts-mode--indent-styles): Use it.
>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>> statement test.
>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>> details).
>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>> +  "Matches when PARENT is not the top level compound statement,
>>>> +the {} that immediately follows the signature."
>>> The first line of a doc string should be a single complete sentence.
>>> Thanks.
>>
>> Ok. Yuan, would you like me to submit a revision?
> 
> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
> 
> Yuan

Certainly - I posted it on the bug tracker.

Thanks,
Jørgen

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

From: Yuan Fu <casouri <at> gmail.com>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 1 Dec 2024 01:25:53 -0800

> On Nov 30, 2024, at 12:49 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
> 
> On 11/30/24 01:16, Yuan Fu wrote:
>>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>> 
>>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>> 
>>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>>> New function.
>>>>> (c-ts-mode--indent-styles): Use it.
>>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>>> statement test.
>>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>>> details).
>>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>>> +  "Matches when PARENT is not the top level compound statement,
>>>>> +the {} that immediately follows the signature."
>>>> The first line of a doc string should be a single complete sentence.
>>>> Thanks.
>>> 
>>> Ok. Yuan, would you like me to submit a revision?
>> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
>> Yuan
> 
> Certainly - I posted it on the bug tracker.

Thanks Jørgen. What did you use to generate the patch? For some reason I can’t apply it. My git skill isn’t that great so it could be my problem. If you can apply it fine maybe you can share the command you used?

BTW, the commit title is missing. When you add the title, you can also add the bug number. For example:

Improve c-ts-mode indentation for macros (bug#74507)

Yuan

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 1 Dec 2024 10:51:38 +0100

[Message part 1 (text/plain, inline)]

On 12/1/24 10:25, Yuan Fu wrote:
> 
> 
>> On Nov 30, 2024, at 12:49 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>
>> On 11/30/24 01:16, Yuan Fu wrote:
>>>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>
>>>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>>>
>>>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>>>> New function.
>>>>>> (c-ts-mode--indent-styles): Use it.
>>>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>>>> statement test.
>>>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>>>> details).
>>>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>>>> +  "Matches when PARENT is not the top level compound statement,
>>>>>> +the {} that immediately follows the signature."
>>>>> The first line of a doc string should be a single complete sentence.
>>>>> Thanks.
>>>>
>>>> Ok. Yuan, would you like me to submit a revision?
>>> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
>>> Yuan
>>
>> Certainly - I posted it on the bug tracker.
> 
> Thanks Jørgen. What did you use to generate the patch? For some reason I can’t apply it. My git skill isn’t that great so it could be my problem. If you can apply it fine maybe you can share the command you used?

I used git format-patch HEAD~1, and I just tested applying it to master 
with `git am 0001-Indent-compounds-in-c-ts-mode-when-is-not-BOL.patch' 
which worked.

> 
> BTW, the commit title is missing. When you add the title, you can also add the bug number. For example:
> 
> Improve c-ts-mode indentation for macros (bug#74507)
> 
> Yuan

Sure. I've attached a new patch with the bug in it. I tested it and it 
applies cleanly with `git am 
0001-Improve-c-ts-mode-compound-indents-bug-74507.patch'

[0001-Improve-c-ts-mode-compound-indents-bug-74507.patch (text/x-patch, attachment)]

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

From: Yuan Fu <casouri <at> gmail.com>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 1 Dec 2024 13:48:24 -0800

> On Dec 1, 2024, at 1:51 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
> 
> On 12/1/24 10:25, Yuan Fu wrote:
>>> On Nov 30, 2024, at 12:49 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>> 
>>> On 11/30/24 01:16, Yuan Fu wrote:
>>>>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>> 
>>>>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>>>> 
>>>>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>>>>> New function.
>>>>>>> (c-ts-mode--indent-styles): Use it.
>>>>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>>>>> statement test.
>>>>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>>>>> details).
>>>>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>>>>> +  "Matches when PARENT is not the top level compound statement,
>>>>>>> +the {} that immediately follows the signature."
>>>>>> The first line of a doc string should be a single complete sentence.
>>>>>> Thanks.
>>>>> 
>>>>> Ok. Yuan, would you like me to submit a revision?
>>>> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
>>>> Yuan
>>> 
>>> Certainly - I posted it on the bug tracker.
>> Thanks Jørgen. What did you use to generate the patch? For some reason I can’t apply it. My git skill isn’t that great so it could be my problem. If you can apply it fine maybe you can share the command you used?
> 
> I used git format-patch HEAD~1, and I just tested applying it to master with `git am 0001-Indent-compounds-in-c-ts-mode-when-is-not-BOL.patch' which worked.
> 
>> BTW, the commit title is missing. When you add the title, you can also add the bug number. For example:
>> Improve c-ts-mode indentation for macros (bug#74507)
>> Yuan
> 
> Sure. I've attached a new patch with the bug in it. I tested it and it applies cleanly with `git am 0001-Improve-c-ts-mode-compound-indents-bug-74507.patch'
> <0001-Improve-c-ts-mode-compound-indents-bug-74507.patch>

Thanks. This patch applied without a problem. 

I reworked your heuristic into a function rule for better organization, and changed the condition slightly:

(defun c-ts-mode--macro-heuristic-rules (node parent &rest _)
  "Heuristic indent rule for control flow macros.

Eg,

    #define IOTA(var, n) for (int var = 0; var != (n); ++var)

    int main()
    {
      IOTA (v, 10) {
        printf(\"%d \", v);   <-- Here we want to indent
        counter++;            <-- Use baseline rule to align
    }                             to prev sibling

Checked by \"Compound Statement after code (Bug#74507)\" test.

NODE and PARENT are the same as other indent rules."
  (when (and (treesit-node-match-p parent "compound_statement")
             (treesit-node-match-p (treesit-node-prev-sibling parent)
                                   "expression_statement"))
    (let ((parent-bol
           (lambda () (save-excursion
                        (goto-char (treesit-node-start parent))
                        (back-to-indentation)
                        (point)))))
      (cond
       ;; Closing brace.
       ((treesit-node-match-p node "}")
        (cons (funcall parent-bol) 0))
       ;; First sibling.
       ((treesit-node-eq (treesit-node-child parent 0 'named) node)
        (cons (funcall parent-bol)
              c-ts-mode-indent-offset))))))

Instead of checking whether PARENT is top-level compound_statement, I make it check whether the previous sibling is an expression_statement. This way the heuristic works when the macro isn’t at top-level too. Here’s the updated test:

#define IOTA(var, n) for (int var = 0; var != (n); ++var)
int main()
{
  IOTA (v, 10) {
    printf("%d ", v);
  }

  const char *msg = "Hello, world!"; {
    puts("Hello, world!");
  }

  for (int i = 0;
       i < 10;
       i++) {
    IOTA (v, 10) {
      printf("%d ", v);
    }
  }

  {
    IOTA (v, 10) {
      printf("%d ", v);
    }
  }
}

The only test case that doesn’t pass right now is this one:

  const char *msg = "Hello, world!"; {
    puts("Hello, world!");
  }

Is this a real use-case?

Yuan

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

From: Jørgen Kvalsvik <j <at> lambda.is>
To: Yuan Fu <casouri <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, theo <at> thornhill.no, 74507 <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 1 Dec 2024 23:46:06 +0100

On 12/1/24 22:48, Yuan Fu wrote:
> 
> 
>> On Dec 1, 2024, at 1:51 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>
>> On 12/1/24 10:25, Yuan Fu wrote:
>>>> On Nov 30, 2024, at 12:49 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>
>>>> On 11/30/24 01:16, Yuan Fu wrote:
>>>>>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>>>
>>>>>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>>>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>>>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>>>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>>>>>
>>>>>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>>>>>> New function.
>>>>>>>> (c-ts-mode--indent-styles): Use it.
>>>>>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>>>>>> statement test.
>>>>>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>>>>>> details).
>>>>>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>>>>>> +  "Matches when PARENT is not the top level compound statement,
>>>>>>>> +the {} that immediately follows the signature."
>>>>>>> The first line of a doc string should be a single complete sentence.
>>>>>>> Thanks.
>>>>>>
>>>>>> Ok. Yuan, would you like me to submit a revision?
>>>>> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
>>>>> Yuan
>>>>
>>>> Certainly - I posted it on the bug tracker.
>>> Thanks Jørgen. What did you use to generate the patch? For some reason I can’t apply it. My git skill isn’t that great so it could be my problem. If you can apply it fine maybe you can share the command you used?
>>
>> I used git format-patch HEAD~1, and I just tested applying it to master with `git am 0001-Indent-compounds-in-c-ts-mode-when-is-not-BOL.patch' which worked.
>>
>>> BTW, the commit title is missing. When you add the title, you can also add the bug number. For example:
>>> Improve c-ts-mode indentation for macros (bug#74507)
>>> Yuan
>>
>> Sure. I've attached a new patch with the bug in it. I tested it and it applies cleanly with `git am 0001-Improve-c-ts-mode-compound-indents-bug-74507.patch'
>> <0001-Improve-c-ts-mode-compound-indents-bug-74507.patch>
> 
> Thanks. This patch applied without a problem.
> 
> I reworked your heuristic into a function rule for better organization, and changed the condition slightly:
> 
> (defun c-ts-mode--macro-heuristic-rules (node parent &rest _)
>    "Heuristic indent rule for control flow macros.
> 
> Eg,
> 
>      #define IOTA(var, n) for (int var = 0; var != (n); ++var)
> 
>      int main()
>      {
>        IOTA (v, 10) {
>          printf(\"%d \", v);   <-- Here we want to indent
>          counter++;            <-- Use baseline rule to align
>      }                             to prev sibling
> 
> Checked by \"Compound Statement after code (Bug#74507)\" test.
> 
> NODE and PARENT are the same as other indent rules."
>    (when (and (treesit-node-match-p parent "compound_statement")
>               (treesit-node-match-p (treesit-node-prev-sibling parent)
>                                     "expression_statement"))
>      (let ((parent-bol
>             (lambda () (save-excursion
>                          (goto-char (treesit-node-start parent))
>                          (back-to-indentation)
>                          (point)))))
>        (cond
>         ;; Closing brace.
>         ((treesit-node-match-p node "}")
>          (cons (funcall parent-bol) 0))
>         ;; First sibling.
>         ((treesit-node-eq (treesit-node-child parent 0 'named) node)
>          (cons (funcall parent-bol)
>                c-ts-mode-indent-offset))))))
> 
> Instead of checking whether PARENT is top-level compound_statement, I make it check whether the previous sibling is an expression_statement. This way the heuristic works when the macro isn’t at top-level too. Here’s the updated test:
> 
> #define IOTA(var, n) for (int var = 0; var != (n); ++var)
> int main()
> {
>    IOTA (v, 10) {
>      printf("%d ", v);
>    }
> 
>    const char *msg = "Hello, world!"; {
>      puts("Hello, world!");
>    }
> 
>    for (int i = 0;
>         i < 10;
>         i++) {
>      IOTA (v, 10) {
>        printf("%d ", v);
>      }
>    }
> 
>    {
>      IOTA (v, 10) {
>        printf("%d ", v);
>      }
>    }
> }
> 
> The only test case that doesn’t pass right now is this one:
> 
>    const char *msg = "Hello, world!"; {
>      puts("Hello, world!");
>    }
> 
> Is this a real use-case?
> 
> Yuan

Maybe - I included it to show that the problem was not tied to it being 
a macro, but rather { not being the first thing on the line. It would be 
nice to support it, and c-mode also indents it properly.

That being said, it is probably rare enough in practice to prio it less.

Thanks,
Jørgen

Message #46 received at 74507-done <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Jørgen Kvalsvik <j <at> lambda.is>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Theodor Thornhill <theo <at> thornhill.no>,
 74507-done <at> debbugs.gnu.org
Subject: Re: bug#74507: [PATCH] Indent compounds c-ts-mode when { is not BOL
Date: Sun, 1 Dec 2024 18:13:19 -0800

> On Dec 1, 2024, at 2:46 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
> 
> On 12/1/24 22:48, Yuan Fu wrote:
>>> On Dec 1, 2024, at 1:51 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>> 
>>> On 12/1/24 10:25, Yuan Fu wrote:
>>>>> On Nov 30, 2024, at 12:49 PM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>> 
>>>>> On 11/30/24 01:16, Yuan Fu wrote:
>>>>>>> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j <at> lambda.is> wrote:
>>>>>>> 
>>>>>>> On 11/29/24 08:57, Eli Zaretskii wrote:
>>>>>>>>> Cc: theo <at> thornhill.no,casouri <at> gmail.com
>>>>>>>>> From: Jørgen Kvalsvik <j <at> lambda.is>
>>>>>>>>> Date: Sun, 24 Nov 2024 10:15:12 +0100
>>>>>>>>> 
>>>>>>>>> * lisp/progmodes/c-ts-mode.el (c-ts-mode--parent-is-not-top-compound):
>>>>>>>>> New function.
>>>>>>>>> (c-ts-mode--indent-styles): Use it.
>>>>>>>>> * test/lisp/progmodes/c-ts-mode-resources/indent.erts: New compound
>>>>>>>>> statement test.
>>>>>>>> Please make sure the lines here are not too long (see CONTRIBUTE for
>>>>>>>> details).
>>>>>>>>> +(defun c-ts-mode--parent-is-not-top-compound (_n parent &rest _)
>>>>>>>>> +  "Matches when PARENT is not the top level compound statement,
>>>>>>>>> +the {} that immediately follows the signature."
>>>>>>>> The first line of a doc string should be a single complete sentence.
>>>>>>>> Thanks.
>>>>>>> 
>>>>>>> Ok. Yuan, would you like me to submit a revision?
>>>>>> Since your assignment is already done, let’s just apply your patch, and I’ll rebase my changes on top of yours. So yeah, do send the revision patch, thanks!
>>>>>> Yuan
>>>>> 
>>>>> Certainly - I posted it on the bug tracker.
>>>> Thanks Jørgen. What did you use to generate the patch? For some reason I can’t apply it. My git skill isn’t that great so it could be my problem. If you can apply it fine maybe you can share the command you used?
>>> 
>>> I used git format-patch HEAD~1, and I just tested applying it to master with `git am 0001-Indent-compounds-in-c-ts-mode-when-is-not-BOL.patch' which worked.
>>> 
>>>> BTW, the commit title is missing. When you add the title, you can also add the bug number. For example:
>>>> Improve c-ts-mode indentation for macros (bug#74507)
>>>> Yuan
>>> 
>>> Sure. I've attached a new patch with the bug in it. I tested it and it applies cleanly with `git am 0001-Improve-c-ts-mode-compound-indents-bug-74507.patch'
>>> <0001-Improve-c-ts-mode-compound-indents-bug-74507.patch>
>> Thanks. This patch applied without a problem.
>> I reworked your heuristic into a function rule for better organization, and changed the condition slightly:
>> (defun c-ts-mode--macro-heuristic-rules (node parent &rest _)
>>   "Heuristic indent rule for control flow macros.
>> Eg,
>>     #define IOTA(var, n) for (int var = 0; var != (n); ++var)
>>     int main()
>>     {
>>       IOTA (v, 10) {
>>         printf(\"%d \", v);   <-- Here we want to indent
>>         counter++;            <-- Use baseline rule to align
>>     }                             to prev sibling
>> Checked by \"Compound Statement after code (Bug#74507)\" test.
>> NODE and PARENT are the same as other indent rules."
>>   (when (and (treesit-node-match-p parent "compound_statement")
>>              (treesit-node-match-p (treesit-node-prev-sibling parent)
>>                                    "expression_statement"))
>>     (let ((parent-bol
>>            (lambda () (save-excursion
>>                         (goto-char (treesit-node-start parent))
>>                         (back-to-indentation)
>>                         (point)))))
>>       (cond
>>        ;; Closing brace.
>>        ((treesit-node-match-p node "}")
>>         (cons (funcall parent-bol) 0))
>>        ;; First sibling.
>>        ((treesit-node-eq (treesit-node-child parent 0 'named) node)
>>         (cons (funcall parent-bol)
>>               c-ts-mode-indent-offset))))))
>> Instead of checking whether PARENT is top-level compound_statement, I make it check whether the previous sibling is an expression_statement. This way the heuristic works when the macro isn’t at top-level too. Here’s the updated test:
>> #define IOTA(var, n) for (int var = 0; var != (n); ++var)
>> int main()
>> {
>>   IOTA (v, 10) {
>>     printf("%d ", v);
>>   }
>>   const char *msg = "Hello, world!"; {
>>     puts("Hello, world!");
>>   }
>>   for (int i = 0;
>>        i < 10;
>>        i++) {
>>     IOTA (v, 10) {
>>       printf("%d ", v);
>>     }
>>   }
>>   {
>>     IOTA (v, 10) {
>>       printf("%d ", v);
>>     }
>>   }
>> }
>> The only test case that doesn’t pass right now is this one:
>>   const char *msg = "Hello, world!"; {
>>     puts("Hello, world!");
>>   }
>> Is this a real use-case?
>> Yuan
> 
> Maybe - I included it to show that the problem was not tied to it being a macro, but rather { not being the first thing on the line. It would be nice to support it, and c-mode also indents it properly.
> 
> That being said, it is probably rare enough in practice to prio it less.

Ok, I pushed your patch and my indentation rework to master. I end up removing the test mentioned above since it’s not a real use-case, in favor of making the heuristic indent rule more specific.

Yuan

Previous Next

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