bug#6806: another reason for the patch

[npostavs@users.sourceforge.net]

[-- Attachment #1: Type: text/plain, Size: 810 bytes --]

Tom Tromey <tom@tromey.com> writes:

> Noam> So the opened comment was correctly terminated, and a new one started,
> Noam> just as documented.  Perhaps you don't like that `comment-multi-line' is
> Noam> nil by default, but it doesn't look like a bug to me.
>
> I'm not sure what to do about this now.
> Should I revert the patch?
>
> If it were solely up to me I would leave the patch in, since I think
> non-nil comment-multi-line is more normal in JS code.  And, if the patch
> is reverted then I think comment-multi-line needs a :safe setting so it
> can by set in .dir-locals.el.

I don't really have much of an opinion regarding the best value for
comment-multi-line.  But we should test that filling works correctly
regardless of its value, and we should add a :safe setting anyway.  Here
is a patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 2160 bytes --]

From cbf522f2446035bcd34a676c2ef3e641b4f20f90 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 11 Feb 2017 23:15:13 -0500
Subject: [PATCH] Test comment-multi-line = nil auto fill case too

* test/lisp/progmodes/js-tests.el (js-mode-auto-fill): Test with
`comment-multi-line' both nil and non-nil.
* lisp/newcomment.el (comment-multi-line): Mark safe if it's a
boolean.
---
 lisp/newcomment.el              |  1 +
 test/lisp/progmodes/js-tests.el | 22 ++++++++++++----------
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/lisp/newcomment.el b/lisp/newcomment.el
index 1af89293b6..4b261c34c6 100644
--- a/lisp/newcomment.el
+++ b/lisp/newcomment.el
@@ -309,6 +309,7 @@ comment-multi-line
 It also affects \\[indent-new-comment-line].  However, if you want this
 behavior for explicit filling, you might as well use \\[newline-and-indent]."
   :type 'boolean
+  :safe #'booleanp
   :group 'comment)
 
 (defcustom comment-empty-lines nil
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index d61f084e0d..99f5898525 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -89,16 +89,18 @@
 (ert-deftest js-mode-auto-fill ()
   (with-temp-buffer
     (js-mode)
-    (setq fill-column 70)
-    (insert "/* ")
-    (dotimes (_ 16)
-      (insert "test "))
-    (do-auto-fill)
-    ;; The bug is that, after auto-fill, the second line starts with
-    ;; "/*", whereas it should start with " * ".
-    (goto-char (point-min))
-    (forward-line)
-    (should (looking-at " \\* test"))))
+    (let ((fill-column 10)
+          (comment-multi-line t))
+      (insert "/* test test")
+      (do-auto-fill)
+      ;; Filling should continue the multi line comment.
+      (should (equal (buffer-string) "/* test\n * test"))
+      (erase-buffer)
+      (insert "/* test test")
+      (setq comment-multi-line nil)
+      (do-auto-fill)
+      ;; Filling should start a new comment on the next line.
+      (should (equal (buffer-string) "/* test */\n/* test")))))
 
 (ert-deftest js-mode-regexp-syntax-bug-25529 ()
   (dolist (regexp-contents '("[^[]"
-- 
2.11.1


[-- Attachment #3: Type: text/plain, Size: 500 bytes --]


Perhaps we should also revert the js-mode comment-multi-line setting,
not sure.

--- i/lisp/progmodes/js.el
+++ w/lisp/progmodes/js.el
@@ -3856,7 +3856,6 @@ js-mode
         comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
   (setq-local comment-line-break-function #'c-indent-new-comment-line)
   (setq-local c-block-comment-start-regexp "/\\*")
-  (setq-local comment-multi-line t)
 
   (setq-local electric-indent-chars
 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".