* bug#25904: Formatting bug with js-mode [not found] ` <CAEtRwfA5_ibO2tmKCDtOHOs9ZCYosn21sKe5gC9LhE3y54GDyQ@mail.gmail.com> @ 2017-02-28 22:50 ` Michael Snead 2017-11-07 23:55 ` Felipe Ochoa ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Michael Snead @ 2017-02-28 22:50 UTC (permalink / raw) To: 25904 [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2017-02-28 22:50 ` bug#25904: Formatting bug with js-mode Michael Snead @ 2017-11-07 23:55 ` Felipe Ochoa 2017-11-12 23:02 ` Dmitry Gutov 2018-04-29 1:20 ` bug#25904: bug#24896: JSX prop indentation after fat arrow Jimmy Yuen Ho Wong 2019-02-10 22:03 ` bug#25904: Patches Jackson Ray Hamilton 2 siblings, 1 reply; 14+ messages in thread From: Felipe Ochoa @ 2017-11-07 23:55 UTC (permalink / raw) To: 25904 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2017-11-07 23:55 ` Felipe Ochoa @ 2017-11-12 23:02 ` Dmitry Gutov 2017-11-20 22:22 ` Felipe Ochoa 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Gutov @ 2017-11-12 23:02 UTC (permalink / raw) To: Felipe Ochoa, 25904 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2017-11-12 23:02 ` Dmitry Gutov @ 2017-11-20 22:22 ` Felipe Ochoa 2017-11-23 21:41 ` Dmitry Gutov 0 siblings, 1 reply; 14+ messages in thread From: Felipe Ochoa @ 2017-11-20 22:22 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 25904 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2017-11-20 22:22 ` Felipe Ochoa @ 2017-11-23 21:41 ` Dmitry Gutov 2018-03-16 1:06 ` Felipe Ochoa 0 siblings, 1 reply; 14+ messages in thread From: Dmitry Gutov @ 2017-11-23 21:41 UTC (permalink / raw) To: Felipe Ochoa; +Cc: 25904 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2017-11-23 21:41 ` Dmitry Gutov @ 2018-03-16 1:06 ` Felipe Ochoa 2018-05-04 21:37 ` Dmitry Gutov 0 siblings, 1 reply; 14+ messages in thread From: Felipe Ochoa @ 2018-03-16 1:06 UTC (permalink / raw) To: Dmitry Gutov; +Cc: 25904 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#25904: Formatting bug with js-mode 2018-03-16 1:06 ` Felipe Ochoa @ 2018-05-04 21:37 ` Dmitry Gutov 0 siblings, 0 replies; 14+ messages in thread From: Dmitry Gutov @ 2018-05-04 21:37 UTC (permalink / raw) To: Felipe Ochoa; +Cc: 25904 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: bug#24896: JSX prop indentation after fat arrow 2017-02-28 22:50 ` bug#25904: Formatting bug with js-mode Michael Snead 2017-11-07 23:55 ` Felipe Ochoa @ 2018-04-29 1:20 ` Jimmy Yuen Ho Wong 2018-04-29 2:43 ` Eli Zaretskii 2019-02-10 22:03 ` bug#25904: Patches Jackson Ray Hamilton 2 siblings, 1 reply; 14+ messages in thread From: Jimmy Yuen Ho Wong @ 2018-04-29 1:20 UTC (permalink / raw) To: 25904; +Cc: Felipe Ochoa, Dmitry Gutov 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: bug#24896: JSX prop indentation after fat arrow 2018-04-29 1:20 ` bug#25904: bug#24896: JSX prop indentation after fat arrow Jimmy Yuen Ho Wong @ 2018-04-29 2:43 ` Eli Zaretskii 0 siblings, 0 replies; 14+ messages in thread From: Eli Zaretskii @ 2018-04-29 2:43 UTC (permalink / raw) To: Jimmy Yuen Ho Wong; +Cc: 25904, felipe.nospam.ochoa, dgutov > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Patches 2017-02-28 22:50 ` bug#25904: Formatting bug with js-mode Michael Snead 2017-11-07 23:55 ` Felipe Ochoa 2018-04-29 1:20 ` bug#25904: bug#24896: JSX prop indentation after fat arrow Jimmy Yuen Ho Wong @ 2019-02-10 22:03 ` Jackson Ray Hamilton 2019-02-13 0:27 ` Dmitry Gutov 2 siblings, 1 reply; 14+ messages in thread From: Jackson Ray Hamilton @ 2019-02-10 22:03 UTC (permalink / raw) To: 25904 [-- 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 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* bug#25904: Patches 2019-02-10 22:03 ` bug#25904: Patches Jackson Ray Hamilton @ 2019-02-13 0:27 ` Dmitry Gutov 2019-02-13 4:01 ` Jackson Ray Hamilton 2019-02-13 15:15 ` Eli Zaretskii 0 siblings, 2 replies; 14+ messages in thread From: Dmitry Gutov @ 2019-02-13 0:27 UTC (permalink / raw) To: Jackson Ray Hamilton, 25904-done 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Patches 2019-02-13 0:27 ` Dmitry Gutov @ 2019-02-13 4:01 ` Jackson Ray Hamilton 2019-02-13 15:15 ` Eli Zaretskii 1 sibling, 0 replies; 14+ messages in thread From: Jackson Ray Hamilton @ 2019-02-13 4:01 UTC (permalink / raw) To: Dmitry Gutov, 25904-done [-- 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 --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Patches 2019-02-13 0:27 ` Dmitry Gutov 2019-02-13 4:01 ` Jackson Ray Hamilton @ 2019-02-13 15:15 ` Eli Zaretskii 2019-02-14 1:20 ` Dmitry Gutov 1 sibling, 1 reply; 14+ messages in thread From: Eli Zaretskii @ 2019-02-13 15:15 UTC (permalink / raw) To: Dmitry Gutov; +Cc: aikeru, 25904 > 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
* bug#25904: Patches 2019-02-13 15:15 ` Eli Zaretskii @ 2019-02-14 1:20 ` Dmitry Gutov 0 siblings, 0 replies; 14+ messages in thread From: Dmitry Gutov @ 2019-02-14 1:20 UTC (permalink / raw) To: Eli Zaretskii; +Cc: aikeru, 25904 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. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-02-14 1:20 UTC | newest] Thread overview: 14+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <CAEtRwfDCdNSBPH=1Ohe3sBZJ70QBzJY3nd+Xs0uk85EuA3GRtw@mail.gmail.com> [not found] ` <CAEtRwfA5_ibO2tmKCDtOHOs9ZCYosn21sKe5gC9LhE3y54GDyQ@mail.gmail.com> 2017-02-28 22:50 ` bug#25904: Formatting bug with js-mode Michael Snead 2017-11-07 23:55 ` Felipe Ochoa 2017-11-12 23:02 ` Dmitry Gutov 2017-11-20 22:22 ` Felipe Ochoa 2017-11-23 21:41 ` Dmitry Gutov 2018-03-16 1:06 ` Felipe Ochoa 2018-05-04 21:37 ` Dmitry Gutov 2018-04-29 1:20 ` bug#25904: bug#24896: JSX prop indentation after fat arrow Jimmy Yuen Ho Wong 2018-04-29 2:43 ` Eli Zaretskii 2019-02-10 22:03 ` bug#25904: Patches Jackson Ray Hamilton 2019-02-13 0:27 ` Dmitry Gutov 2019-02-13 4:01 ` Jackson Ray Hamilton 2019-02-13 15:15 ` Eli Zaretskii 2019-02-14 1:20 ` Dmitry Gutov
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.