bug#15582: notes on js-mode indentation bug

[Tom Tromey]

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