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

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

[Jørgen Kvalsvik]

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

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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Indent-compounds-c-ts-mode-when-is-not-BOL.patch --]
[-- Type: text/patch, Size: 3858 bytes --]

From ad5c50851710711fa1d62708633d7bae92f82fae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B8rgen=20Kvalsvik?= <j@lambda.is>
Date: Tue, 19 Nov 2024 08:01:01 +0100
Subject: [PATCH] Indent compounds c-ts-mode when { is not BOL

Properly indent the body of compound expressions, even when then
compound expression is not at the beginning of line and the parent is
not an if/for/while/etc., and matches the behavior of c-mode.

This fixes a problem that is common with macros and in testing
frameworks. For example, you expect this to indent:

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

If the compound statement is the function body itself, don't apply this
new rule and instead guide by the parent and first sibling.

I'm sure there are subtle interactions that aren't handled properly by
checking for "function_definition" rather than something more general,
but it does fix the test case and the check can be improved as more
cases are found.

* 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.
---
 lisp/progmodes/c-ts-mode.el                   |  8 +++++
 .../progmodes/c-ts-mode-resources/indent.erts | 30 +++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/lisp/progmodes/c-ts-mode.el b/lisp/progmodes/c-ts-mode.el
index 3823c553fda..80233e90241 100644
--- a/lisp/progmodes/c-ts-mode.el
+++ b/lisp/progmodes/c-ts-mode.el
@@ -402,6 +402,11 @@ c-ts-mode--first-sibling
               (treesit-node-start parent)
               (line-end-position))))))
 
+(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."
+  (not (equal "function_definition" (treesit-node-type (treesit-node-parent parent)))))
+
 (defun c-ts-mode--indent-styles (mode)
   "Indent rules supported by `c-ts-mode'.
 MODE is either `c' or `cpp'."
@@ -478,6 +483,7 @@ c-ts-mode--indent-styles
            ;; Closing bracket.  This should be before initializer_list
            ;; (and probably others) rule because that rule (and other
            ;; similar rules) will match the closing bracket.  (Bug#61398)
+           ((and (node-is "}") c-ts-mode--parent-is-not-top-compound) parent-bol 0)
            ((node-is "}") standalone-parent 0)
            ,@(when (eq mode 'cpp)
                '(((node-is "access_specifier") parent-bol 0)
@@ -497,6 +503,8 @@ c-ts-mode--indent-styles
            ((parent-is "field_declaration_list") c-ts-mode--anchor-prev-sibling 0)
 
            ;; Statement in {} blocks.
+           ((and (parent-is "compound_statement") c-ts-mode--parent-is-not-top-compound)
+            parent-bol c-ts-mode-indent-offset)
            ((or (and (parent-is "compound_statement")
                      ;; If the previous sibling(s) are not on their
                      ;; own line, indent as if this node is the first
diff --git a/test/lisp/progmodes/c-ts-mode-resources/indent.erts b/test/lisp/progmodes/c-ts-mode-resources/indent.erts
index 2f3540c3970..61e61677ed7 100644
--- a/test/lisp/progmodes/c-ts-mode-resources/indent.erts
+++ b/test/lisp/progmodes/c-ts-mode-resources/indent.erts
@@ -223,6 +223,36 @@ make_pair(int long_identifier_a[], int long_identifier_b[],
 
 =-=-=
 
+Name: Compound Statement after code
+
+=-=
+#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!");
+}
+}
+
+=-=
+#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!");
+  }
+}
+
+=-=-=
+
 Name: Switch-Case statement
 
 =-=
-- 
2.39.5

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

[Yuan Fu]

> On Nov 24, 2024, at 1:15 AM, Jørgen Kvalsvik <j@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

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

[Eli Zaretskii]

> Cc: theo@thornhill.no, 74507@debbugs.gnu.org
> From: Yuan Fu <casouri@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.

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

[Eli Zaretskii]

> Cc: theo@thornhill.no,casouri@gmail.com
> From: Jørgen Kvalsvik <j@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.

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

[Jørgen Kvalsvik]

On 11/29/24 05:56, Yuan Fu wrote:
> 
> 
>> On Nov 24, 2024, at 1:15 AM, Jørgen Kvalsvik <j@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.

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

[Jørgen Kvalsvik]

On 11/29/24 08:57, Eli Zaretskii wrote:
>> Cc: theo@thornhill.no,casouri@gmail.com
>> From: Jørgen Kvalsvik <j@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?

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

[Yuan Fu]

> On Nov 29, 2024, at 1:05 AM, Jørgen Kvalsvik <j@lambda.is> wrote:
> 
> On 11/29/24 08:57, Eli Zaretskii wrote:
>>> Cc: theo@thornhill.no,casouri@gmail.com
>>> From: Jørgen Kvalsvik <j@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