[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#25904: Formatting bug with js-mode
From: |
Dmitry Gutov |
Subject: |
bug#25904: Formatting bug with js-mode |
Date: |
Thu, 23 Nov 2017 23:41:45 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:57.0) Gecko/20100101 Thunderbird/57.0 |
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.