Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Dmitry Gutov]

Hi Eli,

Any chance I may backport this to emacs-26?

The bug was reported only recently, but it seems pretty annoying 
(infloop during indentation), and the fix is simple.

On 01.02.2019 03:02, Dmitry Gutov wrote:
> branch: master
> commit b01a4295c2f9bb58858880e4e28b05cc8396791c
> Author: Dmitry Gutov <dgutov@yandex.ru>
> Commit: Dmitry Gutov <dgutov@yandex.ru>
> 
>      js--re-search-backward-inner: Fix infloop
>      
>      Fix JS indentation infloop reported in
>      https://github.com/mooz/js2-mode/issues/513.
>      
>      * lisp/progmodes/js.el (js--re-search-backward-inner): Account for
>      multiline string literals.
>      * test/manual/indent/js.js: New test example.
> ---
>   lisp/progmodes/js.el     | 9 +--------
>   test/manual/indent/js.js | 7 +++++++
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/lisp/progmodes/js.el b/lisp/progmodes/js.el
> index e55539c..a94a2fe 100644
> --- a/lisp/progmodes/js.el
> +++ b/lisp/progmodes/js.el
> @@ -785,7 +785,6 @@ macro as normal text."
>   (defun js--re-search-backward-inner (regexp &optional bound count)
>     "Auxiliary function for `js--re-search-backward'."
>     (let ((parse)
> -        str-terminator
>           (orig-macro-start
>            (save-excursion
>              (and (js--beginning-of-macro)
> @@ -796,13 +795,7 @@ macro as normal text."
>                    (save-excursion (backward-char) (looking-at "/[/*]")))
>           (forward-char))
>         (setq parse (syntax-ppss))
> -      (cond ((setq str-terminator (nth 3 parse))
> -             (when (eq str-terminator t)
> -               (setq str-terminator ?/))
> -             (re-search-backward
> -              (concat "\\([^\\]\\|^\\)" (string str-terminator))
> -              (point-at-bol) t))
> -            ((nth 7 parse)
> +      (cond ((nth 8 parse)
>                (goto-char (nth 8 parse)))
>               ((or (nth 4 parse)
>                    (and (eq (char-before) ?/) (eq (char-after) ?*)))
> diff --git a/test/manual/indent/js.js b/test/manual/indent/js.js
> index b0d8bca..df79098 100644
> --- a/test/manual/indent/js.js
> +++ b/test/manual/indent/js.js
> @@ -144,6 +144,13 @@ bar(
>     /abc/
>   )
>   
> +// No infloop inside js--re-search-backward-inner
> +let b = {
> +  a : `
> +  //1
> +  `
> +}
> +
>   // Local Variables:
>   // indent-tabs-mode: nil
>   // js-indent-level: 2
> 
> _______________________________________________
> Emacs-diffs mailing list
> Emacs-diffs@gnu.org
> https://lists.gnu.org/mailman/listinfo/emacs-diffs
> 

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Fri, 1 Feb 2019 03:27:20 +0300
> 
> Any chance I may backport this to emacs-26?

When was this issue introduced?

> The bug was reported only recently, but it seems pretty annoying 
> (infloop during indentation), and the fix is simple.

Taken at face value, it doesn't look simple: you are switching from
using 2 elements of what syntax-ppss returns to a 3rd one.  How do we
know this won't introduce other issues?  Maybe if you describe the
logic of the change, I will agree with you.

Thanks.

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Dmitry Gutov]

On 01.02.2019 10:28, Eli Zaretskii wrote:

> When was this issue introduced?

One could say it was in 77ab7f84550993b44550879e10cd917b7bf02d04 
(2015-03-05) where support for multiline string literals was added, but 
the code expecting the strings to be exactly single-line was not updated.

>> The bug was reported only recently, but it seems pretty annoying
>> (infloop during indentation), and the fix is simple.
> 
> Taken at face value, it doesn't look simple: you are switching from
> using 2 elements of what syntax-ppss returns to a 3rd one.  How do we
> know this won't introduce other issues?  Maybe if you describe the
> logic of the change, I will agree with you.

The purpose of the code is get out of the current literal. In the clause 
that was changes, to go to the beginning of the enclosing string.

(goto-char (nth 8 parse)) is a reliable way to do that. The same 
function does that for comments anyway.

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sat, 2 Feb 2019 03:08:09 +0300
> 
> On 01.02.2019 10:28, Eli Zaretskii wrote:
> 
> > When was this issue introduced?
> 
> One could say it was in 77ab7f84550993b44550879e10cd917b7bf02d04 
> (2015-03-05) where support for multiline string literals was added, but 
> the code expecting the strings to be exactly single-line was not updated.
> 
> >> The bug was reported only recently, but it seems pretty annoying
> >> (infloop during indentation), and the fix is simple.
> > 
> > Taken at face value, it doesn't look simple: you are switching from
> > using 2 elements of what syntax-ppss returns to a 3rd one.  How do we
> > know this won't introduce other issues?  Maybe if you describe the
> > logic of the change, I will agree with you.
> 
> The purpose of the code is get out of the current literal. In the clause 
> that was changes, to go to the beginning of the enclosing string.
> 
> (goto-char (nth 8 parse)) is a reliable way to do that. The same 
> function does that for comments anyway.

OK, please backport this.

Thanks.

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Dmitry Gutov]

On 02.02.2019 10:31, Eli Zaretskii wrote:

>> The purpose of the code is get out of the current literal. In the clause
>> that was changes, to go to the beginning of the enclosing string.
>>
>> (goto-char (nth 8 parse)) is a reliable way to do that. The same
>> function does that for comments anyway.
> 
> OK, please backport this.

I thought I did, but apparently forgot. :-(

Please let me know if it's still okay to do that. I saw a new pretest 
yesterday, so maybe not.

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 24 Feb 2019 02:43:58 +0300
> 
> On 02.02.2019 10:31, Eli Zaretskii wrote:
> 
> > OK, please backport this.
> 
> I thought I did, but apparently forgot. :-(
> 
> Please let me know if it's still okay to do that. I saw a new pretest 
> yesterday, so maybe not.

Still okay.

Re: [Emacs-diffs] master b01a429: js--re-search-backward-inner: Fix infloop

[Dmitry Gutov]

On 24.02.2019 06:36, Eli Zaretskii wrote:

> Still okay.

Now done. Thank you!