Subject: Re: bug#76704: 30.1; Indentation of braces on separate line in js-ts-mode

From: Fabian Brosda <[email-masked]>
Date: Thu, 27 Mar 2025 19:59:33 +0100

Hi Vincenzo,

great that there is already an updated patch. I did test it and it looks
good for functions, conditionals, loops.  Regarding arrow functions (or
variable declarations in general), the result is different, then with
js-mode, but personally I think the result looks more consistent in
js-ts-mode.

There is one exception though. switch-case statements with the brace on
a separate line inside a function break js-ts-mode. But it is not really
a regression, it looks even worse without the patch :D

I tried to add an attachment, with a few examples, but the mail delivery
was rejected.  Therefore putting only the switch-case example inline
into the mail:

js-mode:

function switch_test(a)
{
    switch (a) {
    case 1:
        x;
        break;
    case 2:
        y;
        break;
    default:
        z;
        break;
    }

    switch (a)
    {
        case 1:
        x;
        break;
        case 2:
        y;
        break;
        default:
        z;
        break;
    }
}

js-ts-mode:

function switch_test(a)
{
    switch (a) {
    case 1:
        x;
        break;
    case 2:
        y;
        break;
    default:
        z;
        break;
    }

    switch (a)
{
case 1:
    x;
    break;
case 2:
    y;
    break;
default:
    z;
    break;
}
}

Hope this works now.  If you want more examples, I could put them in a
github gist, but the rest is not always exactly the same in both modes,
but imo fine either way.

Fabian

Vincenzo Pupillo <[email-masked]> writes:

> Thank you Fabian. 
> This new patch also fixes this problem in my tests. Can you please try it in 
> your use cases?
> There would be some other things to fix as well, but maybe it would be better 
> to do it in emacs 31. What do you think Eli?
>
> Vincenzo
>
> In data domenica 23 marzo 2025 20:20:56 Ora standard dell’Europa centrale, 
> Fabian Brosda ha scritto:
>> Hi Vincenzo,
>> 
>> all right, thanks for taking care of it.  Just in case you don't already
>> know and maybe interesting, if the indentation should in fact be exactly
>> the same as in js-mode, I noticed, that your arrow function example is
>> indented differently.
>> 
>> In js-mode:
>> 
>> const Geek = (a, b) =>
>>       {
>> 	      return (a + " " + b);
>>       }
>> 
>> 
>> In js-ts-mode:
>> 
>> const Geek = (a, b) =>
>>     {
>> 	    return (a + " " + b);
>>     }
>> 
>> js-indent-level is set to 4 in both cases. Looks like js-mode in general
>> aligns the part of a variable declaration, with the start of the
>> variable if you add a line break somewhere.
>> 
>> Fabian
>> 
>> Vincenzo Pupillo <[email-masked]> writes:
>> > Ciao Fabian, thank you.
>> > In fact, there are other cases where it does not work, and especially the
>> > behavior is different from js-mode. I need to think about this a bit
>> > more..
>> > 
>> > Vincenzo
>> > 
>> > In data domenica 23 marzo 2025 11:18:02 Ora standard dell’Europa centrale,
>> > 
>> > Fabian Brosda ha scritto:
>> >> Hi,
>> >> thanks for the explanation and the patch.  The result from the patch is
>> >> a GNU-style indentation, where only the braces for functions are not
>> >> indented, while braces for other blocks (if/else, loops, arrow
>> >> functions) are indented.
>> >> 
>> >> What I would like to have is Allman style indentation, where the braces
>> >> for blocks like if/else indent the same as for functions.  This also
>> >> seems the default behavior in the old js-mode.  In this case I think it
>> >> would be correct to not indent the brace for the arrow function.  For
>> >> reference, I found an issue in eslint regarding this indentation:
>> >> 
>> >> https://github.com/eslint/eslint/issues/8493
>> >> 
>> >> Would it be possible to also support the later?
>> >> 
>> >> Fabian
>> >> 
>> >> Vincenzo Pupillo <[email-masked]> writes:
>> >> > Ciao,
>> >> > a rule like this:
>> >> > ((node-is "statement_block") parent-bol 0)
>> >> > 
>> >> > works for function but brakes the indentation of expressione like this:
>> >> > const Geek = (a, b) =>
>> >> > 
>> >> >   {
>> >> > 	
>> >> > 	return (a + " " + b);
>> >> > 	
>> >> >   }
>> >> > 
>> >> > In the attached patch, I have added a new rule specifically for this
>> >> > case.
>> >> > 
>> >> > Vincenzo
>> >> > 
>> >> > In data sabato 22 marzo 2025 13:24:16 Ora standard dell’Europa
>> >> > centrale,
>> >> > Eli>
>> >> > 
>> >> > Zaretskii ha scritto:
>> >> >> Ping!  Juri and Vincenzo, please chime in.
>> >> >> 
>> >> >> > Cc: [email-masked]
>> >> >> > Date: Sun, 09 Mar 2025 12:00:16 +0200
>> >> >> > From: Eli Zaretskii <[email-masked]>
>> >> >> > 
>> >> >> > > Date: Sun, 02 Mar 2025 20:44:39 +0100
>> >> >> > > From:  Fabian Brosda via "Bug reports for GNU Emacs,
>> >> >> > > 
>> >> >> > >  the Swiss army knife of text editors" <[email-masked]>
>> >> >> > > 
>> >> >> > > Hi,
>> >> >> > > 
>> >> >> > > when using js-ts-mode instead of js-mode, braces, which are put on
>> >> >> > > a
>> >> >> > > separate line are indented one level too much.  Here is a simple
>> >> >> > > example
>> >> >> > > of how the indentation looks like, after using indent-region on
>> >> >> > > the
>> >> >> > > whole function:
>> >> >> > > 
>> >> >> > > ```
>> >> >> > > function test(x)
>> >> >> > > 
>> >> >> > >     {
>> >> >> > >     
>> >> >> > > 	    if(x)
>> >> >> > > 	    
>> >> >> > > 	        {
>> >> >> > > 	        
>> >> >> > > 		        return a;
>> >> >> > > 	        
>> >> >> > > 	        }
>> >> >> > > 	    
>> >> >> > > 	    else
>> >> >> > > 	    
>> >> >> > > 	        {
>> >> >> > > 	        
>> >> >> > > 		        return b;
>> >> >> > > 	        
>> >> >> > > 	        }
>> >> >> > >     
>> >> >> > >     }
>> >> >> > > 
>> >> >> > > ```
>> >> >> > > 
>> >> >> > > The used tree-sitter grammar is downloaded from
>> >> >> > > https://github.com/tree-sitter/tree-sitter-javascript.
>> >> >> > > 
>> >> >> > > Having braces on a separate line is probably not the most common
>> >> >> > > for
>> >> >> > > javascript, but using indent-region in the js-mode, does yield the
>> >> >> > > expected result.  The js.el file does already contain a comment
>> >> >> > > mentioning braces in js--treesit-indent-rules.  But even based on
>> >> >> > > the
>> >> >> > > git history it is not clear to me, what exactly is meant:
>> >> >> > > 
>> >> >> > > ```
>> >> >> > > 
>> >> >> > >        ;; "{" on the newline.
>> >> >> > >        ((node-is "statement_block") parent-bol js-indent-level)
>> >> >> > > 
>> >> >> > > ```
>> >> >> > > 
>> >> >> > > Link:
>> >> >> > > https://github.com/emacs-mirror/emacs/blob/master/lisp/progmodes/j
>> >> >> > > s.e
>> >> >> > > l#
>> >> >> > > L3457
>> >> >> > > 
>> >> >> > > If I replace 'js-indent-level' with '0' in this line, the
>> >> >> > > indentation
>> >> >> > > would
>> >> >> > > be correct, but this might have unwanted side-effects I
>> >> >> > > overlooked.
>> >> >> > > 
>> >> >> > > My current emacs version:
>> >> >> > > 
>> >> >> > > GNU Emacs 30.1 (build 2, x86_64-pc-linux-gnu, GTK+ Version
>> >> >> > > 3.24.48,
>> >> >> > > cairo version 1.18.2)
>> >> >> > > 
>> >> >> > > Thanks for looking into this.
>> >> >> > 
>> >> >> > Juri and Vincenzo, any comments?
>> >> > 
>> >> > From 56f0bfe99b0c4cda1e36686e48c58d0d017bda21 Mon Sep 17 00:00:00 2001
>> >> > From: Vincenzo Pupillo <[email-masked]>
>> >> > Date: Sat, 22 Mar 2025 23:43:52 +0100
>> >> > Subject: [PATCH] Fix indentation of "{" when on a new line of a
>> >> > function
>> >> > 
>> >> >  declaration.
>> >> > 
>> >> > * lisp/progmodes/js.el
>> >> > (js--treesit-indent-rules): New rule for the indentation of "{" when
>> >> > of a new line of a function declaration. (bug#76704)
>> >> > ---
>> >> > 
>> >> >  lisp/progmodes/js.el | 5 ++++-
>> >> >  1 file changed, 4 insertions(+), 1 deletion(-)
>> >> > 
>> >> > diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
>> >> > index 3e789218fde..6943ffc2d28 100644
>> >> > --- a/lisp/progmodes/js.el
>> >> > +++ b/lisp/progmodes/js.el
>> >> > @@ -3464,7 +3464,10 @@ js--treesit-indent-rules
>> >> > 
>> >> >         ((parent-is "ternary_expression") parent-bol js-indent-level)
>> >> >         ((parent-is "member_expression") parent-bol js-indent-level)
>> >> >         ((node-is ,switch-case) parent-bol 0)
>> >> > 
>> >> > -       ;; "{" on the newline.
>> >> > +       ;; "{" on the newline for functions.
>> >> > +       ((query "(function_declaration body: (statement_block \"{\")
>> >> > @indent)") +         parent-bol 0)
>> >> > +       ;; "{" on the newline in other cases.
>> >> > 
>> >> >         ((node-is "statement_block") parent-bol js-indent-level)
>> >> >         ((parent-is "named_imports") parent-bol js-indent-level)
>> >> >         ((parent-is "statement_block") parent-bol js-indent-level)
>
> From 2d715869962c3720c80ec3009adade5dd3c4c8b1 Mon Sep 17 00:00:00 2001
> From: Vincenzo Pupillo <[email-masked]>
> Date: Sat, 22 Mar 2025 23:43:52 +0100
> Subject: [PATCH] Fix indentation of "{" when on a new line of a function
>  declaration.
>
> * lisp/progmodes/js.el
> (js--treesit-indent-rules): Fix rule for the indentation of "{" when
> of a new line of a function declaration. (bug#76704)
> Fix other related rules.
> ---
>  lisp/progmodes/js.el | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index 3e789218fde..802d74103df 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -3454,7 +3454,7 @@ js--treesit-indent-rules
>    (let ((switch-case (rx "switch_" (or "case" "default"))))
>      `((javascript
>         ((parent-is "program") parent-bol 0)
> -       ((node-is "}") parent-bol 0)
> +       ((node-is "}") standalone-parent 0)
>         ((node-is ")") parent-bol 0)
>         ((node-is "]") parent-bol 0)
>         ((node-is ">") parent-bol 0)
> @@ -3465,9 +3465,9 @@ js--treesit-indent-rules
>         ((parent-is "member_expression") parent-bol js-indent-level)
>         ((node-is ,switch-case) parent-bol 0)
>         ;; "{" on the newline.
> -       ((node-is "statement_block") parent-bol js-indent-level)
> +       ((node-is "statement_block") parent-bol 0)
>         ((parent-is "named_imports") parent-bol js-indent-level)
> -       ((parent-is "statement_block") parent-bol js-indent-level)
> +       ((parent-is "statement_block") standalone-parent js-indent-level)
>         ((parent-is "variable_declarator") parent-bol js-indent-level)
>         ((parent-is "arguments") parent-bol js-indent-level)
>         ((parent-is "array") parent-bol js-indent-level)
> @@ -3477,12 +3477,11 @@ js--treesit-indent-rules
>         ((parent-is "object_pattern") parent-bol js-indent-level)
>         ((parent-is "object") parent-bol js-indent-level)
>         ((parent-is "pair") parent-bol js-indent-level)
> -       ((parent-is "arrow_function") parent-bol js-indent-level)
> +       ((parent-is "arrow_function") grand-parent 0)
>         ((parent-is "parenthesized_expression") parent-bol js-indent-level)
>         ((parent-is "binary_expression") parent-bol js-indent-level)
>         ((parent-is "class_body") parent-bol js-indent-level)
>         ((parent-is ,switch-case) parent-bol js-indent-level)
> -       ((parent-is "statement_block") parent-bol js-indent-level)
>         ((match "while" "do_statement") parent-bol 0)
>         ((match "else" "if_statement") parent-bol 0)
>         ((parent-is ,(rx (or (seq (or "if" "for" "for_in" "while" "do") "_statement")
> -- 
> 2.49.0