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

Previous Next

Full log

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.