From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED!not-for-mail From: npostavs@users.sourceforge.net Newsgroups: gmane.emacs.bugs Subject: bug#6806: another reason for the patch Date: Sat, 11 Feb 2017 23:21:02 -0500 Message-ID: <87o9y8kpjl.fsf@users.sourceforge.net> References: <874m0xhsp9.fsf@tromey.com> <871svdps7u.fsf@users.sourceforge.net> <87a89s69hz.fsf@tromey.com> NNTP-Posting-Host: blaine.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: blaine.gmane.org 1486873216 27500 195.159.176.226 (12 Feb 2017 04:20:16 GMT) X-Complaints-To: usenet@blaine.gmane.org NNTP-Posting-Date: Sun, 12 Feb 2017 04:20:16 +0000 (UTC) User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.2 (gnu/linux) Cc: 6806@debbugs.gnu.org To: Tom Tromey Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Sun Feb 12 05:20:12 2017 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by blaine.gmane.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccldz-0006eX-Mn for geb-bug-gnu-emacs@m.gmane.org; Sun, 12 Feb 2017 05:20:11 +0100 Original-Received: from localhost ([::1]:50671 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ccle2-0005jl-3W for geb-bug-gnu-emacs@m.gmane.org; Sat, 11 Feb 2017 23:20:14 -0500 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:36580) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ccldv-0005hu-K9 for bug-gnu-emacs@gnu.org; Sat, 11 Feb 2017 23:20:09 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ccldq-0001Qs-L7 for bug-gnu-emacs@gnu.org; Sat, 11 Feb 2017 23:20:07 -0500 Original-Received: from debbugs.gnu.org ([208.118.235.43]:37915) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1ccldq-0001Qo-Fw for bug-gnu-emacs@gnu.org; Sat, 11 Feb 2017 23:20:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1ccldq-0000UW-4J for bug-gnu-emacs@gnu.org; Sat, 11 Feb 2017 23:20:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: npostavs@users.sourceforge.net Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 12 Feb 2017 04:20:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 6806 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 6806-submit@debbugs.gnu.org id=B6806.14868732001876 (code B ref 6806); Sun, 12 Feb 2017 04:20:02 +0000 Original-Received: (at 6806) by debbugs.gnu.org; 12 Feb 2017 04:20:00 +0000 Original-Received: from localhost ([127.0.0.1]:36114 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccldn-0000UB-UT for submit@debbugs.gnu.org; Sat, 11 Feb 2017 23:20:00 -0500 Original-Received: from mail-it0-f48.google.com ([209.85.214.48]:35779) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ccldm-0000Tz-5x for 6806@debbugs.gnu.org; Sat, 11 Feb 2017 23:19:58 -0500 Original-Received: by mail-it0-f48.google.com with SMTP id 203so161686435ith.0 for <6806@debbugs.gnu.org>; Sat, 11 Feb 2017 20:19:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:from:to:cc:subject:references:date:in-reply-to:message-id :user-agent:mime-version; bh=HECHBqqE1gZQaEtb1I0alolVjvZkA9u2FugKZ8AVN+8=; b=Fm+gNuKJ6YZWGMm8Trod623kNt5JAg6FybId3VejOPMLeToPyZ9dMNjerKYTwfvAUz 7NGO+yNOPgjLaDjqCWY8y7iTcPQYL1DFdJhChMdJII4X73I33clG5vM5OT1sn9+U5OgI Qf1PZiElEqlaAWiS8nAzMrCrIUvzZoxseoL1BWw0kelHsMI/RmRjWvPRfk5Bd5w5tCwP bgXxXOcct4sUKPuTqZ9/FDmSNueRpIJ/9HEj4oZdyFgq+cVbY52efHQaaDtEzKyAJQ6V jlggjHFLxWM7wtUFGL6TZh0czkAUNtREDjwzqOI0wMfu4vFQ+rkJLuZHeLlC5Q2N2YM+ 0zKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:from:to:cc:subject:references:date :in-reply-to:message-id:user-agent:mime-version; bh=HECHBqqE1gZQaEtb1I0alolVjvZkA9u2FugKZ8AVN+8=; b=TL/2JJ/lMarV8ynSai/SDx2yuBkBd/yfqr1HuiQJlRD3MeQf97EYnidX8i2XrVxehw SmYMd8c5kPx2fFbgsz9l4drKXA5kfjUxfcFzZ5ReL+dwFZP9fw70zBejkGjyVViHQx+B VDEspbVu0h+AHv01leT15GJhFfOS6AdAgQfVOp1xWxZHbDMgEKt58jevmRzI0RFs01QG LVsaFi2AiKjq6VQ0knWlxW1Ao6Y36iSsMkYZ1q/8KBOxkwSiqpRB9ke9kpo3C7wCoAbV YXRem8YGiaTEBb7chHQ6nPXW1vaobkO7OK3Hp2A65V/TJYKW1JO/bQ7k895ubSgZSNWA OsOQ== X-Gm-Message-State: AIkVDXIhP/zLrwYw0Y6dk88gtzrQs98hO+PzDiHBOq/uJBNr5p8OKVXAsQfjGpM5L6sWMw== X-Received: by 10.36.194.67 with SMTP id i64mr36543596itg.68.1486873192515; Sat, 11 Feb 2017 20:19:52 -0800 (PST) Original-Received: from zony ([45.2.7.65]) by smtp.googlemail.com with ESMTPSA id j14sm283761itd.0.2017.02.11.20.19.51 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Sat, 11 Feb 2017 20:19:51 -0800 (PST) In-Reply-To: <87a89s69hz.fsf@tromey.com> (Tom Tromey's message of "Sat, 11 Feb 2017 08:19:04 -0700") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:129256 Archived-At: --=-=-= Content-Type: text/plain Tom Tromey 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: --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Test-comment-multi-line-nil-auto-fill-case-too.patch Content-Description: patch >From cbf522f2446035bcd34a676c2ef3e641b4f20f90 Mon Sep 17 00:00:00 2001 From: Noam Postavsky 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 --=-=-= Content-Type: text/plain 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 "[]*". --=-=-=--