GNU bug report logs - #40760
27.0.50; An indentation problem with const and chaining in js-mode

Previous Next

To reply to this bug, email your comments to 40760 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; An indentation problem with const and chaining in js-mode
Date: Wed, 22 Apr 2020 11:05:08 +0200

When declaring a const variable which is assigned a value of a long,
chained expression, the default indentation is wrong (compared to a let
declaration):

let a = /regex/
    .test('regex hello');

const a = /regex/
      .test('regex hello');

I would expect (and prefer) this:

let a = /regex/
    .test('regex hello');

const a = /regex/
    .test('regex hello');
    
(checked on emacs -Q)

TIA,
mb



In GNU Emacs 27.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.8)
 of 2019-06-15 built on tars
Repository revision: f6a1647a8be9148b8db76aca601773968af7c343
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Arch Linux

--
Marcin Borkowski
http://mbork.pl

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: bug-gnu-emacs <at> gnu.org
Cc: mbork <at> mbork.pl
Subject: Re: 27.0.50; An indentation problem with const and chaining in js-mode
Date: Mon, 14 Mar 2022 06:10:44 +0100

On 2020-04-22, at 11:05, Marcin Borkowski <mbork <at> mbork.pl> wrote:

> When declaring a const variable which is assigned a value of a long,
> chained expression, the default indentation is wrong (compared to a let
> declaration):
>
> let a = /regex/
>     .test('regex hello');
>
> const a = /regex/
>       .test('regex hello');
>
> I would expect (and prefer) this:
>
> let a = /regex/
>     .test('regex hello');
>
> const a = /regex/
>     .test('regex hello');
>     
> (checked on emacs -Q)

This is the temporary solution I employed:

(setq js--declaration-keyword-re "\\<\\(let\\|var\\)\\>")

I would suggest turning this variable into a user option.

Best,

PS. Please CC me in any replies, I am not subscribed to bug-gnu-emacs.

-- 
Marcin Borkowski
http://mbork.pl

Message #11 received at 40760 <at> debbugs.gnu.org (full text, mbox):

From: Lars Ingebrigtsen <larsi <at> gnus.org>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: 40760 <at> debbugs.gnu.org
Subject: Re: bug#40760: 27.0.50; An indentation problem with const and
 chaining in js-mode
Date: Mon, 14 Mar 2022 10:40:01 +0100

Marcin Borkowski <mbork <at> mbork.pl> writes:

> When declaring a const variable which is assigned a value of a long,
> chained expression, the default indentation is wrong (compared to a let
> declaration):
>
> let a = /regex/
>     .test('regex hello');
>
> const a = /regex/
>       .test('regex hello');

I think this is the intended indentation?  That is, they indent to where
the "a" is.

Marcin Borkowski <mbork <at> mbork.pl> writes:

> This is the temporary solution I employed:
>
> (setq js--declaration-keyword-re "\\<\\(let\\|var\\)\\>")
>
> I would suggest turning this variable into a user option.

This isn't just used for indentation, so altering this const will lead
to other breakages (and so it shouldn't be customiseable, either).

It looks to me like everything here is working as intended (but you're
free to prefer other indentation methods if you want, of course), but I
don't think there's much to do on the Emacs side here.  Anybody else
got an opinion? 

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Message #16 received at 40760 <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 40760 <at> debbugs.gnu.org, mbork <at> mbork.pl
Subject: Re: bug#40760: 27.0.50; An indentation problem with const and
 chaining in js-mode
Date: Mon, 14 Mar 2022 11:13:49 +0100

On 2022-03-14, at 10:40, Lars Ingebrigtsen <larsi <at> gnus.org> wrote:

> Marcin Borkowski <mbork <at> mbork.pl> writes:
>
>> When declaring a const variable which is assigned a value of a long,
>> chained expression, the default indentation is wrong (compared to a let
>> declaration):
>>
>> let a = /regex/
>>     .test('regex hello');
>>
>> const a = /regex/
>>       .test('regex hello');
>
> I think this is the intended indentation?  That is, they indent to where
> the "a" is.

Well, in a tab-only indentation style (used by many people, me included)
this is _very_ wrong, e.g. because it results in Emacs using both tabs
and spaces here.

> Marcin Borkowski <mbork <at> mbork.pl> writes:
>
>> This is the temporary solution I employed:
>>
>> (setq js--declaration-keyword-re "\\<\\(let\\|var\\)\\>")
>>
>> I would suggest turning this variable into a user option.
>
> This isn't just used for indentation, so altering this const will lead
> to other breakages (and so it shouldn't be customiseable, either).

Grep apparently disagrees - I found 5 occurrences of
`js--declaration-keyword-re' in Emacs sources, and all of them seem to
be related to indentation.  So, I don't see any danger here.  (Anyway,
I changed it in my init.el; we'll see how that works.)

Best,

-- 
Marcin Borkowski
http://mbork.pl

Message #19 received at 40760 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Marcin Borkowski <mbork <at> mbork.pl>, Lars Ingebrigtsen <larsi <at> gnus.org>
Cc: 40760 <at> debbugs.gnu.org
Subject: Re: bug#40760: 27.0.50; An indentation problem with const and
 chaining in js-mode
Date: Fri, 18 Mar 2022 03:12:01 +0200

On 14.03.2022 12:13, Marcin Borkowski wrote:
> 
> On 2022-03-14, at 10:40, Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
> 
>> Marcin Borkowski <mbork <at> mbork.pl> writes:
>>
>>> When declaring a const variable which is assigned a value of a long,
>>> chained expression, the default indentation is wrong (compared to a let
>>> declaration):
>>>
>>> let a = /regex/
>>>      .test('regex hello');
>>>
>>> const a = /regex/
>>>        .test('regex hello');
>>
>> I think this is the intended indentation?  That is, they indent to where
>> the "a" is.
> 
> Well, in a tab-only indentation style (used by many people, me included)
> this is _very_ wrong, e.g. because it results in Emacs using both tabs
> and spaces here.

I'm fairly certain it's not a very popular style, but we should try to 
cater to it as well, of course.

>> Marcin Borkowski <mbork <at> mbork.pl> writes:
>>
>>> This is the temporary solution I employed:
>>>
>>> (setq js--declaration-keyword-re "\\<\\(let\\|var\\)\\>")
>>>
>>> I would suggest turning this variable into a user option.
>>
>> This isn't just used for indentation, so altering this const will lead
>> to other breakages (and so it shouldn't be customiseable, either).
> 
> Grep apparently disagrees - I found 5 occurrences of
> `js--declaration-keyword-re' in Emacs sources, and all of them seem to
> be related to indentation.  So, I don't see any danger here.  (Anyway,
> I changed it in my init.el; we'll see how that works.)

I think it would be better to add a more semantically-named user option.

This indentation feature was ported from js2-mode at some point, where 
it is guarded by the (on by default) user option 
js2-pretty-multiline-declarations. The option itself was lost in transition.

See js2-old-indent.el for more info.

Message #22 received at 40760 <at> debbugs.gnu.org (full text, mbox):

From: Marcin Borkowski <mbork <at> mbork.pl>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 40760 <at> debbugs.gnu.org
Subject: Re: bug#40760: 27.0.50; An indentation problem with const and
 chaining in js-mode
Date: Mon, 21 Mar 2022 07:26:00 +0100

On 2022-03-18, at 02:12, Dmitry Gutov <dgutov <at> yandex.ru> wrote:

> On 14.03.2022 12:13, Marcin Borkowski wrote:
>> On 2022-03-14, at 10:40, Lars Ingebrigtsen <larsi <at> gnus.org> wrote:
>> 
>>> Marcin Borkowski <mbork <at> mbork.pl> writes:
>>>
>>>> When declaring a const variable which is assigned a value of a long,
>>>> chained expression, the default indentation is wrong (compared to a let
>>>> declaration):
>>>>
>>>> let a = /regex/
>>>>      .test('regex hello');
>>>>
>>>> const a = /regex/
>>>>        .test('regex hello');
>>>
>>> I think this is the intended indentation?  That is, they indent to where
>>> the "a" is.
>> Well, in a tab-only indentation style (used by many people, me
>> included)
>> this is _very_ wrong, e.g. because it results in Emacs using both tabs
>> and spaces here.
>
> I'm fairly certain it's not a very popular style, but we should try to
> cater to it as well, of course.

Interesting - I thought using spaces for indentation is a no-no nowadays
(at least in JS, Lisp is another thing, for obvious reasons).  But I may
be mistaken, and I don't think tabs are inherently better - though we do
use them in our company.

>
>>> Marcin Borkowski <mbork <at> mbork.pl> writes:
>>>
>>>> This is the temporary solution I employed:
>>>>
>>>> (setq js--declaration-keyword-re "\\<\\(let\\|var\\)\\>")
>>>>
>>>> I would suggest turning this variable into a user option.
>>>
>>> This isn't just used for indentation, so altering this const will lead
>>> to other breakages (and so it shouldn't be customiseable, either).
>> Grep apparently disagrees - I found 5 occurrences of
>> `js--declaration-keyword-re' in Emacs sources, and all of them seem to
>> be related to indentation.  So, I don't see any danger here.  (Anyway,
>> I changed it in my init.el; we'll see how that works.)
>
> I think it would be better to add a more semantically-named user option.

Definitely, the name 

>
> This indentation feature was ported from js2-mode at some point, where
> it is guarded by the (on by default) user option
> js2-pretty-multiline-declarations. The option itself was lost in
> transition.
>
> See js2-old-indent.el for more info.

Very interesting.  FWIW, I almost never have many variables in a single
let/const - I prefer to write

let a = 1;
let b = 2;
const c = 3;
const d = 4;

(and this also is a style I learned where I work).

Best,


-- 
Marcin Borkowski
http://mbork.pl

Message #25 received at 40760 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Marcin Borkowski <mbork <at> mbork.pl>
Cc: Lars Ingebrigtsen <larsi <at> gnus.org>, 40760 <at> debbugs.gnu.org
Subject: Re: bug#40760: 27.0.50; An indentation problem with const and
 chaining in js-mode
Date: Wed, 23 Mar 2022 02:46:45 +0200

On 21.03.2022 08:26, Marcin Borkowski wrote:

> Interesting - I thought using spaces for indentation is a no-no nowadays
> (at least in JS, Lisp is another thing, for obvious reasons).  But I may
> be mistaken, and I don't think tabs are inherently better - though we do
> use them in our company.

There are a bunch of odd styles in use in JS. For example, comma-first 
indentation, where you don't put commas at the end of a line, and 
instead add newline and indentation before them.

But space-based indentation still seems prevalent.

Anyway...

>> This indentation feature was ported from js2-mode at some point, where
>> it is guarded by the (on by default) user option
>> js2-pretty-multiline-declarations. The option itself was lost in
>> transition.
>>
>> See js2-old-indent.el for more info.
> 
> Very interesting.  FWIW, I almost never have many variables in a single
> let/const - I prefer to write
> 
> let a = 1;
> let b = 2;
> const c = 3;
> const d = 4;
> 
> (and this also is a style I learned where I work).

At some point support for multi-var combined declarations was requested 
for, that's when I added that var.

So, the code is out there, it shouldn't be hard to adapt to js-mode, if 
you have the time.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.