GNU bug report logs - #15582
js-mode indent bug

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 15582 in the body.
You can then email your comments to 15582 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jakub Jankiewicz <jcubic <at> onet.pl>
To: bug-gnu-emacs <at> gnu.org
Subject: js-mode indent bug
Date: Thu, 10 Oct 2013 14:06:48 +0200

[Message part 1 (text/plain, inline)]

Hi,

I found this bug in js-mode indent:

when you have multiline if statement with comment at the end, next line
indent is doubled.

If I have js-indent-level equal 8 the code look like this:

            if (x > left && x < left + image.width &&
                y > top && y < top + image.height) {
                    do_somethig();
            }

but when I put comment after if it look like this:

            if (x > left && x < left + image.width &&
                y > top && y < top + image.height) {  // found
                            do_somethig();


when js-indent-level is 2 it have 4 spaces. The same is with while

          while (10 &&
                 20) { // asd
              xx();

four spaces instead of two

version: GNU Emacs 24.2.1


--
Jakub Jankiewicz, Web Developer
http://jcubic.pl

[signature.asc (application/pgp-signature, attachment)]

Message #8 received at 15582 <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: 15582 <at> debbugs.gnu.org
Cc: Daniel Colascione <dan.colascione <at> gmail.com>
Subject: notes on js-mode indentation bug
Date: Mon, 09 Jan 2017 22:03:59 -0700

I looked into this bug.

With the comment, js--continued-expression-p returns t, causing
js--proper-indentation to take this branch of a cond:

(continued-expr-p
 (+ (current-column) (* 2 js-indent-level)
    js-expr-indent-offset))

Without the comment, this doesn't happen, so I think the problem here is
that js--continued-expression-p returns t.  Digging into this a bit
more, the issue is that js--re-search-backward moves point to the wrong
newline, because the newline at the end of the "//" comment is rejected.

This patch works by introducing a new js--find-newline-backward that
tries to do the right thing for line comments.

I've included a new test case (as a patch to a file first appearing in
another pending patch), but considering that there aren't any other
indentation tests for js-mode, I think some review is necessary.

It would be helpful to know when the continued-expr-p branch really is
supposed to trigger.  Or, if there is an informal corpus of indentation
tests, it would be nice to put them all into js-tests.el.

Tom

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 1484b79..0551f2a 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1771,6 +1771,24 @@ js--looking-at-operator-p
                  ;; return NaN anyway.  Shouldn't be a problem.
                  (memq (char-before) '(?, ?} ?{))))))))
 
+(defun js--find-newline-backward ()
+  "Move backward to the nearest newline that is not in a block comment."
+  (let ((continue t)
+        (result t))
+    (while continue
+      (setq continue nil)
+      (if (re-search-backward "\n" nil t)
+          (let ((parse (syntax-ppss)))
+            ;; We match the end of a // comment but not a newline in a
+            ;; block comment.
+            (when (nth 4 parse)
+              (goto-char (nth 8 parse))
+              ;; If we saw a block comment, keep trying.
+              (unless (nth 7 parse)
+                (setq continue t))))
+        (setq result nil)))
+    result))
+
 (defun js--continued-expression-p ()
   "Return non-nil if the current line continues an expression."
   (save-excursion
@@ -1780,7 +1798,7 @@ js--continued-expression-p
             (progn
               (forward-comment (- (point)))
               (not (memq (char-before) '(?, ?\[ ?\()))))
-      (and (js--re-search-backward "\n" nil t)
+      (and (js--find-newline-backward)
            (progn
              (skip-chars-backward " \t")
              (or (bobp) (backward-char))
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index 9bf7258..de322f2 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -59,6 +59,16 @@
  * Load the inspector's shared head.js for use by tests that need to
  * open the something or other"))))
 
+(ert-deftest js-mode-indent-bug-15582 ()
+  (with-temp-buffer
+    (js-mode)
+    (setq-local js-indent-level 8)
+    (insert "if (x > 72 &&\n    y < 85) { // found\n\t")
+    (save-excursion (insert "do_something();\n"))
+    (js-indent-line)
+    (should (equal (buffer-substring (point-at-bol) (point-at-eol))
+                   "\tdo_something();"))))
+
 (provide 'js-tests)
 
 ;;; js-tests.el ends here

Message #11 received at 15582 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tom Tromey <tom <at> tromey.com>, 15582 <at> debbugs.gnu.org
Cc: Daniel Colascione <dan.colascione <at> gmail.com>
Subject: Re: bug#15582: notes on js-mode indentation bug
Date: Sat, 14 Jan 2017 06:51:25 +0300

On 10.01.2017 08:03, Tom Tromey wrote:

> It would be helpful to know when the continued-expr-p branch really is
> supposed to trigger.  Or, if there is an informal corpus of indentation
> tests, it would be nice to put them all into js-tests.el.

It triggers, for instance, in this example:

b +=
  c

But you have probably found it already.

> Tom
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index 1484b79..0551f2a 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -1771,6 +1771,24 @@ js--looking-at-operator-p
>                   ;; return NaN anyway.  Shouldn't be a problem.
>                   (memq (char-before) '(?, ?} ?{))))))))
>
> +(defun js--find-newline-backward ()
> +  "Move backward to the nearest newline that is not in a block comment."

You might want to phrase it like "Move ... ignoring comments.", meaning 
treating the openers of non-block comments as newlines, and the contents 
of block comments as entirely invisible.

BTW, here are a couple of examples that the new fix doesn't handle:

b += /* found */
  c

b += /*
   */
  c

(I think they suggest that the block comment-related behavior of the new 
function might be tweaked a little).

Thanks.

Message #16 received at 15582-done <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: 15582-done <at> debbugs.gnu.org
Subject: done
Date: Sat, 14 Jan 2017 10:45:44 -0700

This was fixed by b47f97218efb8d9966e084bdfd8a86e8c47cf81d.

Tom

Message #19 received at 15582 <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 15582 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Daniel Colascione <dan.colascione <at> gmail.com>
Subject: Re: bug#15582: notes on js-mode indentation bug
Date: Mon, 16 Jan 2017 09:11:17 -0700

Dmitry> BTW, here are a couple of examples that the new fix doesn't handle:

Thanks.  I'll try to fix these.

Tom

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.