* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
@ 2011-06-19 20:38 ` Daniel Colascione
2012-02-15 12:32 ` Felix H. Dahlke
2012-02-24 2:16 ` Dmitry Gutov
` (7 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Daniel Colascione @ 2011-06-19 20:38 UTC (permalink / raw)
To: Felix H. Dahlke; +Cc: 8576, 8584
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Hi Felix,
Thanks for the bug report.
On 4/28/11 12:22 AM, Felix H. Dahlke wrote:
> The problem can be reproduced as follows:
>
> 1. Open a JavaScript file or load js-mode
> 2. Insert the following code:
> var i = 1,
> j = 2;
> 3. Observe that "j" is not highlighted and that pressing TAB on the
> second line doesn't indent it correctly.
>
> Highlighting works correctly if both variables are declared on the
> same line:
> var i = 1, j = 2;
>
After looking at what would be required to address this bug, I'd like to
leave it unfixed for now:
The machinery required to implement the tracking necessary to handle
this construct in a generic way would be quite complex. Simple cases
could perhaps be recognized more quickly, but such a solution would fail
in hard to predict ways. Long-range effects are notoriously difficult to
recognize in font-lock, and reliably rehighlighting them after a change
is even trickier.
Additionally, this construct is relatively rare; if you're going to
split variable declarations across several lines, you might as well use
another "var". Also, the existing highlighting has no ill effects: the
second declaration is just interpreted as an assignment, and the worst
part is a lack of font-lock-variable-name-face, not syntactic incoherence.
I'll take patches, but for now, I think we should defer a solution to
this issue until we have a more sophisticated and general highlighting
scheme.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (Darwin)
iEYEARECAAYFAk3+XiwACgkQ17c2LVA10VvACwCdHD38OsUrXWLaCt3prK4BQ+3a
SqUAoLdQ+C8nVvLXJkvcza3d2JyTXtXs
=58ua
-----END PGP SIGNATURE-----
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-06-19 20:38 ` Daniel Colascione
@ 2012-02-15 12:32 ` Felix H. Dahlke
2012-02-15 18:58 ` Stefan Monnier
0 siblings, 1 reply; 30+ messages in thread
From: Felix H. Dahlke @ 2012-02-15 12:32 UTC (permalink / raw)
To: Daniel Colascione; +Cc: 8576, 8584
[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]
On 06/19/2011 10:38 PM, Daniel Colascione wrote:
> Hi Felix,
>
> Thanks for the bug report.
Hi Daniel,
sorry for getting back so terribly late, I somehow failed to notice the
email and didn't use Emacs for JS much in the past year. But I'm using
js-mode intensively again from now on, so the issue has become important
for me again.
> Additionally, this construct is relatively rare; if you're going to
> split variable declarations across several lines, you might as well use
> another "var".
I disagree, it's a very popular style and I'm using it all the time.
Many others do, look e.g. at the jQuery code:
https://github.com/jquery/jquery/blob/master/src/core.js
> Also, the existing highlighting has no ill effects: the
> second declaration is just interpreted as an assignment, and the worst
> part is a lack of font-lock-variable-name-face, not syntactic incoherence.
True, but it's still very irritating to have Emacs indent it incorrectly
every time you insert/edit such a statement and having to manually fix
the indentation. That's why I used other editors for JS in the past
year, but I really want to use Emacs, so I'm back.
> > I'll take patches, but for now, I think we should defer a solution to
> this issue until we have a more sophisticated and general highlighting
> scheme.
I did have a look at the code, but I'm not familiar with font-lock, so I
couldn't fix it. But I'd be very happy to help with the more general
highlighting scheme.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-02-15 12:32 ` Felix H. Dahlke
@ 2012-02-15 18:58 ` Stefan Monnier
2012-02-15 19:04 ` Glenn Morris
2012-02-15 19:25 ` Daniel Colascione
0 siblings, 2 replies; 30+ messages in thread
From: Stefan Monnier @ 2012-02-15 18:58 UTC (permalink / raw)
To: Felix H. Dahlke; +Cc: 8576, Daniel Colascione, 8584
>> Also, the existing highlighting has no ill effects: the
>> second declaration is just interpreted as an assignment, and the worst
>> part is a lack of font-lock-variable-name-face, not syntactic incoherence.
> True, but it's still very irritating to have Emacs indent it incorrectly
> every time you insert/edit such a statement and having to manually fix
Agreed: it's OK for the highlighting to be a bit off, but indentation
is more irritating. Luckily, we can afford to work harder during
indentation than during highlighting, so it should be fixable.
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-02-15 18:58 ` Stefan Monnier
@ 2012-02-15 19:04 ` Glenn Morris
2012-02-15 19:25 ` Daniel Colascione
1 sibling, 0 replies; 30+ messages in thread
From: Glenn Morris @ 2012-02-15 19:04 UTC (permalink / raw)
To: 8576
I don't know what this issue is supposed to have to do with bug#8584
("erc: New response handler"). Fortunately that bug is archived, so it
isn't having any effect, but please stop cc'ing it anyway.
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-02-15 18:58 ` Stefan Monnier
2012-02-15 19:04 ` Glenn Morris
@ 2012-02-15 19:25 ` Daniel Colascione
2012-02-15 20:41 ` Stefan Monnier
2012-04-04 11:26 ` Felix H. Dahlke
1 sibling, 2 replies; 30+ messages in thread
From: Daniel Colascione @ 2012-02-15 19:25 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576, Felix H. Dahlke, Daniel Colascione, 8584
On 2/15/2012 10:58 AM, Stefan Monnier wrote:
>>> Also, the existing highlighting has no ill effects: the
>>> second declaration is just interpreted as an assignment, and the worst
>>> part is a lack of font-lock-variable-name-face, not syntactic incoherence.
>> True, but it's still very irritating to have Emacs indent it incorrectly
>> every time you insert/edit such a statement and having to manually fix
>
> Agreed: it's OK for the highlighting to be a bit off, but indentation
> is more irritating. Luckily, we can afford to work harder during
> indentation than during highlighting, so it should be fixable.
I'll work on a fix, at least for indentation. The trouble is that the
fix involves a potentially unbounded amount of backward parsing. We'll
see how it goes.
Stefan, is this something that should be in 24.1?
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-02-15 19:25 ` Daniel Colascione
@ 2012-02-15 20:41 ` Stefan Monnier
2012-04-04 11:26 ` Felix H. Dahlke
1 sibling, 0 replies; 30+ messages in thread
From: Stefan Monnier @ 2012-02-15 20:41 UTC (permalink / raw)
To: Daniel Colascione; +Cc: 8576, Felix H. Dahlke, Daniel Colascione, 8584
> I'll work on a fix, at least for indentation. The trouble is that the
> fix involves a potentially unbounded amount of backward parsing.
AFAIK all indentation algorithms involve some unbounded amount of
backward parsing, except for some rare exceptions where the language's
syntax is sufficiently limited/redundant (and line-oriented).
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-02-15 19:25 ` Daniel Colascione
2012-02-15 20:41 ` Stefan Monnier
@ 2012-04-04 11:26 ` Felix H. Dahlke
2012-06-01 7:30 ` Felix H. Dahlke
[not found] ` <mailman.2065.1338535901.855.bug-gnu-emacs@gnu.org>
1 sibling, 2 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-04-04 11:26 UTC (permalink / raw)
To: Daniel Colascione; +Cc: 8576, 8584
On Feb 15, 2012, at 8:25 PM, Daniel Colascione wrote:
> I'll work on a fix, at least for indentation. The trouble is that the
> fix involves a potentially unbounded amount of backward parsing. We'll
> see how it goes.
I fixed this locally in the js.el that comes with Emacs 23.4, based on Dmitry's work. Have you already started on this or would you be interested in a patch?
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-04-04 11:26 ` Felix H. Dahlke
@ 2012-06-01 7:30 ` Felix H. Dahlke
[not found] ` <mailman.2065.1338535901.855.bug-gnu-emacs@gnu.org>
1 sibling, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-06-01 7:30 UTC (permalink / raw)
To: Daniel Colascione; +Cc: 8576, 8584
[-- Attachment #1.1.1: Type: text/plain, Size: 1177 bytes --]
Okay, I've prepared a patch for this, based on Dimitriv's work. I've
been using this for a while now and haven't run into any problems.
The attached patch is against the latest revision in the emacs-23
branch. I can provide a patch for emacs-24 or trunk if that's desirable.
Here's the patched version if you'd like to try it out:
https://gist.github.com/2849799
And here's one that should work in Emacs 24:
https://gist.github.com/2849595
There's still one thing that I'd like to behave differently, but I
thought I'd best discuss this first.
The patched js.el will indent the following code like this:
var foo = 5,
bar = function() {
};
That's fine. But if a function or an object literal come first, it
indents it like this:
var bar = function() {
},
foo = 5;
Which I consider somewhat ugly. I'd like to make it indent as follows:
var bar = function() {
},
foo = 5;
That should only happen for multi var declarations, single declarations
should indent just like before:
var bar = function() {
};
Would that be acceptable? It'll make the code a bit more complicated,
but I think it's worth it.
[-- Attachment #1.1.2: Type: text/html, Size: 2041 bytes --]
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: indent-multiline-var-statements.patch --]
[-- Type: text/x-patch; name="indent-multiline-var-statements.patch", Size: 8656 bytes --]
# Bazaar merge directive format 2 (Bazaar 0.90)
# revision_id: fhd@ubercode.de-20120601072216-wsfs98ornfd1mugv
# target_branch: file:///home/fhd/Projects/emacs/emacs-23/
# testament_sha1: 101562a7a137d29600bd25ef2b014ca7e5e91ae1
# timestamp: 2012-06-01 09:22:39 +0200
# base_revision_id: handa@m17n.org-20120304120609-saqksj43mnq1tz4n
#
# Begin patch
=== modified file 'lisp/progmodes/js.el'
--- lisp/progmodes/js.el 2012-01-11 07:52:35 +0000
+++ lisp/progmodes/js.el 2012-06-01 07:22:16 +0000
@@ -418,6 +418,16 @@
:paren-depth most-negative-fixnum
:type 'toplevel))
+(defconst js--indent-operator-re
+ (concat "[-+*/%<>=&^|?:.]\\([^-+*/]\\|$\\)\\|"
+ (regexp-opt '("in" "instanceof") 'words))
+ "Regular expression matching operators that affect indentation
+of continued expressions.")
+
+(defconst js--declaration-keyword-re
+ (regexp-opt '("var" "let" "const") 'words)
+ "Regular expression matching variable declaration keywords.")
+
;;; User Customization
(defgroup js nil
@@ -1758,41 +1768,78 @@
(list (cons 'c js-comment-lineup-func))))
(c-get-syntactic-indentation (list (cons symbol anchor)))))
+(defun js--multiline-decl-indentation ()
+ "Returns the declaration indentation column if the current line belongs
+to a multiline declaration statement. All declarations are lined up vertically:
+
+var a = 10,
+ b = 20,
+ c = 30;
+"
+ (let (at-opening-bracket)
+ (save-excursion
+ (back-to-indentation)
+ (when (not (looking-at js--declaration-keyword-re))
+ (when (looking-at js--indent-operator-re)
+ (goto-char (match-end 0)))
+ (while (and (not at-opening-bracket)
+ (not (bobp))
+ (let ((pos (point)))
+ (save-excursion
+ (js--backward-syntactic-ws)
+ (or (eq (char-before) ?,)
+ (and (not (eq (char-before) ?\;))
+ (and
+ (prog2
+ (skip-chars-backward "[[:punct:]]")
+ (looking-at js--indent-operator-re)
+ (js--backward-syntactic-ws))
+ (not (eq (char-before) ?\;))))
+ (and (>= pos (point-at-bol))
+ (<= pos (point-at-eol)))))))
+ (condition-case err
+ (backward-sexp)
+ (scan-error (setq at-opening-bracket t))))
+ (when (looking-at js--declaration-keyword-re)
+ (- (1+ (match-end 0)) (point-at-bol)))))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
- (save-excursion
- (back-to-indentation)
- (cond ((nth 4 parse-status)
- (js--get-c-offset 'c (nth 8 parse-status)))
- ((nth 8 parse-status) 0) ; inside string
- ((js--ctrl-statement-indentation))
- ((eq (char-after) ?#) 0)
- ((save-excursion (js--beginning-of-macro)) 4)
- ((nth 1 parse-status)
- (let ((same-indent-p (looking-at
- "[]})]\\|\\_<case\\_>\\|\\_<default\\_>"))
- (continued-expr-p (js--continued-expression-p)))
- (goto-char (nth 1 parse-status))
- (if (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
- (progn
- (skip-syntax-backward " ")
- (when (eq (char-before) ?\)) (backward-list))
- (back-to-indentation)
- (cond (same-indent-p
- (current-column))
- (continued-expr-p
- (+ (current-column) (* 2 js-indent-level)
- js-expr-indent-offset))
- (t
- (+ (current-column) js-indent-level))))
- (unless same-indent-p
- (forward-char)
- (skip-chars-forward " \t"))
- (current-column))))
+ (or
+ (js--multiline-decl-indentation)
+ (save-excursion
+ (back-to-indentation)
+ (cond ((nth 4 parse-status)
+ (js--get-c-offset 'c (nth 8 parse-status)))
+ ((nth 8 parse-status) 0) ; inside string
+ ((js--ctrl-statement-indentation))
+ ((eq (char-after) ?#) 0)
+ ((save-excursion (js--beginning-of-macro)) 4)
+ ((nth 1 parse-status)
+ (let ((same-indent-p (looking-at
+ "[]})]\\|\\_<case\\_>\\|\\_<default\\_>"))
+ (continued-expr-p (js--continued-expression-p)))
+ (goto-char (nth 1 parse-status))
+ (if (looking-at "[({[]\\s-*\\(/[/*]\\|$\\)")
+ (progn
+ (skip-syntax-backward " ")
+ (when (eq (char-before) ?\)) (backward-list))
+ (back-to-indentation)
+ (cond (same-indent-p
+ (current-column))
+ (continued-expr-p
+ (+ (current-column) (* 2 js-indent-level)
+ js-expr-indent-offset))
+ (t
+ (+ (current-column) js-indent-level))))
+ (unless same-indent-p
+ (forward-char)
+ (skip-chars-forward " \t"))
+ (current-column))))
- ((js--continued-expression-p)
- (+ js-indent-level js-expr-indent-offset))
- (t 0))))
+ ((js--continued-expression-p)
+ (+ js-indent-level js-expr-indent-offset))
+ (t 0)))))
(defun js-indent-line ()
"Indent the current line as JavaScript."
# Begin bundle
IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWeOoVD4AA4v/gH3VVdNf////
92AQD7////5gB7w+3TrtNciMZTbna6HRrVUzhkmim0jIJtBMTaJlPCajNQ9TR6NJ6E9GUANPFGgD
IhTAanpqZqaoemU/VPU/VNGnqAADQANAAAA40NA0aZGmjTIDEwQAA0BoDTIDAmQJEUAQSafpNJpo
A2k00aGgeoAAAGgaADjQ0DRpkaaNMgMTBAADQGgNMgMCZAkUAgII00yNTKekbSGQNGh6gZNAAGgD
0jZDMm0MYgpg0dbFVXM/Le4Tc3urftnBDeJrI+W8vtgD6tVP6MVjG1H3dEhL9zN+uA99+pGnX1ux
2pKjPbFSR+ozzZOONOmnCvhnEkgE0XsSFAMYwY09PFINVrAmVCqJgtnTRhTQ+FFncM9tJxaZ40D4
Jm61wi4fpbUYuE3PRTY0N1KF93nPljXR5yfllvYT4GlyttwNQPjcXKK338KUBElnxqVeSSvaZHXs
sA3o8WWb78DXrkoKz7NehrqVEmW771y14omwXuCUzVUuDrki6vlf3LVTXFjbdrcY80bhcFdm2Kmn
NVEMwHrRT3sYERnvkGxriNgzgMRAiDsHT07W9yMZqCjR1di3EJar9Xa4RdrzI0B0DQuplLeWLogW
4yXnZS2MqYRNttBQ/F55uouruyYZJ1DgTbdJhEuMKL4TKmBMnKEYA/ysekLWTEGrIwznQLMq+1jQ
pQ+usMbgn3ZJRcXk6WI5ynHLx1LseyyUJXWud1tUFv+1NJ5t2TtuSw3pya/DCS/ZjI0hrCozyLVX
iqxkwciafHtOzaSCiwGvnI1kF5UHSU4q8kqLK9OlU1b36kb+xZeWQGY8m3eQvGOqfzYyzRIt5Es0
awe+LdTFh3weS01qR2WMtpWyP0y92SE6kNBC1XKQiwYSE0iL+1r6qKAeHeQQUS22WxRME1aCtKW7
ezWClVLEOhNyYUA0YjYIcTXUlS1zdqECBqsZLYnzmpL2Z5G8Yvzh1xKWIP2eJS3l8a8XgQuTPLQH
g+s8yyhGPcuF8BDDBcsEEtIoRiLgnVStJaohtiT2UwgolDg1R2wLRN1ZU+8z7dsKu5rGChcO9WZN
TWm1Szjv7VVXuKkbLyBkZ8pFmfoV+/CFIHJZT1U1Yq+lAcMDuOEwLaqyA8WXPnhFhiUFAnbzqoCL
9UoA0o3EIa6rDYLZt4zF9QUU2KGxayNS1zRZWCma0KBYaoKQmujTcDWI45dKyVdj9zFtWFsEAdO1
gcpTgdQzMKQHwo3XmGPw/otbAh+u6nfpZZVQTOb1zo5rIJw+yHLbM4lXUEKDxVZWJGRHZZ/koVDK
iD1fjeOAV57etDWhIooQuKhOovpNH5eBYijQDFoUTxXUDhGAYRkwhswtFD0Zc/KecoDeoDVp+DXU
P9v9yS2XRdOoORUTPi8RsgRE4pfCBnrRKZBGBoHqlFKmDkjIG1kmhgNHjzCHj/KRh4tm+IxhAg4k
In9RNRzRnQ2qh3ZSaLrI58440Ws7NK6SkvCHaq1IZI1hJoKoN6XHQOKwbj25xJlrYQIzmHkCd6aV
VBg0u8bNfEZC0WU04LoS3xXel4Rc6KWi0oUwibaCCNE/dvltGaRfNQWDLwZtAYulXYt1sACgqLoH
kMBCtJggW8zTxWw7Do5LoyT46bRfZRozwKeOKFBbooywE3Lu6JWFTx24DNUHnI/xXr7TGPHy3Z2Y
P7bnDMG2cxZLWjYgE4gDEpJWeraRbXsxaF6WEOu2Mbd3uZ5od0V1NkDofLJUDAJTIBiMeasF028i
CK8gvGsRZBuCU28ZmjDOOYYgmLGMEkc8BgdUlrhaVlOtluBiM5oumpyO28rM74t6OoMgxUBEbCUD
OAhofAFwoeFKVZ+E1q59vHGUC4lstYE5igCTbR0padF5UWgOnwXYcoac0btGWtSG0xpNpobxXphp
4uqE21ngLNNg32taOD3h5nc1tVNwcYsV5lQVeZTbGRN8AoE9EQ7hRLp77XPxLVpWmaHC7o7no58p
TvFHQ0a0a49S6cZFTIheF4dGg5sckb6xWVmgV5jVhU1e2r4vd7CWEjY7mLaTVriUcGnUqEV/0ARJ
i6xuGgURKGNVtunqRkutWucxp2GDBH0jAdUSRFSaNMAO/12aMlz8Esp776BFeAMRWJ2FgkJh4eeJ
ECArXC2qFEAryYgwA6gCHoAtgwKVMiLbhURJOvcsQwPk1cG00QLkGsMXIv8VyCXdc7rD5UadIuCx
5KkjaVTRQEqJXEUg5SlIaJTTXEIagHeVRJFUX78SwRnd15EJoNW+V6ozCtEOtkohEkWZrUt0WTWb
G7BER3Oikr64G1TXCfHFddq4GZNF+nBDrx3KldAnI4sbgxOZjTl7CuWC05pXK0IYaBixBDhWZVmK
wwCigjWyyNlxUThChze1STAIMBppUh0R5hlRMBocgiw2aritEQU8LjjKI0lUUSNTDdY1qKzB0WiO
ETBhdOiPUWku3yZKRsB1C6dhESGYYX2yiQEIpIvdrvXpl4FxmgXMA26sERAN0CMTke0EeAMDqz1k
gYVlJFgF5dMCviuHWg0/8XckU4UJDjqFQ+A=
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread[parent not found: <mailman.2065.1338535901.855.bug-gnu-emacs@gnu.org>]
* Re: bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
[not found] ` <mailman.2065.1338535901.855.bug-gnu-emacs@gnu.org>
@ 2012-07-05 14:53 ` jaseemabid
0 siblings, 0 replies; 30+ messages in thread
From: jaseemabid @ 2012-07-05 14:53 UTC (permalink / raw)
To: bug-gnu-emacs; +Cc: 8576
On Friday, 1 June 2012 13:00:21 UTC+5:30, Felix H. Dahlke wrote:
> Okay, I've prepared a patch for this, based on Dimitriv's work. I've
> been using this for a while now and haven't run into any problems.
>
>
>
> The attached patch is against the latest revision in the emacs-23
> branch. I can provide a patch for emacs-24 or trunk if that's
> desirable.
>
>
>
> Here's the patched version if you'd like to try it out:
>
>
> <a href="https://gist.github.com/2849799" target="_blank">https://gist.github.com/<WBR>2849799</a>
>
>
>
> And here's one that should work in Emacs 24:
>
>
> <a href="https://gist.github.com/2849595" target="_blank">https://gist.github.com/<WBR>2849595</a>
>
>
>
> There's still one thing that I'd like to behave differently, but I
> thought I'd best discuss this first.
>
>
>
> The patched js.el will indent the following code like this:
>
>
>
> var foo = 5,
>
> bar = function() {
>
> };
>
>
>
> That's fine. But if a function or an object literal come first, it
> indents it like this:
>
>
>
> var bar = function() {
>
> },
>
> foo = 5;
>
>
>
> Which I consider somewhat ugly. I'd like to make it indent as
> follows:
>
>
>
> var bar = function() {
>
> },
>
> foo = 5;
>
>
>
> That should only happen for multi var declarations, single
> declarations should indent just like before:
>
>
>
> var bar = function() {
>
> };
>
>
>
> Would that be acceptable? It'll make the code a bit more
> complicated, but I think it's worth it.
>
>
When the first var is staring with an offset, like inside a nested function or something, this is failing. It feels like the second line is indented wrt to coloumn 0 and not wrt to the "var" statement.
This is what I expect
\t \t var a = "foo",
\t \t \t b = "bar";
but it becomes
\t \t var a = "foo",
\t ..b = "bar";
I might be wrong, Im a noob with emacs. Reporting the behavior because the issue is bugging me a lot. I tried with the gist for v24 on v24.1.1.
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
2011-06-19 20:38 ` Daniel Colascione
@ 2012-02-24 2:16 ` Dmitry Gutov
2012-03-03 1:46 ` bug#8576: " Zeth
` (6 subsequent siblings)
8 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2012-02-24 2:16 UTC (permalink / raw)
To: 8576
Here is a function I wrote to properly indent multiline declarations in
another package:
https://github.com/mooz/js2-mode/blob/master/js2-mode.el#L9917
It is kinda ugly, but it's been working reliably for me for some time
now, and it doesn't rely on anything specific to js2-mode.
I have a CA signed, so it's fair game.
Dmitry.
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
2011-06-19 20:38 ` Daniel Colascione
2012-02-24 2:16 ` Dmitry Gutov
@ 2012-03-03 1:46 ` Zeth
2012-03-03 4:51 ` Daniel Colascione
2012-06-07 23:04 ` bug#8576: 23.2; " Dmitry Gutov
` (5 subsequent siblings)
8 siblings, 1 reply; 30+ messages in thread
From: Zeth @ 2012-03-03 1:46 UTC (permalink / raw)
To: 8576
Hello Emacs people,
Daniel Colascione in message 8 said:
"Additionally, this construct is relatively rare; if you're going to
split variable declarations across several lines, you might as well use
another "var"."
Actually the format required by JSLint, for good reason, is that one
single var statement per function, at the very start of the function, i.e.:
var name, another-name, another-name;
When you have more than 80 characters worth of variable declarations,
you need to start a new line:
var first-name, second-name, third-name,
fourth-name, fifth-name;
The first-name and fourth-name above should align (i.e. the second and
subsequent lines should be indented by 4 characters).
Emacs does not highlight the second and subsequent lines correctly, and
it does not indent them correctly either.
This is a shame because in all other respects, Emacs indents and
highlights in the way you would expect when following the demands of JSLint.
(JSLint demands this behaviour to avoid making hoisting and scoping
mistakes, i.e. those used to other languages with block scope might
believe the placement of var statements further down in a function
implies that a variable is not defined yet, when actually it is.)
Best Wishes,
Zeth
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: js-mode doesn't support multi-line variable declarations
2012-03-03 1:46 ` bug#8576: " Zeth
@ 2012-03-03 4:51 ` Daniel Colascione
0 siblings, 0 replies; 30+ messages in thread
From: Daniel Colascione @ 2012-03-03 4:51 UTC (permalink / raw)
To: Zeth; +Cc: 8576
[-- Attachment #1: Type: text/plain, Size: 579 bytes --]
On 3/2/12 5:46 PM, Zeth wrote:
> When you have more than 80 characters worth of variable declarations,
> you need to start a new line:
>
> var first-name, second-name, third-name,
> fourth-name, fifth-name;
>
> (JSLint demands this behaviour to avoid making hoisting and scoping
> mistakes, i.e. those used to other languages with block scope might
> believe the placement of var statements further down in a function
> implies that a variable is not defined yet, when actually it is.)
>
I'll be able to put some time into fixing this issue this weekend.
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 235 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
` (2 preceding siblings ...)
2012-03-03 1:46 ` bug#8576: " Zeth
@ 2012-06-07 23:04 ` Dmitry Gutov
2012-06-08 3:13 ` Felix H. Dahlke
2012-07-17 3:16 ` Felix H. Dahlke
2012-07-05 23:02 ` bug#8576: 23.2; , " Dmitry Gutov
` (4 subsequent siblings)
8 siblings, 2 replies; 30+ messages in thread
From: Dmitry Gutov @ 2012-06-07 23:04 UTC (permalink / raw)
To: fhd, 8576
> There's still one thing that I'd like to behave differently, but I
> thought I'd best discuss this first.
...
> Would that be acceptable? It'll make the code a bit more complicated,
> but I think it's worth it.
Personally, I think that's the desirable behavior, but you should keep
in mind that this will require the indentation engine to look ahead in
the buffer and account for the case when there's no text after the
point, for example.
Suppose the first value function actually has a non-empty body, and
we're just typing the code for the first time.
Even if we're using a snippet package that inserted "function() {}" for
us, we will type the important comma after "}" only when the function
body is written, and then we'll have to go back to reindent the body again.
I think that's acceptable, but that's not how indentation functions in
Emacs usually work.
-- Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-06-07 23:04 ` bug#8576: 23.2; " Dmitry Gutov
@ 2012-06-08 3:13 ` Felix H. Dahlke
2012-07-17 3:16 ` Felix H. Dahlke
1 sibling, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-06-08 3:13 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 8576
On 06/08/2012 01:04 AM, Dmitry Gutov wrote:
> Personally, I think that's the desirable behavior, but you should keep
> in mind that this will require the indentation engine to look ahead in
> the buffer and account for the case when there's no text after the
> point, for example.
> Suppose the first value function actually has a non-empty body, and
> we're just typing the code for the first time.
> Even if we're using a snippet package that inserted "function() {}"
> for us, we will type the important comma after "}" only when the
> function body is written, and then we'll have to go back to reindent
> the body again.
>
> I think that's acceptable, but that's not how indentation functions in
> Emacs usually work.
You're right, it's harder than I thought. I'm currently looking for
similar problems in other modes and see how they handle it.
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-06-07 23:04 ` bug#8576: 23.2; " Dmitry Gutov
2012-06-08 3:13 ` Felix H. Dahlke
@ 2012-07-17 3:16 ` Felix H. Dahlke
1 sibling, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-07-17 3:16 UTC (permalink / raw)
Cc: 8576
[-- Attachment #1.1: Type: text/plain, Size: 1276 bytes --]
I tried some things but couldn't really make it work like I wanted. So I
decided to just improve the patch a bit. I am now using the proper
indentation which also fixes the issue reported by jaseemabid.
I've attached the patch for Emacs 24 and updated the Gists:
https://gist.github.com/2849595 (Emacs 24)
https://gist.github.com/2849799 (Emacs 23)
The problem with my desired behaviour is indeed that it requires forward
parsing. So when you're indenting a full statement like this, it works:
var x = {
num: 5
},
y = 6;
However, when you're initially typing this (pressing TAB on each line),
it ends up like this:
var x = {
num: 5
},
y = 6;
You'd have to go back and indent the whole thing again after adding the
closing brace followed by a comma. That's actually pretty annoying in
practice and would probably lead to highly inconsistent indendation, so
I decided to leave it out for now.
The only solution I can think of would be to go back up after typing a
closing brace followed by a comma and indent the preceeding lines. Some
modes do that, but it would require drastic changes to js.el. I'd rather
get this patch in first and see if the issue annoys me (or anyone else)
enough to work further on this.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: indent-multi-line-var-statements.patch --]
[-- Type: text/x-patch; name="indent-multi-line-var-statements.patch", Size: 2811 bytes --]
=== modified file 'lisp/progmodes/js.el'
--- lisp/progmodes/js.el 2012-07-11 23:13:41 +0000
+++ lisp/progmodes/js.el 2012-07-17 03:08:57 +0000
@@ -416,6 +416,16 @@
:paren-depth most-negative-fixnum
:type 'toplevel))
+(defconst js--indent-operator-re
+ (concat "[-+*/%<>=&^|?:.]\\([^-+*/]\\|$\\)\\|"
+ (regexp-opt '("in" "instanceof") 'words))
+ "Regular expression matching operators that affect indentation
+of continued expressions.")
+
+(defconst js--declaration-keyword-re
+ (regexp-opt '("var" "let" "const") 'words)
+ "Regular expression matching variable declaration keywords.")
+
;;; User Customization
(defgroup js nil
@@ -1759,6 +1769,36 @@
(list (cons 'c js-comment-lineup-func))))
(c-get-syntactic-indentation (list (cons symbol anchor)))))
+(defun js--multi-line-declaration-indentation ()
+ "Helper function for `js--proper-indentation'.
+Return the proper indentation of the current line if it belongs to a declaration
+statement spanning multiple lines; otherwise, return nil."
+ (let (at-opening-bracket)
+ (save-excursion
+ (back-to-indentation)
+ (when (not (looking-at js--declaration-keyword-re))
+ (when (looking-at js--indent-operator-re)
+ (goto-char (match-end 0)))
+ (while (and (not at-opening-bracket)
+ (not (bobp))
+ (let ((pos (point)))
+ (save-excursion
+ (js--backward-syntactic-ws)
+ (or (eq (char-before) ?,)
+ (and (not (eq (char-before) ?\;))
+ (prog2
+ (skip-chars-backward "[[:punct:]]")
+ (looking-at js--indent-operator-re)
+ (js--backward-syntactic-ws))
+ (not (eq (char-before) ?\;)))
+ (and (>= pos (point-at-bol))
+ (<= pos (point-at-eol)))))))
+ (condition-case err
+ (backward-sexp)
+ (scan-error (setq at-opening-bracket t))))
+ (when (looking-at js--declaration-keyword-re)
+ (+ (current-indentation) js-indent-level))))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
(save-excursion
@@ -1767,6 +1807,7 @@
(js--get-c-offset 'c (nth 8 parse-status)))
((nth 8 parse-status) 0) ; inside string
((js--ctrl-statement-indentation))
+ ((js--multi-line-declaration-indentation))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
((nth 1 parse-status)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; , js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
` (3 preceding siblings ...)
2012-06-07 23:04 ` bug#8576: 23.2; " Dmitry Gutov
@ 2012-07-05 23:02 ` Dmitry Gutov
2012-07-06 0:52 ` Dmitry Gutov
` (3 subsequent siblings)
8 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2012-07-05 23:02 UTC (permalink / raw)
To: jaseemabid; +Cc: 8576
> When the first var is staring with an offset, like inside a nested
> function or something, this is failing. It feels like the second line
> is indented wrt to coloumn 0 and not wrt to the "var" statement.
I don't see the behavior you describe. With the gist for 24 loaded, this
indents fine in js-mode:
function() {
function() {
var a = "foo",
b = "bar";
}
}
-- Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; , js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
` (4 preceding siblings ...)
2012-07-05 23:02 ` bug#8576: 23.2; , " Dmitry Gutov
@ 2012-07-06 0:52 ` Dmitry Gutov
[not found] ` <mailman.4157.1341536766.855.bug-gnu-emacs@gnu.org>
` (2 subsequent siblings)
8 siblings, 0 replies; 30+ messages in thread
From: Dmitry Gutov @ 2012-07-06 0:52 UTC (permalink / raw)
To: 8576
[-- Attachment #1: Type: text/plain, Size: 298 bytes --]
> When the first var is staring with an offset, like inside a nested
> function or something, this is failing. It feels like the second line
> is indented wrt to coloumn 0 and not wrt to the "var" statement.
Reproduced when `indent-tabs-mode' is t.
Here's the updated patch for trunk.
-- Dmitry
[-- Attachment #2: js-multiline-decls.diff --]
[-- Type: text/plain, Size: 2793 bytes --]
diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 2e943be..98883a9 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1680,6 +1680,9 @@ This performs fontification according to `js--class-styles'."
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
+(defconst js--declaration-keyword-re
+ (regexp-opt '("var" "let" "const") 'words)
+ "Regular matching variable declaration keywords.")
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
@@ -1759,6 +1762,42 @@ nil."
(list (cons 'c js-comment-lineup-func))))
(c-get-syntactic-indentation (list (cons symbol anchor)))))
+(defun js--multiline-decl-indentation ()
+ "Returns the declaration indentation column if the current line belongs
+to a multiline declaration statement. All declarations are lined up vertically:
+
+var a = 10,
+ b = 20,
+ c = 30;
+"
+ (let (at-opening-bracket)
+ (save-excursion
+ (back-to-indentation)
+ (when (not (looking-at js--declaration-keyword-re))
+ (when (looking-at js--indent-operator-re)
+ (goto-char (match-end 0)))
+ (while (and (not at-opening-bracket)
+ (not (bobp))
+ (let ((pos (point)))
+ (save-excursion
+ (js--backward-syntactic-ws)
+ (or (eq (char-before) ?,)
+ (and (not (eq (char-before) ?\;))
+ (and
+ (prog2
+ (skip-chars-backward "[[:punct:]]")
+ (looking-at js--indent-operator-re)
+ (js--backward-syntactic-ws))
+ (not (eq (char-before) ?\;))))
+ (and (>= pos (point-at-bol))
+ (<= pos (point-at-eol)))))))
+ (condition-case err
+ (backward-sexp)
+ (scan-error (setq at-opening-bracket t))))
+ (when (looking-at js--declaration-keyword-re)
+ (goto-char (match-end 0))
+ (1+ (current-column)))))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
(save-excursion
@@ -1769,6 +1808,7 @@ nil."
((js--ctrl-statement-indentation))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
+ ((js--multiline-decl-indentation))
((nth 1 parse-status)
;; A single closing paren/bracket should be indented at the
;; same level as the opening statement. Same goes for
^ permalink raw reply related [flat|nested] 30+ messages in thread[parent not found: <mailman.4157.1341536766.855.bug-gnu-emacs@gnu.org>]
* bug#8576: 23.2; , js-mode doesn't support multi-line variable declarations
[not found] ` <mailman.4157.1341536766.855.bug-gnu-emacs@gnu.org>
@ 2012-07-06 1:23 ` jaseemabid
0 siblings, 0 replies; 30+ messages in thread
From: jaseemabid @ 2012-07-06 1:23 UTC (permalink / raw)
To: gnu.emacs.bug; +Cc: 8576
On Friday, 6 July 2012 06:22:23 UTC+5:30, Dmitry Gutov wrote:
> > When the first var is staring with an offset, like inside a nested
> > function or something, this is failing. It feels like the second line
> > is indented wrt to coloumn 0 and not wrt to the "var" statement.
>
> Reproduced when `indent-tabs-mode' is t.
>
> Here's the updated patch for trunk.
>
> -- Dmitry
>
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index 2e943be..98883a9 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -1680,6 +1680,9 @@ This performs fontification according to `js--class-styles'."
> (js--regexp-opt-symbol '("in" "instanceof")))
> "Regexp matching operators that affect indentation of continued expressions.")
>
> +(defconst js--declaration-keyword-re
> + (regexp-opt '("var" "let" "const") 'words)
> + "Regular matching variable declaration keywords.")
>
> (defun js--looking-at-operator-p ()
> "Return non-nil if point is on a JavaScript operator, other than a comma."
> @@ -1759,6 +1762,42 @@ nil."
> (list (cons 'c js-comment-lineup-func))))
> (c-get-syntactic-indentation (list (cons symbol anchor)))))
>
> +(defun js--multiline-decl-indentation ()
> + "Returns the declaration indentation column if the current line belongs
> +to a multiline declaration statement. All declarations are lined up vertically:
> +
> +var a = 10,
> + b = 20,
> + c = 30;
> +"
> + (let (at-opening-bracket)
> + (save-excursion
> + (back-to-indentation)
> + (when (not (looking-at js--declaration-keyword-re))
> + (when (looking-at js--indent-operator-re)
> + (goto-char (match-end 0)))
> + (while (and (not at-opening-bracket)
> + (not (bobp))
> + (let ((pos (point)))
> + (save-excursion
> + (js--backward-syntactic-ws)
> + (or (eq (char-before) ?,)
> + (and (not (eq (char-before) ?\;))
> + (and
> + (prog2
> + (skip-chars-backward "[[:punct:]]")
> + (looking-at js--indent-operator-re)
> + (js--backward-syntactic-ws))
> + (not (eq (char-before) ?\;))))
> + (and (>= pos (point-at-bol))
> + (<= pos (point-at-eol)))))))
> + (condition-case err
> + (backward-sexp)
> + (scan-error (setq at-opening-bracket t))))
> + (when (looking-at js--declaration-keyword-re)
> + (goto-char (match-end 0))
> + (1+ (current-column)))))))
> +
> (defun js--proper-indentation (parse-status)
> "Return the proper indentation for the current line."
> (save-excursion
> @@ -1769,6 +1808,7 @@ nil."
> ((js--ctrl-statement-indentation))
> ((eq (char-after) ?#) 0)
> ((save-excursion (js--beginning-of-macro)) 4)
> + ((js--multiline-decl-indentation))
> ((nth 1 parse-status)
> ;; A single closing paren/bracket should be indented at the
> ;; same level as the opening statement. Same goes for
In the mean while, I tried js2-mode. Reported the same issue there.
Fixed it here : https://github.com/mooz/js2-mode/commit/3ee849316253121ec4ee51268bc814ab60d63b2f
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
` (6 preceding siblings ...)
[not found] ` <mailman.4157.1341536766.855.bug-gnu-emacs@gnu.org>
@ 2012-07-17 4:21 ` Dmitry Gutov
2012-07-17 4:37 ` Felix H. Dahlke
2012-07-17 5:00 ` Dmitry Gutov
8 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2012-07-17 4:21 UTC (permalink / raw)
To: fhd; +Cc: 8576
"Felix H. Dahlke" <fhd@ubercode.de> writes:
> I tried some things but couldn't really make it work like I wanted.
> So I decided to just improve the patch a bit. I am now using the
> proper indentation which also fixes the issue reported by jaseemabid.
Yes, that's a bit better than my fix.
You shouldn't add the `js--indent-operator-re' constant, though, it's
already present in js-mode.
> The only solution I can think of would be to go back up after typing a
> closing brace followed by a comma and indent the preceeding lines.
> Some modes do that, but it would require drastic changes to js.el.
> I'd rather get this patch in first and see if the issue annoys me (or
> anyone else) enough to work further on this.
I don't think this will be much of a problem.
There is an alternative approach, though: one js2-mode user requested
that when the first value in the declaration is a function/object/array,
it should always be indented: https://github.com/mooz/js2-mode/issues/3
That option probably won't be very popular.
--Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 4:21 ` bug#8576: 23.2; " Dmitry Gutov
@ 2012-07-17 4:37 ` Felix H. Dahlke
0 siblings, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-07-17 4:37 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 8576
[-- Attachment #1.1: Type: text/plain, Size: 1288 bytes --]
On 07/17/2012 06:21 AM, Dmitry Gutov wrote:
> You shouldn't add the `js--indent-operator-re' constant, though, it's
> already present in js-mode.
Oh, you're right, thanks. I've updated the gists and attached a new patch.
>> The only solution I can think of would be to go back up after typing a
>> closing brace followed by a comma and indent the preceeding lines.
>> Some modes do that, but it would require drastic changes to js.el.
>> I'd rather get this patch in first and see if the issue annoys me (or
>> anyone else) enough to work further on this.
>
> I don't think this will be much of a problem.
Probably not, but I fear it's going to be a lot of work - at least when
I do it.
> There is an alternative approach, though: one js2-mode user requested
> that when the first value in the declaration is a function/object/array,
> it should always be indented: https://github.com/mooz/js2-mode/issues/3
>
> That option probably won't be very popular.
Yes, I thought about suggesting that. But considering that that makes
single variable declarations of functions/object literals look weird, I
figured that wouldn't be desirable. I saw that you can configure this in
j2-mode, perhaps that's an option. But I'd personally prefer the clever
solution.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: indent-multi-line-var-statements.patch --]
[-- Type: text/x-patch; name="indent-multi-line-var-statements.patch", Size: 2925 bytes --]
=== modified file 'lisp/progmodes/js.el'
--- lisp/progmodes/js.el 2012-07-11 23:13:41 +0000
+++ lisp/progmodes/js.el 2012-07-17 04:29:06 +0000
@@ -1675,12 +1675,15 @@
"each"))
"Regexp matching keywords optionally followed by an opening brace.")
+(defconst js--declaration-keyword-re
+ (regexp-opt '("var" "let" "const") 'words)
+ "Regular expression matching variable declaration keywords.")
+
(defconst js--indent-operator-re
(concat "[-+*/%<>=&^|?:.]\\([^-+*/]\\|$\\)\\|"
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
-
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
(save-match-data
@@ -1759,6 +1762,36 @@
(list (cons 'c js-comment-lineup-func))))
(c-get-syntactic-indentation (list (cons symbol anchor)))))
+(defun js--multi-line-declaration-indentation ()
+ "Helper function for `js--proper-indentation'.
+Return the proper indentation of the current line if it belongs to a declaration
+statement spanning multiple lines; otherwise, return nil."
+ (let (at-opening-bracket)
+ (save-excursion
+ (back-to-indentation)
+ (when (not (looking-at js--declaration-keyword-re))
+ (when (looking-at js--indent-operator-re)
+ (goto-char (match-end 0)))
+ (while (and (not at-opening-bracket)
+ (not (bobp))
+ (let ((pos (point)))
+ (save-excursion
+ (js--backward-syntactic-ws)
+ (or (eq (char-before) ?,)
+ (and (not (eq (char-before) ?\;))
+ (prog2
+ (skip-chars-backward "[[:punct:]]")
+ (looking-at js--indent-operator-re)
+ (js--backward-syntactic-ws))
+ (not (eq (char-before) ?\;)))
+ (and (>= pos (point-at-bol))
+ (<= pos (point-at-eol)))))))
+ (condition-case err
+ (backward-sexp)
+ (scan-error (setq at-opening-bracket t))))
+ (when (looking-at js--declaration-keyword-re)
+ (+ (current-indentation) js-indent-level))))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
(save-excursion
@@ -1767,6 +1800,7 @@
(js--get-c-offset 'c (nth 8 parse-status)))
((nth 8 parse-status) 0) ; inside string
((js--ctrl-statement-indentation))
+ ((js--multi-line-declaration-indentation))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
((nth 1 parse-status)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2011-04-28 7:22 bug#8576: 23.2; js-mode doesn't support multi-line variable declarations Felix H. Dahlke
` (7 preceding siblings ...)
2012-07-17 4:21 ` bug#8576: 23.2; " Dmitry Gutov
@ 2012-07-17 5:00 ` Dmitry Gutov
2012-07-17 6:32 ` Stefan Monnier
8 siblings, 1 reply; 30+ messages in thread
From: Dmitry Gutov @ 2012-07-17 5:00 UTC (permalink / raw)
To: fhd; +Cc: 8576
"Felix H. Dahlke" <fhd@ubercode.de> writes:
> On 07/17/2012 06:21 AM, Dmitry Gutov wrote:
> I've updated the gists and attached a new patch.
I'm sorry, I spoke too soon. Now I see that your patch relies on the
value of `js-indent-level' being 4. And what if the keyword is "const"?
Unless you're fine with the following indentation, the patch I posted
earlier is better:
const a = 5,
b = 6;
>>> The only solution I can think of would be to go back up after
>>> typing a closing brace followed by a comma and indent the
>>> preceeding lines.
>>> Some modes do that, but it would require drastic changes to js.el.
>>> I'd rather get this patch in first and see if the issue annoys me
>>> (or anyone else) enough to work further on this.
>>
>> I don't think this will be much of a problem.
>
> Probably not, but I fear it's going to be a lot of work - at least
> when I do it.
I meant that nobody will care, most likely. But the implementation
shouldn't be too hard either - add a function to `post-command-hook'
which would check if the command is `self-insert-command', check if we
inserted a comma, skip backward to the previous non-whitespace char, see
if it's } or ], etc.
--Dmitry
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 5:00 ` Dmitry Gutov
@ 2012-07-17 6:32 ` Stefan Monnier
2012-07-17 8:24 ` Felix H. Dahlke
0 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2012-07-17 6:32 UTC (permalink / raw)
To: Dmitry Gutov; +Cc: 8576, fhd
> const a = 5,
> b = 6;
Yup, that'd be a bug.
>>>> The only solution I can think of would be to go back up after
>>>> typing a closing brace followed by a comma and indent the
>>>> preceeding lines.
>>>> Some modes do that, but it would require drastic changes to js.el.
>>>> I'd rather get this patch in first and see if the issue annoys me
>>>> (or anyone else) enough to work further on this.
>>> I don't think this will be much of a problem.
>> Probably not, but I fear it's going to be a lot of work - at least
>> when I do it.
> I meant that nobody will care, most likely. But the implementation
> shouldn't be too hard either - add a function to `post-command-hook'
> which would check if the command is `self-insert-command', check if we
> inserted a comma, skip backward to the previous non-whitespace char, see
> if it's } or ], etc.
Rather than a post-command-hook, that would be a post-self-insert-hook.
And that would need to be conditional on some js-electric-foo variable
(or better yet, be made to work with electric-indent-mode).
Note that looking forward during indentation, while occasionally
annoying, is not that big of a problem in practice: contrary to popular
belief, we don't write code quite as linearly as one might think.
We at least as frequently edit code in place.
BTW, rather than a post-self-insert-hook, you could put a special
text-property `js--indent-depends-on-next-line' on the line, and then
when indenting a line, you could check if the previous line has that
property and if so indent both lines. I'm not claiming it's a better
approach, just an alternative one.
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 6:32 ` Stefan Monnier
@ 2012-07-17 8:24 ` Felix H. Dahlke
2012-07-17 9:50 ` Stefan Monnier
0 siblings, 1 reply; 30+ messages in thread
From: Felix H. Dahlke @ 2012-07-17 8:24 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576, Dmitry Gutov
[-- Attachment #1: Type: text/plain, Size: 2183 bytes --]
Quoting Stefan Monnier <monnier@IRO.UMontreal.CA>:
> Thank you for your efforts trying to find a good fix for this problem.
> I'd like to install your changes (or some future version of them once
> the wrinkles are worked out), but it's sufficiently non-trivial that
> we need you to sign some copyright paperwork.
I already sent a signed CA to the FSF, it should arrive shortly.
>> const a = 5,
>> b = 6;
>
> Yup, that'd be a bug.
Depends, IMO. Do we want to indent on the column, or by the configured
indentation level? I personally want to indent on column, so that
would indeed be a problem for me (never noticed because I use a tab
size of 4 and no const keyword). Should this be configurable?
If we don't make it configurable, I'd vote for going with Dmitry's
variant, as that seems to be the more popular style.
>> I meant that nobody will care, most likely. But the implementation
>> shouldn't be too hard either - add a function to `post-command-hook'
>> which would check if the command is `self-insert-command', check if we
>> inserted a comma, skip backward to the previous non-whitespace char, see
>> if it's } or ], etc.
>
> Rather than a post-command-hook, that would be a post-self-insert-hook.
> And that would need to be conditional on some js-electric-foo variable
> (or better yet, be made to work with electric-indent-mode).
>
> Note that looking forward during indentation, while occasionally
> annoying, is not that big of a problem in practice: contrary to popular
> belief, we don't write code quite as linearly as one might think.
> We at least as frequently edit code in place.
>
> BTW, rather than a post-self-insert-hook, you could put a special
> text-property `js--indent-depends-on-next-line' on the line, and then
> when indenting a line, you could check if the previous line has that
> property and if so indent both lines. I'm not claiming it's a better
> approach, just an alternative one.
Thanks for the hints Dmitry and Stefan, as you undoubtly noticed, I am
quite new to Emacs development, but willing to learn :)
But I'd rather get this patch in in the simple form first, and enhance
the behaviour with another one.
[-- Attachment #2: PGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 8:24 ` Felix H. Dahlke
@ 2012-07-17 9:50 ` Stefan Monnier
2012-07-17 9:51 ` Felix H. Dahlke
2012-07-17 17:33 ` Felix H. Dahlke
0 siblings, 2 replies; 30+ messages in thread
From: Stefan Monnier @ 2012-07-17 9:50 UTC (permalink / raw)
To: Felix H. Dahlke; +Cc: 8576, Dmitry Gutov
>>> const a = 5,
>>> b = 6;
>> Yup, that'd be a bug.
> Depends, IMO. Do we want to indent on the column, or by the configured
> indentation level?
The user already has the choice (without even a config-var) by choosing
between
const a = 5,
b = 6;
and
const
a = 5,
b = 6;
> Should this be configurable?
I don't think that's needed.
> But I'd rather get this patch in in the simple form first, and enhance the
> behaviour with another one.
Agreed,
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 9:50 ` Stefan Monnier
@ 2012-07-17 9:51 ` Felix H. Dahlke
2012-07-17 17:33 ` Felix H. Dahlke
1 sibling, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2012-07-17 9:51 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576, Dmitry Gutov
[-- Attachment #1: Type: text/plain, Size: 661 bytes --]
Okay, I'll incorporate Dmitry's fix when I get home then.
Quoting Stefan Monnier <monnier@IRO.UMontreal.CA>:
>>>> const a = 5,
>>>> b = 6;
>>> Yup, that'd be a bug.
>> Depends, IMO. Do we want to indent on the column, or by the configured
>> indentation level?
>
> The user already has the choice (without even a config-var) by choosing
> between
>
> const a = 5,
> b = 6;
> and
> const
> a = 5,
> b = 6;
>
>> Should this be configurable?
>
> I don't think that's needed.
>
>> But I'd rather get this patch in in the simple form first, and enhance the
>> behaviour with another one.
>
> Agreed,
>
>
> Stefan
>
[-- Attachment #2: PGP Digital Signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 9:50 ` Stefan Monnier
2012-07-17 9:51 ` Felix H. Dahlke
@ 2012-07-17 17:33 ` Felix H. Dahlke
2013-01-10 3:48 ` Felix H. Dahlke
1 sibling, 1 reply; 30+ messages in thread
From: Felix H. Dahlke @ 2012-07-17 17:33 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576, Dmitry Gutov
[-- Attachment #1.1: Type: text/plain, Size: 76 bytes --]
Okay, I've updated the gists and attached the modified patch as discussed.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: indent-multi-line-var-statements.patch --]
[-- Type: text/x-patch; name="indent-multi-line-var-statements.patch", Size: 2943 bytes --]
=== modified file 'lisp/progmodes/js.el'
--- lisp/progmodes/js.el 2012-07-11 23:13:41 +0000
+++ lisp/progmodes/js.el 2012-07-17 17:25:45 +0000
@@ -1675,12 +1675,15 @@
"each"))
"Regexp matching keywords optionally followed by an opening brace.")
+(defconst js--declaration-keyword-re
+ (regexp-opt '("var" "let" "const") 'words)
+ "Regular expression matching variable declaration keywords.")
+
(defconst js--indent-operator-re
(concat "[-+*/%<>=&^|?:.]\\([^-+*/]\\|$\\)\\|"
(js--regexp-opt-symbol '("in" "instanceof")))
"Regexp matching operators that affect indentation of continued expressions.")
-
(defun js--looking-at-operator-p ()
"Return non-nil if point is on a JavaScript operator, other than a comma."
(save-match-data
@@ -1759,6 +1762,37 @@
(list (cons 'c js-comment-lineup-func))))
(c-get-syntactic-indentation (list (cons symbol anchor)))))
+(defun js--multi-line-declaration-indentation ()
+ "Helper function for `js--proper-indentation'.
+Return the proper indentation of the current line if it belongs to a declaration
+statement spanning multiple lines; otherwise, return nil."
+ (let (at-opening-bracket)
+ (save-excursion
+ (back-to-indentation)
+ (when (not (looking-at js--declaration-keyword-re))
+ (when (looking-at js--indent-operator-re)
+ (goto-char (match-end 0)))
+ (while (and (not at-opening-bracket)
+ (not (bobp))
+ (let ((pos (point)))
+ (save-excursion
+ (js--backward-syntactic-ws)
+ (or (eq (char-before) ?,)
+ (and (not (eq (char-before) ?\;))
+ (prog2
+ (skip-chars-backward "[[:punct:]]")
+ (looking-at js--indent-operator-re)
+ (js--backward-syntactic-ws))
+ (not (eq (char-before) ?\;)))
+ (and (>= pos (point-at-bol))
+ (<= pos (point-at-eol)))))))
+ (condition-case err
+ (backward-sexp)
+ (scan-error (setq at-opening-bracket t))))
+ (when (looking-at js--declaration-keyword-re)
+ (goto-char (match-end 0))
+ (1+ (current-column)))))))
+
(defun js--proper-indentation (parse-status)
"Return the proper indentation for the current line."
(save-excursion
@@ -1767,6 +1801,7 @@
(js--get-c-offset 'c (nth 8 parse-status)))
((nth 8 parse-status) 0) ; inside string
((js--ctrl-statement-indentation))
+ ((js--multi-line-declaration-indentation))
((eq (char-after) ?#) 0)
((save-excursion (js--beginning-of-macro)) 4)
((nth 1 parse-status)
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2012-07-17 17:33 ` Felix H. Dahlke
@ 2013-01-10 3:48 ` Felix H. Dahlke
2013-01-11 23:25 ` Stefan Monnier
0 siblings, 1 reply; 30+ messages in thread
From: Felix H. Dahlke @ 2013-01-10 3:48 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576, Dmitry Gutov
So, it's been a while. Anything I can do to help get this merged?
^ permalink raw reply [flat|nested] 30+ messages in thread
* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2013-01-10 3:48 ` Felix H. Dahlke
@ 2013-01-11 23:25 ` Stefan Monnier
2013-01-12 2:59 ` Felix H. Dahlke
0 siblings, 1 reply; 30+ messages in thread
From: Stefan Monnier @ 2013-01-11 23:25 UTC (permalink / raw)
To: Felix H. Dahlke; +Cc: 8576-done
> So, it's been a while. Anything I can do to help get this merged?
Sending such a `ping' is a great way to help ;-)
Installed, thank you, and sorry for the delay,
Stefan
^ permalink raw reply [flat|nested] 30+ messages in thread* bug#8576: 23.2; js-mode doesn't support multi-line variable declarations
2013-01-11 23:25 ` Stefan Monnier
@ 2013-01-12 2:59 ` Felix H. Dahlke
0 siblings, 0 replies; 30+ messages in thread
From: Felix H. Dahlke @ 2013-01-12 2:59 UTC (permalink / raw)
To: Stefan Monnier; +Cc: 8576-done
On 1/12/13 12:25 AM, Stefan Monnier wrote:
>> So, it's been a while. Anything I can do to help get this merged?
> Sending such a `ping' is a great way to help ;-)
> Installed, thank you, and sorry for the delay,
Nevermind, could have done so earlier :) Thanks for merging!
^ permalink raw reply [flat|nested] 30+ messages in thread