bug#19399: 24.4; fill-paragraph always errors on second slash of // in js-mode

bug#19399: 24.4; fill-paragraph always errors on second slash of // in js-mode

[Taylor Venable]

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

#1 Visit a file: C-x C-f test.js
#2 Type this line: // emacs
#3 Put the cursor on the second slash and type M-q (fill-paragraph)
#4 Get an error: Wrong type argument: stringp, nil

Here's the debugger backtrace:

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  looking-at(nil)
  c-literal-limits(nil t)
  c-mask-paragraph(t nil fill-paragraph nil)
  c-fill-paragraph(nil)
  js-c-fill-paragraph(nil)
  fill-paragraph(nil t)
  call-interactively(fill-paragraph nil nil)
  command-execute(fill-paragraph)

Thanks for your time.



In GNU Emacs 24.4.1 (x86_64-unknown-linux-gnu, GTK+ Version 3.14.3)
 of 2014-10-21 on bitzer.hoetzel.info
Windowing system distributor `The X.Org Foundation', version 11.0.11602000
System Description:    Arch Linux

Configured using:
 `configure --prefix=/usr --sysconfdir=/etc --libexecdir=/usr/lib
 --localstatedir=/var --with-x-toolkit=gtk3 --with-xft
 'CFLAGS=-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong
 --param=ssp-buffer-size=4' CPPFLAGS=-D_FORTIFY_SOURCE=2
 LDFLAGS=-Wl,-O1,--sort-common,--as-needed,-z,relro'

Important settings:
  value of $LC_COLLATE: POSIX
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Javascript

Minor modes in effect:
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent input:
C-x C-f t e s t . j s <return> <right> M-q <help-echo>
<help-echo> <help-echo> <help-echo> <help-echo> <help-echo>
<help-echo> <menu-bar> <help-menu> <send-emacs-bug
-report>

Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.
c-literal-limits: Wrong type argument: stringp, nil

Load-path shadows:
None found.

Features:
(shadow sort gnus-util mail-extr emacsbug message format-spec rfc822 mml
mml-sec mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev
gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils js advice help-fns byte-opt bytecomp byte-compile
cconv json imenu thingatpt cc-mode cc-fonts easymenu cc-guess cc-menus
cc-cmds cc-styles cc-align cc-engine cc-vars cc-defs time-date tooltip
electric uniquify ediff-hook vc-hooks lisp-float-type mwheel x-win x-dnd
tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select
scroll-bar mouse jit-lock font-lock syntax facemenu font-core frame cham
georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese hebrew greek romanian slovak czech european ethiopic
indian cyrillic chinese case-table epa-hook jka-cmpr-hook help simple
abbrev minibuffer nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
dbusbind gfilenotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty emacs)

Memory information:
((conses 16 99799 9585)
 (symbols 48 20607 0)
 (miscs 40 38 135)
 (strings 32 18066 4471)
 (string-bytes 1 618611)
 (vectors 16 12570)
 (vector-slots 8 422502 3583)
 (floats 8 64 268)
 (intervals 56 197 0)
 (buffers 960 12)
 (heap 1024 40578 947))

[-- Attachment #2: Type: text/html, Size: 3955 bytes --]

bug#19399: js mode comment-filling fixlet

[Tom Tromey]

This patch fixes a couple js comment-filling bugs.

Dmitry mentioned comment-line-break-function in bug#22431; this patch
just sets it in js-mode.

For bug#19399, debugging showed that js-mode left
c-block-comment-start-regexp nil, so this sets it to the correct value.

I would normally check this in but I think it should probably be
reviewed.

Tom

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e3f64a8..375ae9d 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3760,6 +3760,8 @@ js-mode
         c-line-comment-starter "//"
         c-comment-start-regexp "/[*/]\\|\\s!"
         comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+  (setq-local comment-line-break-function #'c-indent-new-comment-line)
+  (setq-local c-block-comment-start-regexp "\\*/")
 
   (setq-local electric-indent-chars
 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".

bug#19399: js mode comment-filling fixlet

[npostavs]

Tom Tromey <tom@tromey.com> writes:

> For bug#19399, debugging showed that js-mode left
> c-block-comment-start-regexp nil, so this sets it to the correct value.

> +  (setq-local c-block-comment-start-regexp "\\*/")

Shouldn't that be "/\\*"?  Otherwise it looks like a comment-end regexp...

bug#19399: js mode comment-filling fixlet

[Tom Tromey]

>>>>> "npostavs" == npostavs  <npostavs@users.sourceforge.net> writes:

>> +  (setq-local c-block-comment-start-regexp "\\*/")

npostavs> Shouldn't that be "/\\*"?  Otherwise it looks like a comment-end regexp...

Yes, darn it.  Thanks for noticing that, I'd only tried it with "//"
comments (which previously were signalling).

I think I should perhaps write some tests for this patch.

Tom

bug#19399: js mode comment-filling fixlet

[Tom Tromey]

Tom> I think I should perhaps write some tests for this patch.

Here's the patch with tests.  I couldn't actually find a way to make the
contents of c-block-comment-start-regexp matter.

Tom

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index e3f64a8..1484b79 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -3760,6 +3760,8 @@ js-mode
         c-line-comment-starter "//"
         c-comment-start-regexp "/[*/]\\|\\s!"
         comment-start-skip "\\(//+\\|/\\*+\\)\\s *")
+  (setq-local comment-line-break-function #'c-indent-new-comment-line)
+  (setq-local c-block-comment-start-regexp "/\\*")
 
   (setq-local electric-indent-chars
 	      (append "{}():;," electric-indent-chars)) ;FIXME: js2-mode adds "[]*".
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
new file mode 100644
index 0000000..9bf7258
--- /dev/null
+++ b/test/lisp/progmodes/js-tests.el
@@ -0,0 +1,64 @@
+;;; js-tests.el --- Test suite for js-mode
+
+;; Copyright (C) 2017 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;;; Code:
+
+(require 'ert)
+(require 'js)
+
+(ert-deftest js-mode-fill-bug-19399 ()
+  (with-temp-buffer
+    (insert "/")
+    (save-excursion (insert "/ comment"))
+    (js-mode)
+    (fill-paragraph)
+    (should (equal (buffer-substring (point-min) (point-max))
+                   "// comment"))))
+
+(ert-deftest js-mode-fill-bug-22431 ()
+  (with-temp-buffer
+    (insert "/**\n")
+    (insert " * Load the inspector's shared head.js for use by tests that ")
+    (insert "need to open the something or other")
+    (js-mode)
+    ;; This fails with auto-fill but not fill-paragraph.
+    (do-auto-fill)
+    (should (equal (buffer-substring (point-min) (point-max))
+                   "/**
+ * Load the inspector's shared head.js for use by tests that need to
+ * open the something or other"))))
+
+(ert-deftest js-mode-fill-bug-22431-fill-paragraph-at-start ()
+  (with-temp-buffer
+    (insert "/**\n")
+    (insert " * Load the inspector's shared head.js for use by tests that ")
+    (insert "need to open the something or other")
+    (js-mode)
+    (goto-char (point-min))
+    (fill-paragraph)
+    (should (equal (buffer-substring (point-min) (point-max))
+                   "/**
+ * Load the inspector's shared head.js for use by tests that need to
+ * open the something or other"))))
+
+(provide 'js-tests)
+
+;;; js-tests.el ends here

bug#19399: js mode comment-filling fixlet

[Dmitry Gutov]

On 10.01.2017 06:40, Tom Tromey wrote:
> Tom> I think I should perhaps write some tests for this patch.
>
> Here's the patch with tests.  I couldn't actually find a way to make the
> contents of c-block-comment-start-regexp matter.

Thanks for the tests (here using ERT seems appropriate).

But if the contents of the variable don't matter, why not skip on 
setting it in js-mode?

bug#19399: js mode comment-filling fixlet

[Tom Tromey]

>>>>> "Dmitry" == Dmitry Gutov <dgutov@yandex.ru> writes:

Dmitry> But if the contents of the variable don't matter, why not skip on
Dmitry> setting it in js-mode?

If it is not a string, then an error occurs.
What I couldn't find is a way to make a test that fails when the value
is incorrect.  This is fine, in that the current value is correct; it
just would be nice to have a test.

Tom

bug#19399: done

[Tom Tromey]

These were both fixed by 05fe74bec239bebea84cb6803120321c367d67d3.

Tom