GNU bug report logs - #26070
26.0.50; js-mode slash insertion bug

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26070 in the body.
You can then email your comments to 26070 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Copley <rcopley <at> gmail.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>, tom <at> tromey.com
Subject: 26.0.50; js-mode slash insertion bug
Date: Sun, 12 Mar 2017 10:23:28 +0000

After inserting a slash before other text in a js-mode buffer,
typing more characters has no effect, until you type C-g to quit.

Recipe to reproduce from 'emacs -Q':
M-x js-mode RET
x   ;; inserts "x"
C-b ;; backward char
/   ;; inserts "/"
x   ;; --- no effect! ---
C-g ;; quit

I couldn't see what code is running, because C-g doesn't enter the
debugger in this situation even when debug-on-quit is true, but I
bisected it to this commit:

commit 862d6438cfa6c6c035033697751f3d002357b024
Author: Tom Tromey <tom <at> tromey.com>
Date:   Sun Feb 5 11:40:18 2017 -0700

    Recognize JS regexp literals more correctly

In GNU Emacs 26.0.50 (build 6, x86_64-w64-mingw32)
 of 2017-03-12 built on MACHINE
Repository revision: 026c2cbf354fab138a65ad7093f17fbb23edb23c
Windowing system distributor 'Microsoft Corp.', version 10.0.14393
Recent messages:
For information about GNU Emacs and the GNU system, type C-h C-a.

Configured using:
 'configure --prefix=/mingw64 --with-modules
 --enable-locallisppath=/c/emacs/site-lisp 'CFLAGS=-O0 -g -ggdb''

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND DBUS NOTIFY ACL GNUTLS LIBXML2 ZLIB
TOOLKIT_SCROLL_BARS MODULES

Important settings:
  value of $LANG: ENG
  locale-coding-system: cp1252

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-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

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message puny seq byte-opt subr-x gv
bytecomp byte-compile cl-extra help-mode cconv cl-loaddefs pcase cl-lib
dired dired-loaddefs format-spec rfc822 mml easymenu mml-sec
password-cache epa derived epg epg-config gnus-util rmail rmail-loaddefs
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils time-date mule-util tooltip eldoc electric uniquify
ediff-hook vc-hooks lisp-float-type mwheel dos-w32 ls-lisp disp-table
term/w32-win w32-win w32-vars term/common-win tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded 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 w32notify dbusbind w32 multi-tty make-network-process emacs)

Memory information:
((conses 16 99056 6862)
 (symbols 56 20091 1)
 (miscs 48 48 87)
 (strings 32 19084 4910)
 (string-bytes 1 584358)
 (vectors 16 14743)
 (vector-slots 8 478865 4018)
 (floats 8 54 62)
 (intervals 56 230 0)
 (buffers 976 11))

Message #8 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: Richard Copley <rcopley <at> gmail.com>
Cc: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>, tom <at> tromey.com
Subject: Re: 26.0.50; js-mode slash insertion bug
Date: Mon, 13 Mar 2017 14:50:56 +0100

Richard> I couldn't see what code is running, because C-g doesn't enter the
Richard> debugger in this situation even when debug-on-quit is true, but I
Richard> bisected it to this commit:

Thanks for finding this and bisecting it.

At first I thought the problem was that the regexp literal matching code
in js-syntax-propertize should leave point after the construct.
However, the call to js-syntax-propertize really ought to do that... but
this code is always requiring the trailing "/", which is what is going
wrong.

I think once js-syntax-propertize is called, the regexp should always
succeed, so this patch makes the trailing "/" optional.

Can you try this and let me know if it works for you?

Tom

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index 84c9111..fa865db 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1714,7 +1714,7 @@ js--syntax-propertize-regexp-regexp
               (not (any ?\] ?\\))
               (and "\\" not-newline)))
          "]")))
-   (group "/"))
+   (group (zero-or-one "/")))
   "Regular expression matching a JavaScript regexp literal.")
 
 (defun js-syntax-propertize-regexp (end)
@@ -1724,8 +1724,8 @@ js-syntax-propertize-regexp
       (goto-char (nth 8 ppss))
       (when (and (looking-at js--syntax-propertize-regexp-regexp)
                  ;; Don't touch text after END.
-                 (<= (match-end 1) end))
-        (put-text-property (match-beginning 1) (match-end 1)
+                 (or (not (match-end 1)) (<= (match-end 1) end)))
+        (put-text-property (match-beginning 1) (or (match-end 1) (match-end 0))
                            'syntax-table (string-to-syntax "\"/"))
         (goto-char (match-end 0))))))
 

Message #11 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Copley <rcopley <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: Re: 26.0.50; js-mode slash insertion bug
Date: Mon, 13 Mar 2017 19:12:31 +0000

> Can you try this and let me know if it works for you?

Thanks! I had a very quick play around and it seems to work.

Message #14 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tom Tromey <tom <at> tromey.com>, Richard Copley <rcopley <at> gmail.com>
Cc: 26070 <at> debbugs.gnu.org
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Tue, 14 Mar 2017 07:56:35 +0200

On 13.03.2017 15:50, Tom Tromey wrote:

> I think once js-syntax-propertize is called, the regexp should always
> succeed, so this patch makes the trailing "/" optional.

Does it solve the infloop in the case like:

/x[

as well?

If not, I think the following is a better patch (could use some 
tweaking, probably):

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index aed42a8..d9e6ef3 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1721,12 +1721,14 @@ js-syntax-propertize-regexp
     (when (eq (nth 3 ppss) ?/)
       ;; A /.../ regexp.
       (goto-char (nth 8 ppss))
-      (when (and (looking-at js--syntax-propertize-regexp-regexp)
-                 ;; Don't touch text after END.
-                 (<= (match-end 1) end))
-        (put-text-property (match-beginning 1) (match-end 1)
-                           'syntax-table (string-to-syntax "\"/"))
-        (goto-char (match-end 0))))))
+      (if (and (looking-at js--syntax-propertize-regexp-regexp)
+               ;; Don't touch text after END.
+               (<= (match-end 1) end))
+          (progn
+            (put-text-property (match-beginning 1) (match-end 1)
+                               'syntax-table (string-to-syntax "\"/"))
+            (goto-char (match-end 0)))
+        (goto-char (1+ (match-beginning 0)))))))

 (defun js-syntax-propertize (start end)
   ;; JavaScript allows immediate regular expression objects, written 
/.../.

Message #17 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Richard Copley <rcopley <at> gmail.com>, 26070 <at> debbugs.gnu.org,
 Tom Tromey <tom <at> tromey.com>
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Tue, 14 Mar 2017 12:06:02 +0100

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

>> I think once js-syntax-propertize is called, the regexp should always
>> succeed, so this patch makes the trailing "/" optional.

Dmitry> Does it solve the infloop in the case like:
Dmitry> /x[
Dmitry> as well?

Yes, with the patch I don't get an infloop no matter which way I insert
those characters.

Tom

Message #20 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Richard Copley <rcopley <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: 26070 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Sun, 19 Mar 2017 11:22:32 +0000

On 14 March 2017 at 11:06, Tom Tromey <tom <at> tromey.com> wrote:
>>>>>> "Dmitry" == Dmitry Gutov <dgutov <at> yandex.ru> writes:
>
>>> I think once js-syntax-propertize is called, the regexp should always
>>> succeed, so this patch makes the trailing "/" optional.
>
> Dmitry> Does it solve the infloop in the case like:
> Dmitry> /x[
> Dmitry> as well?
>
> Yes, with the patch I don't get an infloop no matter which way I insert
> those characters.
>
> Tom

Caught one!

Insert the text below in a JS-mode buffer, go to the beginning of the
blank first line in function h() and type "/". There is an infloop.

The infloop doesn't happen if the "/" in function g() is changed to
a different operator.

You can replace the 0s with other stuff, but you need at least that
many characters (delete a 0 or a space and there is no infloop).

function f() {
    function g()
    {
        1 / 2;
    }

    function h() {

        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00000000000000000000000000000000000000000000000000;
        00;
    }
}

Message #23 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 26070 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Wed, 22 Mar 2017 16:18:22 -0600

>>>>> "Richard" == Richard Copley <rcopley <at> gmail.com> writes:

Richard> Caught one!

Thank you.

Richard> Insert the text below in a JS-mode buffer, go to the beginning of the
Richard> blank first line in function h() and type "/". There is an infloop.

How did you find this?  I'm very curious.

Anyway, please try this patch on top of the previous one I sent.
Or if you'd prefer I can send a combined patch.

Tom

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index fa865db..c220353 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1722,12 +1722,13 @@ js-syntax-propertize-regexp
     (when (eq (nth 3 ppss) ?/)
       ;; A /.../ regexp.
       (goto-char (nth 8 ppss))
-      (when (and (looking-at js--syntax-propertize-regexp-regexp)
-                 ;; Don't touch text after END.
-                 (or (not (match-end 1)) (<= (match-end 1) end)))
-        (put-text-property (match-beginning 1) (or (match-end 1) (match-end 0))
+      (when (looking-at js--syntax-propertize-regexp-regexp)
+        ;; Don't touch text after END.
+        (when (> end (match-end 1))
+          (setq end (match-end 1)))
+        (put-text-property (match-beginning 1) end
                            'syntax-table (string-to-syntax "\"/"))
-        (goto-char (match-end 0))))))
+        (goto-char end)))))
 
 (defun js-syntax-propertize (start end)
   ;; JavaScript allows immediate regular expression objects, written /.../.

Message #26 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Richard Copley <rcopley <at> gmail.com>
To: Tom Tromey <tom <at> tromey.com>
Cc: 26070 <at> debbugs.gnu.org, Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Thu, 23 Mar 2017 07:00:58 +0000

On 22 March 2017 at 22:18, Tom Tromey <tom <at> tromey.com> wrote:
>>>>>> "Richard" == Richard Copley <rcopley <at> gmail.com> writes:
>
> Richard> Caught one!
>
> Thank you.
>
> Richard> Insert the text below in a JS-mode buffer, go to the beginning of the
> Richard> blank first line in function h() and type "/". There is an infloop.
>
> How did you find this?  I'm very curious.

I didn't go looking. The recipe is reduced from a program I was
writing. The infloop happened when I went to comment out a line.
Would have happened to somebody sooner or later :)

> Anyway, please try this patch on top of the previous one I sent.

Seems good, thanks very much.

> Or if you'd prefer I can send a combined patch.
>
> Tom
>
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index fa865db..c220353 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -1722,12 +1722,13 @@ js-syntax-propertize-regexp
>      (when (eq (nth 3 ppss) ?/)
>        ;; A /.../ regexp.
>        (goto-char (nth 8 ppss))
> -      (when (and (looking-at js--syntax-propertize-regexp-regexp)
> -                 ;; Don't touch text after END.
> -                 (or (not (match-end 1)) (<= (match-end 1) end)))
> -        (put-text-property (match-beginning 1) (or (match-end 1) (match-end 0))
> +      (when (looking-at js--syntax-propertize-regexp-regexp)
> +        ;; Don't touch text after END.
> +        (when (> end (match-end 1))
> +          (setq end (match-end 1)))
> +        (put-text-property (match-beginning 1) end
>                             'syntax-table (string-to-syntax "\"/"))
> -        (goto-char (match-end 0))))))
> +        (goto-char end)))))
>
>  (defun js-syntax-propertize (start end)
>    ;; JavaScript allows immediate regular expression objects, written /.../.

Message #29 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: Richard Copley <rcopley <at> gmail.com>
Cc: 26070 <at> debbugs.gnu.org, Tom Tromey <tom <at> tromey.com>,
 Dmitry Gutov <dgutov <at> yandex.ru>
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Thu, 23 Mar 2017 21:53:11 -0600

Richard> Seems good, thanks very much.

Thanks for giving it a try.

Here's the latest version of these two patches, with test cases.
Dmitry, please let me know what you think.

Tom

commit ff870ef0324a3e5c43cf2d183df5db72e824efca
Author: Tom Tromey <tom <at> tromey.com>
Date:   Mon Mar 13 21:53:59 2017 +0100

    fix two js-mode syntax propertization bugs
    
    Bug#26070:
    * lisp/progmodes/js.el (js--syntax-propertize-regexp-regexp): Add
    zero-or-one to regular expression.
    (js-syntax-propertize-regexp): Update.  Propertize body of regexp
    literal up to END.
    * test/lisp/progmodes/js-tests.el (js-mode-propertize-bug-1)
    (js-mode-propertize-bug-2): New tests.

diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
index aed42a8..3c720c0 100644
--- a/lisp/progmodes/js.el
+++ b/lisp/progmodes/js.el
@@ -1713,7 +1713,7 @@ js--syntax-propertize-regexp-regexp
               (not (any ?\] ?\\))
               (and "\\" not-newline)))
          "]")))
-   (group "/"))
+   (group (zero-or-one "/")))
   "Regular expression matching a JavaScript regexp literal.")
 
 (defun js-syntax-propertize-regexp (end)
@@ -1721,12 +1721,13 @@ js-syntax-propertize-regexp
     (when (eq (nth 3 ppss) ?/)
       ;; A /.../ regexp.
       (goto-char (nth 8 ppss))
-      (when (and (looking-at js--syntax-propertize-regexp-regexp)
-                 ;; Don't touch text after END.
-                 (<= (match-end 1) end))
-        (put-text-property (match-beginning 1) (match-end 1)
+      (when (looking-at js--syntax-propertize-regexp-regexp)
+        ;; Don't touch text after END.
+        (when (> end (match-end 1))
+          (setq end (match-end 1)))
+        (put-text-property (match-beginning 1) end
                            'syntax-table (string-to-syntax "\"/"))
-        (goto-char (match-end 0))))))
+        (goto-char end)))))
 
 (defun js-syntax-propertize (start end)
   ;; JavaScript allows immediate regular expression objects, written /.../.
diff --git a/test/lisp/progmodes/js-tests.el b/test/lisp/progmodes/js-tests.el
index e030675..8e1bac1 100644
--- a/test/lisp/progmodes/js-tests.el
+++ b/test/lisp/progmodes/js-tests.el
@@ -140,6 +140,43 @@
       (font-lock-ensure)
       (should (eq (get-text-property (point) 'face) (caddr test))))))
 
+(ert-deftest js-mode-propertize-bug-1 ()
+  (with-temp-buffer
+    (js-mode)
+    (save-excursion (insert "x"))
+    (insert "/")
+    ;; The bug was a hang.
+    (should t)))
+
+(ert-deftest js-mode-propertize-bug-2 ()
+  (with-temp-buffer
+    (js-mode)
+    (insert "function f() {
+    function g()
+    {
+        1 / 2;
+    }
+
+    function h() {
+")
+    (save-excursion
+      (insert "
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00000000000000000000000000000000000000000000000000;
+        00;
+    }
+}
+"))
+    (insert "/")
+    ;; The bug was a hang.
+    (should t)))
+
 (provide 'js-tests)
 
 ;;; js-tests.el ends here

Message #32 received at 26070 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tom Tromey <tom <at> tromey.com>, Richard Copley <rcopley <at> gmail.com>
Cc: 26070 <at> debbugs.gnu.org
Subject: Re: bug#26070: 26.0.50; js-mode slash insertion bug
Date: Fri, 31 Mar 2017 10:53:38 +0300

On 24.03.2017 05:53, Tom Tromey wrote:

> Here's the latest version of these two patches, with test cases.
> Dmitry, please let me know what you think.

It's okay with me, thanks. As long as we're sure that 
js--syntax-propertize-regexp-regexp will always match.

Message #37 received at 26070-done <at> debbugs.gnu.org (full text, mbox):

From: Tom Tromey <tom <at> tromey.com>
To: 26070-done <at> debbugs.gnu.org
Subject: fixed
Date: Sat, 01 Apr 2017 15:15:28 -0600

I've checked in the patch.

Tom

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.