Package: emacs;
Reported by: Fabian Brosda <f.brosda <at> gmx.de>
Date: Mon, 3 Mar 2025 04:33:03 UTC
Severity: minor
Found in version 30.1
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
Message #34 received at 76704 <at> debbugs.gnu.org (full text, mbox):
From: Vincenzo Pupillo <v.pupillo <at> gmail.com> To: juri <at> linkov.net, Eli Zaretskii <eliz <at> gnu.org>, Fabian Brosda <f.brosda <at> gmx.de> Cc: 76704 <at> debbugs.gnu.org Subject: Re: bug#76704: 30.1; Indentation of braces on separate line in js-ts-mode Date: Fri, 28 Mar 2025 10:53:01 +0100
Ciao Fabian,
In data giovedì 27 marzo 2025 19:59:33 Ora standard dell’Europa centrale,
Fabian Brosda ha scritto:
> 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.
Could you please show me this difference?
>
> 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.
Thanks Fabian, more complex examples would be very helpful. I used the React
sources and found more problems.
Vincenzo
>
> Fabian
>
> Vincenzo Pupillo <v.pupillo <at> gmail.com> 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 <v.pupillo <at> gmail.com> 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 <v.pupillo <at> gmail.com> 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: 76704 <at> debbugs.gnu.org
> >> >> >> > Date: Sun, 09 Mar 2025 12:00:16 +0200
> >> >> >> > From: Eli Zaretskii <eliz <at> gnu.org>
> >> >> >> >
> >> >> >> > > 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" <bug-gnu-emacs <at> gnu.org>
> >> >> >> > >
> >> >> >> > > 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/progmode
> >> >> >> > > s/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 <v.pupillo <at> gmail.com>
> >> >> > 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 <v.pupillo <at> gmail.com>
> > 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
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.