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

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

[Marcin Borkowski]

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

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

[Marcin Borkowski]

On 2020-04-22, at 11:05, Marcin Borkowski <mbork@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

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

[Lars Ingebrigtsen]

Marcin Borkowski <mbork@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@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

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

[Marcin Borkowski]

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

> Marcin Borkowski <mbork@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@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

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

[Dmitry Gutov]

On 14.03.2022 12:13, Marcin Borkowski wrote:
> 
> On 2022-03-14, at 10:40, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
>> Marcin Borkowski <mbork@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@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.

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

[Marcin Borkowski]

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

> On 14.03.2022 12:13, Marcin Borkowski wrote:
>> On 2022-03-14, at 10:40, Lars Ingebrigtsen <larsi@gnus.org> wrote:
>> 
>>> Marcin Borkowski <mbork@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@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

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

[Dmitry Gutov]

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.