[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#6806: another reason for the patch
From: |
npostavs |
Subject: |
bug#6806: another reason for the patch |
Date: |
Sat, 11 Feb 2017 23:21:02 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) |
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:
>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
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
"[]*".