unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
@ 2023-12-27  6:21 Noah Peart
  2023-12-30  4:24 ` Yuan Fu
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2023-12-27  6:21 UTC (permalink / raw)
  To: 68054


[-- Attachment #1.1: Type: text/plain, Size: 1367 bytes --]

Tags: patch

* Bug: `js-ts-mode` and `typescript-ts-mode` are missing indentation
rules for lexical declarations that span multiple lines.

Recipe to reproduce:

Using the following function to configure js-ts-mode and indent the
buffer:

    (defun try-indent ()
      (interactive)
      (js-ts-mode)
      (setq-local indent-tabs-mode nil)
      (setq-local js-indent-level 4)
      (indent-region (point-min) (point-max)))

Add the following example to a buffer and call `try-indent`.

    let foo = 1,
    bar = 2; // no indent rule matches this line

No indentation is applied to the second line.

This patch adds a simple indentation rules for `js-ts-mode` and
`typescript-ts-mode` to handle the multi-line lexical declarations.


In GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo version 1.16.0) of 2023-12-26 built on nverno-XPS-8940
Repository revision: d376462c7183752bf44b9bd20bf5020fe7eaf75a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Ubuntu 22.04.3 LTS

Configured using:
 'configure --prefix=/usr/local --with-modules --with-tree-sitter
--with-threads --with-x-toolkit=gtk3 --with-xwidgets --with-gnutls
--with-json --with-mailutils --with-jpeg --with-png --with-rsvg
--with-tiff --with-xml2 --with-xpm --with-imagemagick CC=gcc-12
CXX=gcc-12'

[-- Attachment #1.2: Type: text/html, Size: 1551 bytes --]

[-- Attachment #2: js-ts-indent-decls.patch --]
[-- Type: text/x-patch, Size: 2072 bytes --]

From 4c2793fcdb1199253396511a13c13519c76bb817 Mon Sep 17 00:00:00 2001
From: Noah Peart <noah.v.peart@gmail.com>
Date: Wed, 27 Dec 2023 00:50:46 -0500
Subject: [PATCH] Add tree-sitter indent rule for lexical decls in
 js/typescript

* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--indent-rules): Add indent rule for
lexical_declarations.

* lisp/progmodes/js.el (js--treesit-indent-rules): Add indent rule
for lexical_declarations.
---
 lisp/progmodes/js.el                 | 1 +
 lisp/progmodes/typescript-ts-mode.el | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 61bd94222ac..935655f779f 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3449,6 +3449,7 @@ js--treesit-indent-rules
        ((parent-is "named_imports") parent-bol js-indent-level)
        ((parent-is "statement_block") parent-bol js-indent-level)
        ((parent-is "variable_declarator") parent-bol js-indent-level)
+       ((parent-is "lexical_declaration") parent-bol js-indent-level)
        ((parent-is "arguments") parent-bol js-indent-level)
        ((parent-is "array") parent-bol js-indent-level)
        ((parent-is "formal_parameters") parent-bol js-indent-level)
diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index 7f0b7236301..a0f6e7c9501 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -114,6 +114,7 @@ typescript-ts-mode--indent-rules
      ((parent-is "switch_default") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "type_arguments") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "variable_declarator") parent-bol typescript-ts-mode-indent-offset)
+     ((parent-is "lexical_declaration") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "arguments") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "array") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "formal_parameters") parent-bol typescript-ts-mode-indent-offset)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-27  6:21 bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript Noah Peart
@ 2023-12-30  4:24 ` Yuan Fu
  2023-12-30 20:31   ` Yuan Fu
  0 siblings, 1 reply; 16+ messages in thread
From: Yuan Fu @ 2023-12-30  4:24 UTC (permalink / raw)
  To: Noah Peart; +Cc: 68054



> On Dec 26, 2023, at 10:21 PM, Noah Peart <noah.v.peart@gmail.com> wrote:
> 
> Tags: patch
> 
> * Bug: `js-ts-mode` and `typescript-ts-mode` are missing indentation
> rules for lexical declarations that span multiple lines.
> 
> Recipe to reproduce:
> 
> Using the following function to configure js-ts-mode and indent the
> buffer:
> 
>     (defun try-indent ()
>       (interactive)
>       (js-ts-mode)
>       (setq-local indent-tabs-mode nil)
>       (setq-local js-indent-level 4)
>       (indent-region (point-min) (point-max)))
> 
> Add the following example to a buffer and call `try-indent`.
> 
>     let foo = 1,
>     bar = 2; // no indent rule matches this line
> 
> No indentation is applied to the second line.
> 
> This patch adds a simple indentation rules for `js-ts-mode` and
> `typescript-ts-mode` to handle the multi-line lexical declarations.

Thanks, and sorry for not seeing this. I’ll take a look tonight.

Yuan




^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-30  4:24 ` Yuan Fu
@ 2023-12-30 20:31   ` Yuan Fu
  2023-12-31  0:31     ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Yuan Fu @ 2023-12-30 20:31 UTC (permalink / raw)
  To: Noah Peart; +Cc: 68054



> On Dec 29, 2023, at 8:24 PM, Yuan Fu <casouri@gmail.com> wrote:
> 
> 
> 
>> On Dec 26, 2023, at 10:21 PM, Noah Peart <noah.v.peart@gmail.com> wrote:
>> 
>> Tags: patch
>> 
>> * Bug: `js-ts-mode` and `typescript-ts-mode` are missing indentation
>> rules for lexical declarations that span multiple lines.
>> 
>> Recipe to reproduce:
>> 
>> Using the following function to configure js-ts-mode and indent the
>> buffer:
>> 
>>    (defun try-indent ()
>>      (interactive)
>>      (js-ts-mode)
>>      (setq-local indent-tabs-mode nil)
>>      (setq-local js-indent-level 4)
>>      (indent-region (point-min) (point-max)))
>> 
>> Add the following example to a buffer and call `try-indent`.
>> 
>>    let foo = 1,
>>    bar = 2; // no indent rule matches this line
>> 
>> No indentation is applied to the second line.
>> 
>> This patch adds a simple indentation rules for `js-ts-mode` and
>> `typescript-ts-mode` to handle the multi-line lexical declarations.

It seems that js-mode indents bar to align with foo, rather than indenting one level. I feel that we should do the same, WDYT?

Yuan




^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-30 20:31   ` Yuan Fu
@ 2023-12-31  0:31     ` Dmitry Gutov
  2023-12-31  5:35       ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2023-12-31  0:31 UTC (permalink / raw)
  To: Yuan Fu, Noah Peart; +Cc: 68054

On 30/12/2023 22:31, Yuan Fu wrote:
> 
>> On Dec 29, 2023, at 8:24 PM, Yuan Fu<casouri@gmail.com>  wrote:
>>
>>
>>
>>> On Dec 26, 2023, at 10:21 PM, Noah Peart<noah.v.peart@gmail.com>  wrote:
>>>
>>> Tags: patch
>>>
>>> * Bug: `js-ts-mode` and `typescript-ts-mode` are missing indentation
>>> rules for lexical declarations that span multiple lines.
>>>
>>> Recipe to reproduce:
>>>
>>> Using the following function to configure js-ts-mode and indent the
>>> buffer:
>>>
>>>     (defun try-indent ()
>>>       (interactive)
>>>       (js-ts-mode)
>>>       (setq-local indent-tabs-mode nil)
>>>       (setq-local js-indent-level 4)
>>>       (indent-region (point-min) (point-max)))
>>>
>>> Add the following example to a buffer and call `try-indent`.
>>>
>>>     let foo = 1,
>>>     bar = 2; // no indent rule matches this line
>>>
>>> No indentation is applied to the second line.
>>>
>>> This patch adds a simple indentation rules for `js-ts-mode` and
>>> `typescript-ts-mode` to handle the multi-line lexical declarations.
> It seems that js-mode indents bar to align with foo, rather than indenting one level. I feel that we should do the same, WDYT?

Yes, please. This also makes a difference when the variables are 
declared with 'const' instead of 'let'.

We should also support 'var' declarations: the parent node type to match 
against is "variable_declaration".





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-31  0:31     ` Dmitry Gutov
@ 2023-12-31  5:35       ` Noah Peart
  2023-12-31 13:41         ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2023-12-31  5:35 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 68054

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

Yea, I agree that would be better - would you align on start the variable
names, or '=' like
`c-lineup-assignments`?

On Sat, Dec 30, 2023 at 7:31 PM Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 30/12/2023 22:31, Yuan Fu wrote:
> >
> >> On Dec 29, 2023, at 8:24 PM, Yuan Fu<casouri@gmail.com>  wrote:
> >>
> >>
> >>
> >>> On Dec 26, 2023, at 10:21 PM, Noah Peart<noah.v.peart@gmail.com>
> wrote:
> >>>
> >>> Tags: patch
> >>>
> >>> * Bug: `js-ts-mode` and `typescript-ts-mode` are missing indentation
> >>> rules for lexical declarations that span multiple lines.
> >>>
> >>> Recipe to reproduce:
> >>>
> >>> Using the following function to configure js-ts-mode and indent the
> >>> buffer:
> >>>
> >>>     (defun try-indent ()
> >>>       (interactive)
> >>>       (js-ts-mode)
> >>>       (setq-local indent-tabs-mode nil)
> >>>       (setq-local js-indent-level 4)
> >>>       (indent-region (point-min) (point-max)))
> >>>
> >>> Add the following example to a buffer and call `try-indent`.
> >>>
> >>>     let foo = 1,
> >>>     bar = 2; // no indent rule matches this line
> >>>
> >>> No indentation is applied to the second line.
> >>>
> >>> This patch adds a simple indentation rules for `js-ts-mode` and
> >>> `typescript-ts-mode` to handle the multi-line lexical declarations.
> > It seems that js-mode indents bar to align with foo, rather than
> indenting one level. I feel that we should do the same, WDYT?
>
> Yes, please. This also makes a difference when the variables are
> declared with 'const' instead of 'let'.
>
> We should also support 'var' declarations: the parent node type to match
> against is "variable_declaration".
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-31  5:35       ` Noah Peart
@ 2023-12-31 13:41         ` Dmitry Gutov
  2024-01-01  4:56           ` Yuan Fu
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2023-12-31 13:41 UTC (permalink / raw)
  To: Noah Peart; +Cc: Yuan Fu, 68054

On 31/12/2023 07:35, Noah Peart wrote:
> Yea, I agree that would be better - would you align on start the 
> variable names, or '=' like
> `c-lineup-assignments`?

Like js-mode would be good.

I'm not familiar with c-lineup-assignments, but we could add different 
variations later.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2023-12-31 13:41         ` Dmitry Gutov
@ 2024-01-01  4:56           ` Yuan Fu
  2024-04-17 20:21             ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Yuan Fu @ 2024-01-01  4:56 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Noah Peart, 68054



> On Dec 31, 2023, at 5:41 AM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> 
> On 31/12/2023 07:35, Noah Peart wrote:
>> Yea, I agree that would be better - would you align on start the variable names, or '=' like
>> `c-lineup-assignments`?
> 
> Like js-mode would be good.
> 
> I'm not familiar with c-lineup-assignments, but we could add different variations later.

Also, if you are feeling adventurous, I noticed that the second variable in a lexical_declaration isn’t fontified in variable-name-face. It’d be nice to fix that as well.

Yuan




^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-01-01  4:56           ` Yuan Fu
@ 2024-04-17 20:21             ` Noah Peart
  2024-04-23  5:07               ` Yuan Fu
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2024-04-17 20:21 UTC (permalink / raw)
  To: Yuan Fu; +Cc: Dmitry Gutov, 68054


[-- Attachment #1.1: Type: text/plain, Size: 1917 bytes --]

Sorry, I forgot about this. I've just added a rule to align the
variable_declarators
in let, var, and const declarations, but I need some feedback about the
indentation
for values in the variable_declarators following dangling '='.

For example, which of the following would be preferable?

1) indent the dangling values with respect to start of the declaration

    const a =
        (x: string): string => {
            return x + x;
        },
          bbb =
        {
            "x": 0
        },
          cccc =
        1,
          ddddd = 0;

2) indent them with respect to the start of the variable_declarator

    const a =
              (x: string): string => {
                  return x + x;
              },
          bbb =
              {
                  "x": 0
              },
          cccc =
              1,
          ddddd = 0;

3) align with the variable declarators (this is the same as js-mode)

    const a =
          (x: string): string => {
              return x + x;
          },
          bbb =
          {
              "x": 0
          },
          cccc =
          1,
          ddddd = 0;

I've attached a patch with with the rules for the 3 options here.

On Sun, Dec 31, 2023 at 8:56 PM Yuan Fu <casouri@gmail.com> wrote:

>
>
> > On Dec 31, 2023, at 5:41 AM, Dmitry Gutov <dmitry@gutov.dev> wrote:
> >
> > On 31/12/2023 07:35, Noah Peart wrote:
> >> Yea, I agree that would be better - would you align on start the
> variable names, or '=' like
> >> `c-lineup-assignments`?
> >
> > Like js-mode would be good.
> >
> > I'm not familiar with c-lineup-assignments, but we could add different
> variations later.
>
> Also, if you are feeling adventurous, I noticed that the second variable
> in a lexical_declaration isn’t fontified in variable-name-face. It’d be
> nice to fix that as well.
>
> Yuan

[-- Attachment #1.2: Type: text/html, Size: 2805 bytes --]

[-- Attachment #2: typescript-ts-decl-indent.patch --]
[-- Type: text/x-patch, Size: 2492 bytes --]

diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index ab1d76ab20e..9dbe5df8368 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -91,6 +91,16 @@ tsx-ts-mode--indent-compatibility-b893426
      `(((match "<" "jsx_text") parent 0)
        ((parent-is "jsx_text") parent typescript-ts-mode-indent-offset)))))
 
+(defun typescript-ts-mode--anchor-decl (_n parent &rest _)
+  "Return the position after the declaration keyword before PARENT.
+
+This anchor allows aligning variable_declarators in variable and lexical
+declarations, accounting for the length of keyword (var, let, or const)."
+  (let ((decl (treesit-parent-until
+               parent (rx (or "variable" "lexical") "_declaration") t)))
+    (+ (treesit-node-start decl)
+       (length (treesit-node-text (treesit-node-child decl 0))))))
+
 (defun typescript-ts-mode--indent-rules (language)
   "Rules used for indentation.
 Argument LANGUAGE is either `typescript' or `tsx'."
@@ -113,7 +123,24 @@ typescript-ts-mode--indent-rules
      ((parent-is "switch_case") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "switch_default") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "type_arguments") parent-bol typescript-ts-mode-indent-offset)
-     ((parent-is "variable_declarator") parent-bol typescript-ts-mode-indent-offset)
+
+     ;; 1. indent with respect to declaration start:
+     ((parent-is "variable_declarator") grand-parent typescript-ts-mode-indent-offset)
+     ((parent-is ,(rx (or "variable" "lexical") "_declaration"))
+      typescript-ts-mode--anchor-decl 1)
+
+     ;; 2. indent with respect to variable_declarator start:
+     ((match nil "variable_declarator" nil 0 1)
+      parent-bol typescript-ts-mode-indent-offset)
+     ((n-p-gp nil "variable_declarator" ,(rx (or "variable" "lexical") "_declaration"))
+      parent typescript-ts-mode-indent-offset)
+     ((parent-is ,(rx (or "variable" "lexical") "_declaration"))
+      typescript-ts-mode--anchor-decl 1)
+
+     ;; 3. align with variable declarator (like js-mode)
+     ((parent-is ,(rx (or "variable" "lexical") "_" (or "declaration" "declarator")))
+      typescript-ts-mode--anchor-decl 1)
+
      ((parent-is "arguments") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "array") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "formal_parameters") parent-bol typescript-ts-mode-indent-offset)

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-17 20:21             ` Noah Peart
@ 2024-04-23  5:07               ` Yuan Fu
  2024-04-24  0:15                 ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Yuan Fu @ 2024-04-23  5:07 UTC (permalink / raw)
  To: Noah Peart; +Cc: Dmitry Gutov, 68054



> On Apr 17, 2024, at 1:21 PM, Noah Peart <noah.v.peart@gmail.com> wrote:
> 
> Sorry, I forgot about this. I've just added a rule to align the variable_declarators
> in let, var, and const declarations, but I need some feedback about the indentation
> for values in the variable_declarators following dangling '='.

Thanks!

> 
> For example, which of the following would be preferable?
> 
> 1) indent the dangling values with respect to start of the declaration
> 
>     const a =
>         (x: string): string => {
>             return x + x;
>         },
>           bbb =
>         {
>             "x": 0
>         },
>           cccc =
>         1,
>           ddddd = 0;
>   
> 2) indent them with respect to the start of the variable_declarator
> 
>     const a =
>               (x: string): string => {
>                   return x + x;
>               },
>           bbb =
>               {
>                   "x": 0
>               },
>           cccc =
>               1,
>           ddddd = 0;
> 
> 3) align with the variable declarators (this is the same as js-mode)
> 
>     const a =
>           (x: string): string => {
>               return x + x;
>           },
>           bbb =
>           {
>               "x": 0
>           },
>           cccc =
>           1,
>           ddddd = 0;
> 
> I've attached a patch with with the rules for the 3 options here.

I don’t really know what’s the convention, if there is one. Maybe Dmitry has better idea. Personally I like option 1.

Yuan




^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-23  5:07               ` Yuan Fu
@ 2024-04-24  0:15                 ` Dmitry Gutov
  2024-04-24  2:36                   ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-04-24  0:15 UTC (permalink / raw)
  To: Yuan Fu, Noah Peart; +Cc: 68054

On 23/04/2024 08:07, Yuan Fu wrote:
>> 1) indent the dangling values with respect to start of the declaration
>>
>>      const a =
>>          (x: string): string => {
>>              return x + x;
>>          },
>>            bbb =
>>          {
>>              "x": 0
>>          },
>>            cccc =
>>          1,
>>            ddddd = 0;
>>    
>> 2) indent them with respect to the start of the variable_declarator
>>
>>      const a =
>>                (x: string): string => {
>>                    return x + x;
>>                },
>>            bbb =
>>                {
>>                    "x": 0
>>                },
>>            cccc =
>>                1,
>>            ddddd = 0;
>>
>> 3) align with the variable declarators (this is the same as js-mode)
>>
>>      const a =
>>            (x: string): string => {
>>                return x + x;
>>            },
>>            bbb =
>>            {
>>                "x": 0
>>            },
>>            cccc =
>>            1,
>>            ddddd = 0;
>>
>> I've attached a patch with with the rules for the 3 options here.
> I don’t really know what’s the convention, if there is one. Maybe Dmitry has better idea. Personally I like option 1.

What js-mode does (looks like 3) shouldn't be too bad, but I wouldn't 
mind an extra indentation level for such cases, too (example 2).

This seems like a rare enough case, so it probably doesn't matter too 
much, so I'd suggest picking one style and implementing it, and then 
adjusting based on user feedback later.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-24  0:15                 ` Dmitry Gutov
@ 2024-04-24  2:36                   ` Noah Peart
  2024-04-24 23:15                     ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2024-04-24  2:36 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 68054


[-- Attachment #1.1: Type: text/plain, Size: 2644 bytes --]

> What js-mode does (looks like 3) shouldn't be too bad, but I wouldn't
mind an extra indentation level for such cases, too (example 2).

The problem I found with option 2/3 was cases like the following (which
also seem like the most common) where I expect the start of
the function be indented according to `typescript-ts-mode-indent-offset`,
not with the declaration keyword "const" as in `js-mode` (and option 3).

    const someFuncWithReallyLongName =
      async (x: number, y: number, z: number): Promise<void> => {
        // ...
      };

> This seems like a rare enough case, so it probably doesn't matter too
much, so I'd suggest picking one style and implementing it, and then
adjusting based on user feedback later.

I've attached a patch implementing option 1 for now (with a test), but
I'm happy to change the style whenever.

Thanks!

On Tue, Apr 23, 2024 at 5:15 PM Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 23/04/2024 08:07, Yuan Fu wrote:
> >> 1) indent the dangling values with respect to start of the declaration
> >>
> >>      const a =
> >>          (x: string): string => {
> >>              return x + x;
> >>          },
> >>            bbb =
> >>          {
> >>              "x": 0
> >>          },
> >>            cccc =
> >>          1,
> >>            ddddd = 0;
> >>
> >> 2) indent them with respect to the start of the variable_declarator
> >>
> >>      const a =
> >>                (x: string): string => {
> >>                    return x + x;
> >>                },
> >>            bbb =
> >>                {
> >>                    "x": 0
> >>                },
> >>            cccc =
> >>                1,
> >>            ddddd = 0;
> >>
> >> 3) align with the variable declarators (this is the same as js-mode)
> >>
> >>      const a =
> >>            (x: string): string => {
> >>                return x + x;
> >>            },
> >>            bbb =
> >>            {
> >>                "x": 0
> >>            },
> >>            cccc =
> >>            1,
> >>            ddddd = 0;
> >>
> >> I've attached a patch with with the rules for the 3 options here.
> > I don’t really know what’s the convention, if there is one. Maybe Dmitry
> has better idea. Personally I like option 1.
>
> What js-mode does (looks like 3) shouldn't be too bad, but I wouldn't
> mind an extra indentation level for such cases, too (example 2).
>
> This seems like a rare enough case, so it probably doesn't matter too
> much, so I'd suggest picking one style and implementing it, and then
> adjusting based on user feedback later.
>

[-- Attachment #1.2: Type: text/html, Size: 3842 bytes --]

[-- Attachment #2: 0001-Add-typescript-ts-mode-indentation-for-multi-assignm.patch --]
[-- Type: text/x-patch, Size: 3856 bytes --]

From f202355048c8d915c6978f6e9300dd63104b8a52 Mon Sep 17 00:00:00 2001
From: Noah Peart <noah.v.peart@gmail.com>
Date: Fri, 19 Apr 2024 01:46:50 -0700
Subject: [PATCH] Add typescript-ts-mode indentation for multi-assignment decls

* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--indent-rules): Add indentation rules for
lexical and variable declarations with multiple assignments.
* test/lisp/progmodes/typescript-ts-mode-resources/indent.erts:
Add indent test for variable declarations.
---
 .last-build-hash                              |  1 +
 lisp/progmodes/typescript-ts-mode.el          | 15 +++++++++-
 .../typescript-ts-mode-resources/indent.erts  | 28 +++++++++++++++++++
 3 files changed, 43 insertions(+), 1 deletion(-)
 create mode 100644 .last-build-hash

diff --git a/.last-build-hash b/.last-build-hash
new file mode 100644
index 00000000000..77fe4446df5
--- /dev/null
+++ b/.last-build-hash
@@ -0,0 +1 @@
+a2e327cbca1e756373109d4788ea635250d23224
diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index ab1d76ab20e..3c10a19e712 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -91,6 +91,17 @@ tsx-ts-mode--indent-compatibility-b893426
      `(((match "<" "jsx_text") parent 0)
        ((parent-is "jsx_text") parent typescript-ts-mode-indent-offset)))))
 
+(defun typescript-ts-mode--anchor-decl (_n parent &rest _)
+  "Return the position after the declaration keyword before PARENT.
+
+This anchor allows aligning variable_declarators in variable and lexical
+declarations, accounting for the length of keyword (var, let, or const)."
+  (let* ((declaration (treesit-parent-until
+                       parent (rx (or "variable" "lexical") "_declaration") t))
+         (decl (treesit-node-child declaration 0)))
+    (+ (treesit-node-start declaration)
+       (- (treesit-node-end decl) (treesit-node-start decl)))))
+
 (defun typescript-ts-mode--indent-rules (language)
   "Rules used for indentation.
 Argument LANGUAGE is either `typescript' or `tsx'."
@@ -113,7 +124,9 @@ typescript-ts-mode--indent-rules
      ((parent-is "switch_case") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "switch_default") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "type_arguments") parent-bol typescript-ts-mode-indent-offset)
-     ((parent-is "variable_declarator") parent-bol typescript-ts-mode-indent-offset)
+     ((parent-is "variable_declarator") grand-parent typescript-ts-mode-indent-offset)
+     ((parent-is ,(rx (or "variable" "lexical") "_declaration"))
+      typescript-ts-mode--anchor-decl 1)
      ((parent-is "arguments") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "array") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "formal_parameters") parent-bol typescript-ts-mode-indent-offset)
diff --git a/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts b/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
index bec96ad82e0..908dd8103b5 100644
--- a/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
+++ b/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
@@ -62,6 +62,34 @@ const foo = (x: string) => {
 };
 =-=-=
 
+Name: Lexical and variable declarations
+
+=-=
+const foo = () => {
+  let x = 1,
+      yyyy: {
+        [k: string | number]: string,
+      } = {
+        "foo": "foo",
+        "bar": "bar",
+      };
+  var obar = 1,
+      fo: { [x: any]: any } = {
+        "a": 1,
+        "b": 2,
+      };
+  const someFuncWithReallyLongName =
+    async (x: number, y: number, z: number): Promise<void> => {
+      return new Promise();
+    };
+  const cccc = 1,
+        bbb = {
+          "x": 0
+        },
+        ddddd = 0;
+};
+=-=-=
+
 Code:
   (lambda ()
     (setq indent-tabs-mode nil)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-24  2:36                   ` Noah Peart
@ 2024-04-24 23:15                     ` Dmitry Gutov
  2024-04-25 22:48                       ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-04-24 23:15 UTC (permalink / raw)
  To: Noah Peart; +Cc: Yuan Fu, 68054

On 24/04/2024 05:36, Noah Peart wrote:
>  > What js-mode does (looks like 3) shouldn't be too bad, but I wouldn't
> mind an extra indentation level for such cases, too (example 2).
> 
> The problem I found with option 2/3 was cases like the following (which
> also seem like the most common) where I expect the start of
> the function be indented according to `typescript-ts-mode-indent-offset`,
> not with the declaration keyword "const" as in `js-mode` (and option 3).
> 
>      const someFuncWithReallyLongName =
>        async (x: number, y: number, z: number): Promise<void> => {
>          // ...
>        };

I don't know, in my understanding the line break after the "=" (or its 
absence) is usually a good enough look to choose between indentation 
offsets for a given statement. I.e. with the break after = it would be 
indented deeply, and without it (keeping "async" on the first line) the 
body would have the same base indentation as the "const" statement.

Though of course some users like it differently anyway.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-24 23:15                     ` Dmitry Gutov
@ 2024-04-25 22:48                       ` Noah Peart
  2024-04-26  1:06                         ` Dmitry Gutov
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2024-04-25 22:48 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 68054

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

Ok, I can make a patch for option 3 (the same as js-mode) instead. It's
implementation is the simplest also.  Would that work?

Thanks

On Wed, Apr 24, 2024 at 4:15 PM Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 24/04/2024 05:36, Noah Peart wrote:
> >  > What js-mode does (looks like 3) shouldn't be too bad, but I wouldn't
> > mind an extra indentation level for such cases, too (example 2).
> >
> > The problem I found with option 2/3 was cases like the following (which
> > also seem like the most common) where I expect the start of
> > the function be indented according to `typescript-ts-mode-indent-offset`,
> > not with the declaration keyword "const" as in `js-mode` (and option 3).
> >
> >      const someFuncWithReallyLongName =
> >        async (x: number, y: number, z: number): Promise<void> => {
> >          // ...
> >        };
>
> I don't know, in my understanding the line break after the "=" (or its
> absence) is usually a good enough look to choose between indentation
> offsets for a given statement. I.e. with the break after = it would be
> indented deeply, and without it (keeping "async" on the first line) the
> body would have the same base indentation as the "const" statement.
>
> Though of course some users like it differently anyway.
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-25 22:48                       ` Noah Peart
@ 2024-04-26  1:06                         ` Dmitry Gutov
  2024-05-02 13:26                           ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Gutov @ 2024-04-26  1:06 UTC (permalink / raw)
  To: Noah Peart; +Cc: Yuan Fu, 68054

On 26/04/2024 01:48, Noah Peart wrote:
> Ok, I can make a patch for option 3 (the same as js-mode) instead. It's
> implementation is the simplest also.  Would that work?

I'd be okay with it, yes, thank you. If only because it's good to have 
similar defaults in both modes, and evolve them together until one is 
deprecated.

FTR, the option more in line with my previous explanation would be 
option 3, I think. Rhere's no real need to implement it right now, I 
guess, but for illustration:

const a =
         (x: string): string => {
           return x + x;
         },
       bbb =
         {
           "x": 0
         },
       cccc =
         1,
       ddddd = 0;

const bbb =
       {
         "x": 0
       }, a = (x: string): string => {
         return x + x;
       },
       bbb = {
         "x": 0
       },
       cccc = 1,
       ddddd = 0;

js-mode also has by necessity the below exceptions: when the first 
variable's value starts on the same line, and it's multiline (usually 
that means that the same line ends with a {), then its indentation level 
goes back to the statement, not to the "const" keyword:

const a = (x: string): string => {
   return x + x;
}

var bbb = {
   "x": 0
}

This is basically to support the non-multivar declarations better. 
js-ts-mode now has the exact same indentation; these cases might come up 
when you change how the first two cases indent, however.





^ permalink raw reply	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-04-26  1:06                         ` Dmitry Gutov
@ 2024-05-02 13:26                           ` Noah Peart
  2024-05-02 13:38                             ` Noah Peart
  0 siblings, 1 reply; 16+ messages in thread
From: Noah Peart @ 2024-05-02 13:26 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 68054


[-- Attachment #1.1: Type: text/plain, Size: 1794 bytes --]

Ok, here is an updated patch for option 3.  The indentation looks like the
examples
you've provided - it's the same indentation as in js-mode for these cases.


On Thu, Apr 25, 2024 at 6:06 PM Dmitry Gutov <dmitry@gutov.dev> wrote:

> On 26/04/2024 01:48, Noah Peart wrote:
> > Ok, I can make a patch for option 3 (the same as js-mode) instead. It's
> > implementation is the simplest also.  Would that work?
>
> I'd be okay with it, yes, thank you. If only because it's good to have
> similar defaults in both modes, and evolve them together until one is
> deprecated.
>
> FTR, the option more in line with my previous explanation would be
> option 3, I think. Rhere's no real need to implement it right now, I
> guess, but for illustration:
>
> const a =
>          (x: string): string => {
>            return x + x;
>          },
>        bbb =
>          {
>            "x": 0
>          },
>        cccc =
>          1,
>        ddddd = 0;
>
> const bbb =
>        {
>          "x": 0
>        }, a = (x: string): string => {
>          return x + x;
>        },
>        bbb = {
>          "x": 0
>        },
>        cccc = 1,
>        ddddd = 0;
>
> js-mode also has by necessity the below exceptions: when the first
> variable's value starts on the same line, and it's multiline (usually
> that means that the same line ends with a {), then its indentation level
> goes back to the statement, not to the "const" keyword:
>
> const a = (x: string): string => {
>    return x + x;
> }
>
> var bbb = {
>    "x": 0
> }
>
> This is basically to support the non-multivar declarations better.
> js-ts-mode now has the exact same indentation; these cases might come up
> when you change how the first two cases indent, however.
>

[-- Attachment #1.2: Type: text/html, Size: 2404 bytes --]

[-- Attachment #2: 0001-Add-typescript-ts-mode-indentation-for-multi-assignm.patch --]
[-- Type: text/x-patch, Size: 3510 bytes --]

From 1e23b33a3b4b930386a932ee6177eaf1b6e6a96e Mon Sep 17 00:00:00 2001
From: Noah Peart <noah.v.peart@gmail.com>
Date: Fri, 19 Apr 2024 01:46:50 -0700
Subject: [PATCH] Add typescript-ts-mode indentation for multi-assignment decls

* lisp/progmodes/typescript-ts-mode.el
(typescript-ts-mode--indent-rules): Add indentation rules for
lexical and variable declarations with multiple assignments.
* test/lisp/progmodes/typescript-ts-mode-resources/indent.erts:
Add indent test for variable declarations.
---
 lisp/progmodes/typescript-ts-mode.el          | 14 ++++++++-
 .../typescript-ts-mode-resources/indent.erts  | 31 +++++++++++++++++++
 2 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/typescript-ts-mode.el b/lisp/progmodes/typescript-ts-mode.el
index ab1d76ab20e..ed60819388f 100644
--- a/lisp/progmodes/typescript-ts-mode.el
+++ b/lisp/progmodes/typescript-ts-mode.el
@@ -91,6 +91,17 @@ tsx-ts-mode--indent-compatibility-b893426
      `(((match "<" "jsx_text") parent 0)
        ((parent-is "jsx_text") parent typescript-ts-mode-indent-offset)))))
 
+(defun typescript-ts-mode--anchor-decl (_n parent &rest _)
+  "Return the position after the declaration keyword before PARENT.
+
+This anchor allows aligning variable_declarators in variable and lexical
+declarations, accounting for the length of keyword (var, let, or const)."
+  (let* ((declaration (treesit-parent-until
+                       parent (rx (or "variable" "lexical") "_declaration") t))
+         (decl (treesit-node-child declaration 0)))
+    (+ (treesit-node-start declaration)
+       (- (treesit-node-end decl) (treesit-node-start decl)))))
+
 (defun typescript-ts-mode--indent-rules (language)
   "Rules used for indentation.
 Argument LANGUAGE is either `typescript' or `tsx'."
@@ -113,7 +124,8 @@ typescript-ts-mode--indent-rules
      ((parent-is "switch_case") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "switch_default") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "type_arguments") parent-bol typescript-ts-mode-indent-offset)
-     ((parent-is "variable_declarator") parent-bol typescript-ts-mode-indent-offset)
+     ((parent-is ,(rx (or "variable" "lexical") "_" (or "declaration" "declarator")))
+      typescript-ts-mode--anchor-decl 1)
      ((parent-is "arguments") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "array") parent-bol typescript-ts-mode-indent-offset)
      ((parent-is "formal_parameters") parent-bol typescript-ts-mode-indent-offset)
diff --git a/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts b/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
index bec96ad82e0..877382953c1 100644
--- a/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
+++ b/test/lisp/progmodes/typescript-ts-mode-resources/indent.erts
@@ -62,6 +62,37 @@ const foo = (x: string) => {
 };
 =-=-=
 
+Name: Lexical and variable declarations
+
+=-=
+const foo = () => {
+  let x = 1,
+      yyyy: {
+        [k: string | number]: string,
+      } = {
+        "foo": "foo",
+        "bar": "bar",
+      };
+  var obar = 1,
+      fo: { [x: any]: any } = {
+        "a": 1,
+        "b": 2,
+      };
+  const cccc = 1,
+        bbb = {
+          "x": 0
+        },
+        ddddd = 0;
+  // First decls with value starting on same line
+  const a = (x: string): string => {
+    return x + x;
+  };
+  var bbb = {
+    "x": 0
+  };
+};
+=-=-=
+
 Code:
   (lambda ()
     (setq indent-tabs-mode nil)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript
  2024-05-02 13:26                           ` Noah Peart
@ 2024-05-02 13:38                             ` Noah Peart
  0 siblings, 0 replies; 16+ messages in thread
From: Noah Peart @ 2024-05-02 13:38 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Yuan Fu, 68054

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

> js-ts-mode now has the exact same indentation

I'm not sure what you meant by this - did you mean js-ts-mode has
the same indentation for non-multivar declarations as the examples
shown above? js-ts-mode is still missing indentation rules for multiple
decl statements.
The patch would work for js-ts-mode as well, but I didn't want to copy the
code between the files.

So much of the grammar is shared between js-ts-mode and
typescript-ts-mode, it might be worth combining - neovim uses an ecma
grammar
from which they both inherit.

On Thu, May 2, 2024 at 6:26 AM Noah Peart <noah.v.peart@gmail.com> wrote:

> Ok, here is an updated patch for option 3.  The indentation looks like the
> examples
> you've provided - it's the same indentation as in js-mode for these cases.
>
>
> On Thu, Apr 25, 2024 at 6:06 PM Dmitry Gutov <dmitry@gutov.dev> wrote:
>
>> On 26/04/2024 01:48, Noah Peart wrote:
>> > Ok, I can make a patch for option 3 (the same as js-mode) instead. It's
>> > implementation is the simplest also.  Would that work?
>>
>> I'd be okay with it, yes, thank you. If only because it's good to have
>> similar defaults in both modes, and evolve them together until one is
>> deprecated.
>>
>> FTR, the option more in line with my previous explanation would be
>> option 3, I think. Rhere's no real need to implement it right now, I
>> guess, but for illustration:
>>
>> const a =
>>          (x: string): string => {
>>            return x + x;
>>          },
>>        bbb =
>>          {
>>            "x": 0
>>          },
>>        cccc =
>>          1,
>>        ddddd = 0;
>>
>> const bbb =
>>        {
>>          "x": 0
>>        }, a = (x: string): string => {
>>          return x + x;
>>        },
>>        bbb = {
>>          "x": 0
>>        },
>>        cccc = 1,
>>        ddddd = 0;
>>
>> js-mode also has by necessity the below exceptions: when the first
>> variable's value starts on the same line, and it's multiline (usually
>> that means that the same line ends with a {), then its indentation level
>> goes back to the statement, not to the "const" keyword:
>>
>> const a = (x: string): string => {
>>    return x + x;
>> }
>>
>> var bbb = {
>>    "x": 0
>> }
>>
>> This is basically to support the non-multivar declarations better.
>> js-ts-mode now has the exact same indentation; these cases might come up
>> when you change how the first two cases indent, however.
>>
>

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

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2024-05-02 13:38 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-27  6:21 bug#68054: [PATCH] Add tree-sitter indent rule for lexical decls in js/typescript Noah Peart
2023-12-30  4:24 ` Yuan Fu
2023-12-30 20:31   ` Yuan Fu
2023-12-31  0:31     ` Dmitry Gutov
2023-12-31  5:35       ` Noah Peart
2023-12-31 13:41         ` Dmitry Gutov
2024-01-01  4:56           ` Yuan Fu
2024-04-17 20:21             ` Noah Peart
2024-04-23  5:07               ` Yuan Fu
2024-04-24  0:15                 ` Dmitry Gutov
2024-04-24  2:36                   ` Noah Peart
2024-04-24 23:15                     ` Dmitry Gutov
2024-04-25 22:48                       ` Noah Peart
2024-04-26  1:06                         ` Dmitry Gutov
2024-05-02 13:26                           ` Noah Peart
2024-05-02 13:38                             ` Noah Peart

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).