bug#25904: Formatting bug with js-mode

bug#25904: Formatting bug with js-mode

[Michael Snead]

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

Hi there. Apologies if this is the wrong place to submit a bug on js-mode
for emacs. I am an emacs newbie (in fact, I've only used spacemacs and
still relatively new there too) and not sure where to post about this.

Basically I am trying to find the right place to report this issue, which
was determined to be an upstream issue with js-mode:

https://github.com/felipeochoa/rjsx-mode/issues/34

In essence, a fat arrow expression followed by a linefeed, followed by an
expression (such as a JSX tag with children) that is not wrapped in
parenthesis is valid syntax and recognized by editors like WebStorm and
VSCode but doesn't work correctly in this editor.

If this is not the right place I would be grateful for directions on where
this bug belongs.

Thanks!

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

bug#25904: Formatting bug with js-mode

[Felipe Ochoa]

Here's an initial implementation of a solution for this 
issue. (Also filed as js2#314 [1])

foo.bar.baz(very => 
  very  // current code aligns return value to paren above 
).biz(baz => 
  baz
);

I've been using this code successfully for a number of days now, 
but since it's my first time touching indentation code, it likely 
needs review for I comments and whitespace handling, etc.

(Note: disabling `js-indent-align-list-continuation' also 
addresses this issue, but disabling it changes indentation in 
other parts.)

[1]: https://github.com/mooz/js2-mode/issues/314

---
 lisp/progmodes/js.el | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 1f86909..18f888d 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1848,7 +1848,9 @@ This performs fontification according to `js--class-styles'."
              (skip-chars-backward " \t")
              (or (bobp) (backward-char))
              (and (> (point) (point-min))
-                  (save-excursion (backward-char) (not (looking-at "[/*]/")))
+                  ;; Need to exclude => here since js--looking-at-operator-p thinks
+                  ;; it's looking at an assignment operator
+                  (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>")))
                   (js--looking-at-operator-p)
                   (and (progn (backward-char)
                               (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))
@@ -2107,7 +2109,18 @@ indentation is aligned to that column."
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
              (if (or (not js-indent-align-list-continuation)
-                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+                     ;; check for a arrow function without parens
+                     (and (looking-at "(\\s-*\\(async\\s-*\\)?")
+                          ;; TODO: should call (forward-comment most-positive-fixnum)?
+                          (save-excursion
+                            (goto-char (match-end 0))
+                            (cond
+                             ((eq (char-after) ?\()
+                              (forward-sexp)
+                              (looking-at-p "\\s-*=>\\s-*\\(/[/*]\\|$\\)"))
+                             (t (looking-at-p
+                                 (concat js--name-re "\\s-*=>\\s-*\\(/[/*]\\|$\\)")))))))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))
--
2.7.4

bug#25904: Formatting bug with js-mode

[Dmitry Gutov]

Hi Felipe!

On 11/8/17 1:55 AM, Felipe Ochoa wrote:
> Here's an initial implementation of a solution for this issue. (Also 
> filed as js2#314 [1])
> 
> foo.bar.baz(very =>  very  // current code aligns return value to paren 
> above ).biz(baz =>  baz
> );
> 
> I've been using this code successfully for a number of days now, but 
> since it's my first time touching indentation code, it likely needs 
> review for I comments and whitespace handling, etc.

I think the code is reasonable, but the patch needs a few examples for 
emacs/test/manual/js*.js. Maybe add a new file, or maybe reuse an 
existing one.

It also wouldn't hurt that the existing examples are unchanged with the 
new code.

>               (skip-chars-backward " \t")
>               (or (bobp) (backward-char))
>               (and (> (point) (point-min))
> -                  (save-excursion (backward-char) (not (looking-at 
> "[/*]/")))
> +                  ;; Need to exclude => here since 
> js--looking-at-operator-p thinks
> +                  ;; it's looking at an assignment operator
> +                  (save-excursion (backward-char) (not (looking-at 
> "[/*]/\\|=>")))

OK.

> @@ -2107,7 +2109,18 @@ indentation is aligned to that column."
>                   (continued-expr-p (js--continued-expression-p)))
>               (goto-char (nth 1 parse-status)) ; go to the opening char
>               (if (or (not js-indent-align-list-continuation)
> -                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
> +                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
> +                     ;; check for a arrow function without parens
> +                     (and (looking-at "(\\s-*\\(async\\s-*\\)?")
> +                          ;; TODO: should call (forward-comment 
> most-positive-fixnum)?

To allow comments between the opening paren and the arglist? Does 
anybody write them there?

> +                          (save-excursion
> +                            (goto-char (match-end 0))
> +                            (cond
> +                             ((eq (char-after) ?\()
> +                              (forward-sexp)
> +                              (looking-at-p 
> "\\s-*=>\\s-*\\(/[/*]\\|$\\)"))
> +                             (t (looking-at-p
> +                                 (concat js--name-re 
> "\\s-*=>\\s-*\\(/[/*]\\|$\\)")))))))

I imagine this (*) could be transformed into a single regexp, though it 
would be pretty complex (rx could help, though!).

(*) Looking at an opening paren followed by ([optional] lambda arglist 
and an arrow) then [optional] comment and newline.

Arglist matcher might be on the big side, but I'm guessing the 
performance will be better. Not sure how big of an issue this is.

If it's not one regexp, moving some of the new code into a helper 
function (with a sensible name) seems prudent.

bug#25904: Formatting bug with js-mode

[Felipe Ochoa]

Hi Dmitry!

> I think the code is reasonable, but the patch needs a few 
> examples for emacs/test/manual/js*.js. Maybe add a new file, or 
> maybe reuse an existing one.

I've added a test to js.js in the latest patch below. 
 
> It also wouldn't hurt that the existing examples are unchanged 
> with the new code.

I checked all the existing examples manually and all were 
unchanged when applying indent-region on the entire buffer.

> To allow comments between the opening paren and the arglist? 
> Does anybody write them there?

Oops -- this comment was supposed to be after the arrow. I was 
thinking of return type annotation comments, but I just checked 
flow and I don't think they support this. We can just remove the 
comment

> I imagine this (*) could be transformed into a single regexp, 
> though it would be pretty complex (rx could help, though!).
>
> (*) Looking at an opening paren followed by ([optional] lambda 
> arglist and an arrow) then [optional] comment and newline.

Is there an arglist regexp already in use somewhere? I'd rather 
not roll my own since dealing with default argument values and 
spreads and such seems quite challenging.

> If it's not one regexp, moving some of the new code into a 
> helper function (with a sensible name) seems prudent.

I've done this in the latest patch.

---
 lisp/progmodes/js.el     | 26 ++++++++++++++++++++++++--
 test/manual/indent/js.js |  9 +++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 1f86909..6e057b0 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1848,7 +1848,9 @@ This performs fontification according to `js--class-styles'."
              (skip-chars-backward " \t")
              (or (bobp) (backward-char))
              (and (> (point) (point-min))
-                  (save-excursion (backward-char) (not (looking-at "[/*]/")))
+                  ;; Need to exclude => here since js--looking-at-operator-p thinks
+                  ;; it's looking at an assignment operator
+                  (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>")))
                   (js--looking-at-operator-p)
                   (and (progn (backward-char)
                               (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))
@@ -2077,6 +2079,21 @@ indentation is aligned to that column."
         (when comma-p
           (goto-char (1+ declaration-keyword-end))))))))

+(defun js--looking-at-broken-arrow-function-p ()
+  "Helper function for `js--proper-indentation'.
+Return t if point is at the start of a (possibly async) arrow
+function and the last non-comment, non-whitespace token of the
+current like is the \"=>\" token."
+  (and (looking-at "\\s-*\\(async\\s-*\\)?")
+       (save-excursion
+         (goto-char (match-end 0))
+         (cond
+          ((eq (char-after) ?\()
+           (forward-list) ; Is this better than forward-sexp?
+           (looking-at-p "\\s-*=>\\s-*\\(/[/*]\\|$\\)"))
+          (t (looking-at-p
+              (concat js--name-re "\\s-*=>\\s-*\\(/[/*]\\|$\\)")))))))
+
 (defun js--proper-indentation (parse-status)
   "Return the proper indentation for the current line."
   (save-excursion
@@ -2107,7 +2124,12 @@ indentation is aligned to that column."
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
              (if (or (not js-indent-align-list-continuation)
-                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+                     (when (eq (char-after) ?\()
+                       (save-excursion
+                         (forward-char)
+                         (skip-syntax-forward " ")
+                         (js--looking-at-broken-arrow-function-p))))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))
diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
index b0d8bca..2286cc1 100644
--- a/test/manual/indent/js.js
+++ b/test/manual/indent/js.js
@@ -144,6 +144,15 @@ bar(
   /abc/
 )

+// #25904
+foo.bar.baz(very =>
+  very
+).biz(([baz={a: [123]}, boz]) =>
+  baz
+).snarf((snorf) =>
+  snorf
+);
+
 // Local Variables:
 // indent-tabs-mode: nil
 // js-indent-level: 2
--
2.7.4

bug#25904: Formatting bug with js-mode

[Dmitry Gutov]

On 11/21/17 12:22 AM, Felipe Ochoa wrote:

> I've added a test to js.js in the latest patch below.

Thanks.

>> To allow comments between the opening paren and the arglist? Does 
>> anybody write them there?
> 
> Oops -- this comment was supposed to be after the arrow. I was thinking 
> of return type annotation comments, but I just checked flow and I don't 
> think they support this. We can just remove the comment

Sure.

> Is there an arglist regexp already in use somewhere? I'd rather not roll 
> my own since dealing with default argument values and spreads and such 
> seems quite challenging.

No, sorry, I forgot about destructuring and etc. forward-list is fine here.

>> If it's not one regexp, moving some of the new code into a helper 
>> function (with a sensible name) seems prudent.
> 
> I've done this in the latest patch.

Thanks.

> +(defun js--looking-at-broken-arrow-function-p ()
> +  "Helper function for `js--proper-indentation'.
> +Return t if point is at the start of a (possibly async) arrow
> +function and the last non-comment, non-whitespace token of the
> +current like is the \"=>\" token."
> +  (and (looking-at "\\s-*\\(async\\s-*\\)?")

This looks weird: the regexp will always match. Better to write it as

(if (looking-at NON-OPT-REGEXP) (goto-char ...)), where NON-OPT-REGEXP 
does not include the optional qualifier (?).

And the (save-excursion...) form seems unnecessary, since the caller 
already uses it.

> +       (save-excursion
> +         (goto-char (match-end 0))
> +         (cond
> +          ((eq (char-after) ?\()
> +           (forward-list) ; Is this better than forward-sexp?

I think so: it should be a bit faster, and it doesn't require you to 
bind forward-sexp-function to nil (for e.g. js2-mode).

> +           (looking-at-p "\\s-*=>\\s-*\\(/[/*]\\|$\\)"))
> +          (t (looking-at-p
> +              (concat js--name-re "\\s-*=>\\s-*\\(/[/*]\\|$\\)")))))))

Should we extract "\\s-*=>\\s-*\\(/[/*]\\|$\\)" to a local variable, or 
a constant?

> (defun js--proper-indentation (parse-status)
>    "Return the proper indentation for the current line."
>    (save-excursion
> @@ -2107,7 +2124,12 @@ indentation is aligned to that column."
>                   (continued-expr-p (js--continued-expression-p)))
>               (goto-char (nth 1 parse-status)) ; go to the opening char
>               (if (or (not js-indent-align-list-continuation)
> -                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
> +                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
> +                     (when (eq (char-after) ?\()
> +                       (save-excursion
> +                         (forward-char)
> +                         (skip-syntax-forward " ")

This seems a bit extraneous: the regexps in the called function all 
start with "\\s-*", right? Let's maybe have the one or the other.

> +                         (js--looking-at-broken-arrow-function-p))))
>                   (progn ; nothing following the opening paren/bracket
>                     (skip-syntax-backward " ")
>                     (when (eq (char-before) ?\)) (backward-list))
> diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
> index b0d8bca..2286cc1 100644
> --- a/test/manual/indent/js.js
> +++ b/test/manual/indent/js.js
> @@ -144,6 +144,15 @@ bar(
>    /abc/
> )
> 
> +// #25904

We write references to debbugs as bug#25904. bug-reference-mode can 
linkify the result.

> +foo.bar.baz(very =>
> +  very
> +).biz(([baz={a: [123]}, boz]) =>
> +  baz
> +).snarf((snorf) =>
> +  snorf
> +);
> +

Thank you.

Please attach the resulting patch as a file produced by 'git 
format-patch', with a ChangeLog style message entry, as described in 
CONTRIBUTE.

bug#25904: Formatting bug with js-mode

[Felipe Ochoa]

This has taken a *little* longer than I'd hoped, but here's a new
patch with all the discussed changes. Hopefully I got the commit
message formatting and content right

From 0c7eed3fd9f7f77a071296c191e8569471bac4e5 Mon Sep 17 00:00:00 2001
From: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
Date: Tue, 7 Nov 2017 16:50:42 -0500
Subject: [PATCH] Fixes js indentation after arrow function without parens

From: felipe <felipe@fov.space>

* lisp/progmodes/js.el (js--proper-indentation)
(js--looking-at-broken-arrow-function-p)
* test/manual/indent/js.js: Indent expression following an arrow as
though it were enclosed in parentheses

(Bug#25904)
---
 lisp/progmodes/js.el     | 24 ++++++++++++++++++++++--
 test/manual/indent/js.js |  9 +++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index f30e591..5a333a9 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1849,7 +1849,9 @@ js--continued-expression-p
              (skip-chars-backward " \t")
              (or (bobp) (backward-char))
              (and (> (point) (point-min))
-                  (save-excursion (backward-char) (not (looking-at "[/*]/")))
+                  ;; Need to exclude => here since
js--looking-at-operator-p thinks
+                  ;; it's looking at an assignment operator
+                  (save-excursion (backward-char) (not (looking-at
"[/*]/\\|=>")))
                   (js--looking-at-operator-p)
                   (and (progn (backward-char)
                               (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))
@@ -2078,6 +2080,23 @@ js--maybe-goto-declaration-keyword-end
         (when comma-p
           (goto-char (1+ declaration-keyword-end))))))))

+(defconst js--line-terminating-arrow "\\s-*=>\\s-*\\(/[/*]\\|$\\)"
+  "Regexp to match a \"=>\" token if it's the last non-whitespace,
non-comment token in a line.")
+
+(defun js--looking-at-broken-arrow-function-p ()
+  "Helper function for `js--proper-indentation'.
+Return t if point is at the start of a (possibly async) arrow
+function and the last non-comment, non-whitespace token of the
+current like is the \"=>\" token."
+  (when (looking-at "\\s-*async\\s-*")
+    (goto-char (match-end 0)))
+  (cond
+   ((eq (char-after) ?\()
+    (forward-list)
+    (looking-at-p js--line-terminating-arrow))
+   (t (looking-at-p
+       (concat js--name-re js--line-terminating-arrow)))))
+
 (defun js--proper-indentation (parse-status)
   "Return the proper indentation for the current line."
   (save-excursion
@@ -2108,7 +2127,8 @@ js--proper-indentation
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
              (if (or (not js-indent-align-list-continuation)
-                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+                     (progn (forward-char)
(js--looking-at-broken-arrow-function-p)))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))
diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
index b0d8bca..fc39c12 100644
--- a/test/manual/indent/js.js
+++ b/test/manual/indent/js.js
@@ -144,6 +144,15 @@ bar(
   /abc/
 )

+// bug#25904
+foo.bar.baz(very => // A comment
+  very
+).biz(([baz={a: [123]}, boz]) =>
+  baz
+).snarf((snorf) => /* Another comment */
+  snorf
+);
+
 // Local Variables:
 // indent-tabs-mode: nil
 // js-indent-level: 2
-- 
2.7.4

bug#25904: bug#24896: JSX prop indentation after fat arrow

[Jimmy Yuen Ho Wong]

If this patch works, can we make it into Emacs 26 before the release?

Thanks a lot for working on this. This has been the most annoying bug in
js-mode.

bug#25904: bug#24896: JSX prop indentation after fat arrow

[Eli Zaretskii]

> From: Jimmy Yuen Ho Wong <wyuenho@gmail.com>
> Date: Sun, 29 Apr 2018 02:20:39 +0100
> Cc: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>,
> 	Dmitry Gutov <dgutov@yandex.ru>
> 
> If this patch works, can we make it into Emacs 26 before the release?

No, it's too late for that.  Sorry.

bug#25904: Formatting bug with js-mode

[Dmitry Gutov]

On 3/16/18 3:06 AM, Felipe Ochoa wrote:
> This has taken a *little* longer than I'd hoped, but here's a new
> patch with all the discussed changes. Hopefully I got the commit
> message formatting and content right

Not really. First of all, I asked for an attached file ('git am' needs a 
file), and not its contents inside the message body. Which suffered 
word-wrap damage during transmission (because that's what email clients do).

I would commit it this time anyway, though, if not for the next point.

>  From 0c7eed3fd9f7f77a071296c191e8569471bac4e5 Mon Sep 17 00:00:00 2001
> From: Felipe Ochoa <felipe.nospam.ochoa@gmail.com>
> Date: Tue, 7 Nov 2017 16:50:42 -0500
> Subject: [PATCH] Fixes js indentation after arrow function without parens
> 
> From: felipe <felipe@fov.space>
> 
> * lisp/progmodes/js.el (js--proper-indentation)
> (js--looking-at-broken-arrow-function-p)

I think there was going to be some description here.

And there's no mention of changes to js--continued-expression-p and 
js--proper-indentation. They would have the meat of the explanation.

> * test/manual/indent/js.js: Indent expression following an arrow as
> though it were enclosed in parentheses

This needs a period, at least.

> (Bug#25904)

We usually put the bug number inside one of the sentences of the change 
log entry, not just at the end of it.

The patch does work, but if you could make the details right, it would 
be great. We've missed the emacs-26 train, so there's no real hurry.

bug#25904: Patches

[Jackson Ray Hamilton]

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

Hello Dmitry and Felipe,

I’ve taken a stab at formatting Felipe’s fix for arrow function indentation according to the coding standards.  Compared to his patch, I made some trivial name/doc changes, and changed

(progn (forward-char) (js--looking-at-broken-arrow-function-p))

to

(save-excursion (forward-char) (js--looking-at-broken-arrow-function-p))

since we probably want to undo the forward-char in case the form returns nil.

In order to get all the JS indentation tests to pass with make, I also had to make a variable safe file-locally.

Both changes are attached.  They could be applied to the emacs-26 branch.  (Or master, but emacs-26 would be preferable so we can deliver it sooner.  I tested on both branches.)

Jackson

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

[-- Attachment #2: 0001-Indent-arrows-expression-bodies-like-function-bodies.patch --]
[-- Type: text/x-patch, Size: 3802 bytes --]

From 383a6aa13f90346b489bc4b8f900fe767a725b90 Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 12:26:21 -0800
Subject: [PATCH] =?UTF-8?q?Indent=20arrows=E2=80=99=20expression=20bodies?=
 =?UTF-8?q?=20like=20function=20bodies=20(Bug#25904)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lisp/progmodes/js.el (js--continued-expression-p): Don’t confuse
‘=>’ for a ‘>’ operator.
(js--line-terminating-arrow-re): New variable.
(js--looking-at-broken-arrow-function-p): New function.
(js--proper-indentation): Don’t align arrow functions’ expression
bodies starting on new lines like list continuations, instead align
them like function bodies (js-indent-align-list-continuation need not
be nil).

* test/manual/indent/js.js: Add test for Bug#25904.

Co-authored-by: Felipe Ochoa <felipe@fov.space>
---
 lisp/progmodes/js.el     | 23 +++++++++++++++++++++--
 test/manual/indent/js.js |  9 +++++++++
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 65ffb0e02f..787afb4e10 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1849,7 +1849,7 @@ js--continued-expression-p
              (skip-chars-backward " \t")
              (or (bobp) (backward-char))
              (and (> (point) (point-min))
-                  (save-excursion (backward-char) (not (looking-at "[/*]/")))
+                  (save-excursion (backward-char) (not (looking-at "[/*]/\\|=>")))
                   (js--looking-at-operator-p)
                   (and (progn (backward-char)
                               (not (looking-at "+\\+\\|--\\|/[/*]"))))))))))
@@ -2078,6 +2078,24 @@ js--maybe-goto-declaration-keyword-end
         (when comma-p
           (goto-char (1+ declaration-keyword-end))))))))
 
+(defconst js--line-terminating-arrow-re "\\s-*=>\\s-*\\(/[/*]\\|$\\)"
+  "Regexp matching the last \"=>\" (arrow) token on a line.
+Whitespace and comments around the arrow are ignored.")
+
+(defun js--looking-at-broken-arrow-function-p ()
+  "Helper function for `js--proper-indentation'.
+Return t if point is at the start of a (possibly async) arrow
+function and the last non-comment, non-whitespace token of the
+current line is the \"=>\" token."
+  (when (looking-at "\\s-*async\\s-*")
+    (goto-char (match-end 0)))
+  (cond
+   ((eq (char-after) ?\()
+    (forward-list)
+    (looking-at-p js--line-terminating-arrow-re))
+   (t (looking-at-p
+       (concat js--name-re js--line-terminating-arrow-re)))))
+
 (defun js--proper-indentation (parse-status)
   "Return the proper indentation for the current line."
   (save-excursion
@@ -2108,7 +2126,8 @@ js--proper-indentation
                  (continued-expr-p (js--continued-expression-p)))
              (goto-char (nth 1 parse-status)) ; go to the opening char
              (if (or (not js-indent-align-list-continuation)
-                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)"))
+                     (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+                     (save-excursion (forward-char) (js--looking-at-broken-arrow-function-p)))
                  (progn ; nothing following the opening paren/bracket
                    (skip-syntax-backward " ")
                    (when (eq (char-before) ?\)) (backward-list))
diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
index b0d8bcabd2..fc39c12244 100644
--- a/test/manual/indent/js.js
+++ b/test/manual/indent/js.js
@@ -144,6 +144,15 @@ bar(
   /abc/
 )
 
+// bug#25904
+foo.bar.baz(very => // A comment
+  very
+).biz(([baz={a: [123]}, boz]) =>
+  baz
+).snarf((snorf) => /* Another comment */
+  snorf
+);
+
 // Local Variables:
 // indent-tabs-mode: nil
 // js-indent-level: 2
-- 
2.11.0


[-- Attachment #3: 0001-js-indent-align-list-continuation-Make-variable-safe.patch --]
[-- Type: text/x-patch, Size: 900 bytes --]

From 918f873d1d702d4e23353747b97b77004dd738de Mon Sep 17 00:00:00 2001
From: Jackson Ray Hamilton <jackson@jacksonrayhamilton.com>
Date: Sat, 9 Feb 2019 11:50:05 -0800
Subject: [PATCH] js-indent-align-list-continuation: Make variable safe

* lisp/progmodes/js.el (js-indent-align-list-continuation): Indicate
variable is safe as a file-local variable.  This fixes the
js-indent-align-list-continuation-nil test when run with make.
---
 lisp/progmodes/js.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index a94a2fe134..6e18e30517 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -477,6 +477,7 @@ js-indent-align-list-continuation
   "Align continuation of non-empty ([{ lines in `js-mode'."
   :version "26.1"
   :type 'boolean
+  :safe 'booleanp
   :group 'js)
 
 (defcustom js-comment-lineup-func #'c-lineup-C-comments
-- 
2.11.0

bug#25904: Patches

[Dmitry Gutov]

Version: 27.1

On 11.02.2019 01:03, Jackson Ray Hamilton wrote:

> Both changes are attached.  They could be applied to the emacs-26 
> branch.  (Or master, but emacs-26 would be preferable so we can deliver 
> it sooner.  I tested on both branches.)

Pushed to master, thank you.

Regarding emacs-26, please feel free to lobby it with Eli. I think it's 
a fine patch, but I'm not sure it's important enough regression-wise.

bug#25904: Patches

[Jackson Ray Hamilton]

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

Fair enough, thanks for the merge.

> On February 12, 2019 at 4:27 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
>
>
> Version: 27.1
>
> On 11.02.2019 01:03, Jackson Ray Hamilton wrote:
>
> > Both changes are attached.  They could be applied to the emacs-26
> > branch.  (Or master, but emacs-26 would be preferable so we can deliver
> > it sooner.  I tested on both branches.)
>
> Pushed to master, thank you.
>
> Regarding emacs-26, please feel free to lobby it with Eli. I think it's
> a fine patch, but I'm not sure it's important enough regression-wise.

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

bug#25904: Patches

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Wed, 13 Feb 2019 03:27:26 +0300
> 
> Regarding emacs-26, please feel free to lobby it with Eli. I think it's 
> a fine patch, but I'm not sure it's important enough regression-wise.

The second one, the one which makes the variable safe in some cases,
can be cherry-picked to emacs-26.  I believe it's independent of the
main change, quite safe, and is a Good Thing anyway.

Thanks.

bug#25904: Patches

[Dmitry Gutov]

On 13.02.2019 18:15, Eli Zaretskii wrote:
> The second one, the one which makes the variable safe in some cases,
> can be cherry-picked to emacs-26.  I believe it's independent of the
> main change, quite safe, and is a Good Thing anyway.

OK, done.

Not what the question was about, though.