bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-lisp-json.el-json-readtable-dispatch-Fix-error-data.patch --]
[-- Type: text/x-diff, Size: 820 bytes --]

From d297d156fec4d141c6287d3da4e7aa538280b43e Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Fri, 16 Feb 2018 17:11:49 +0000
Subject: [PATCH] * lisp/json.el (json-readtable-dispatch): Fix error data

---
 lisp/json.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/json.el b/lisp/json.el
index b03a482ca6..b3267df6e5 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -685,7 +685,7 @@ json-readtable-dispatch
       (push (list c 'json-read-number) table))
     (pcase-dolist (`(,c . ,rest) table)
       (push `((eq ,char ,c) (,@rest)) res))
-    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
+    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))
 
 (defun json-read ()
   "Parse and return the JSON object following point.
-- 
2.15.1


[-- Attachment #2: Type: text/plain, Size: 517 bytes --]


When signalling json-readtable-error, the macro json-readtable-dispatch
passes the unrecognised character as error data verbatim, rather than as
a list.  Attached is a patch which fixes this.

Thanks,

-- 
Basil

In GNU Emacs 26.0.91 (build 3, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2018-02-16 built on thunk
Repository revision: edc06adf96f4aa9d8b707181015acfe61d396edb
Windowing system distributor 'The X.Org Foundation', version 11.0.11906000
System Description:	Debian GNU/Linux testing (buster)

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Noam Postavsky]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
> +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))

> When signalling json-readtable-error, the macro json-readtable-dispatch
> passes the unrecognised character as error data verbatim, rather than as
> a list.  Attached is a patch which fixes this.

Should this be applied to emacs-26?  It seems to fix a regression
relative to Emacs 25 (where this error just had `nil' as the data) and
the patch looks safe enough.

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Date: Thu, 15 Mar 2018 22:39:18 -0400
> Cc: 30489@debbugs.gnu.org
> 
> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
> 
> > -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
> > +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))
> 
> > When signalling json-readtable-error, the macro json-readtable-dispatch
> > passes the unrecognised character as error data verbatim, rather than as
> > a list.  Attached is a patch which fixes this.
> 
> Should this be applied to emacs-26?  It seems to fix a regression
> relative to Emacs 25 (where this error just had `nil' as the data) and
> the patch looks safe enough.

What change(s) introduced the regression?

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Noam Postavsky]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Noam Postavsky <npostavs@gmail.com>
>> Date: Thu, 15 Mar 2018 22:39:18 -0400
>> Cc: 30489@debbugs.gnu.org
>> 
>> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>> 
>> > -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
>> > +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))
>> 
>> > When signalling json-readtable-error, the macro json-readtable-dispatch
>> > passes the unrecognised character as error data verbatim, rather than as
>> > a list.  Attached is a patch which fixes this.
>> 
>> Should this be applied to emacs-26?  It seems to fix a regression
>> relative to Emacs 25 (where this error just had `nil' as the data) and
>> the patch looks safe enough.
>
> What change(s) introduced the regression?

[1: 9314e6c56e]: 2017-09-04 03:40:30 -0400
  Embed JSON readtable into json-read
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9314e6c56e248a5060a6c125e2088c4fbffe123b

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@gmail.com>
> Cc: contovob@tcd.ie,  30489@debbugs.gnu.org
> Date: Fri, 16 Mar 2018 05:52:06 -0400
> 
> >> > -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
> >> > +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))
> >> 
> >> > When signalling json-readtable-error, the macro json-readtable-dispatch
> >> > passes the unrecognised character as error data verbatim, rather than as
> >> > a list.  Attached is a patch which fixes this.
> >> 
> >> Should this be applied to emacs-26?  It seems to fix a regression
> >> relative to Emacs 25 (where this error just had `nil' as the data) and
> >> the patch looks safe enough.
> >
> > What change(s) introduced the regression?
> 
> [1: 9314e6c56e]: 2017-09-04 03:40:30 -0400
>   Embed JSON readtable into json-read
>   https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=9314e6c56e248a5060a6c125e2088c4fbffe123b

Thanks, then the patch in question is OK for the release branch.

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Noam Postavsky]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> @@ -685,7 +685,7 @@ json-readtable-dispatch
>        (push (list c 'json-read-number) table))
>      (pcase-dolist (`(,c . ,rest) table)
>        (push `((eq ,char ,c) (,@rest)) res))
> -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
> +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))

Actually, shouldn't this be (list ,char)?

Otherwise, I get this warning when compiling:

In end of data:
../../lisp/json.el:766:1:Warning: the function ‘char’ is not known to be
    defined.

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Pass-json-readtable-error-data-as-a-list-bug-30489.patch --]
[-- Type: text/x-diff, Size: 874 bytes --]

From b917a7409659dde2ccc836d80741ad5a89d8785c Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Fri, 16 Feb 2018 17:11:49 +0000
Subject: [PATCH] Pass json-readtable-error data as a list (bug#30489)

* lisp/json.el (json-readtable-dispatch): Fix error data.
---
 lisp/json.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/json.el b/lisp/json.el
index b03a482ca6..d374f452e6 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -685,7 +685,7 @@ json-readtable-dispatch
       (push (list c 'json-read-number) table))
     (pcase-dolist (`(,c . ,rest) table)
       (push `((eq ,char ,c) (,@rest)) res))
-    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
+    `(cond ,@res (t (signal 'json-readtable-error (list ,char))))))
 
 (defun json-read ()
   "Parse and return the JSON object following point.
-- 
2.16.2


[-- Attachment #2: Type: text/plain, Size: 828 bytes --]


Noam Postavsky <npostavs@gmail.com> writes:

> "Basil L. Contovounesios" <contovob@tcd.ie> writes:
>
>> @@ -685,7 +685,7 @@ json-readtable-dispatch
>>        (push (list c 'json-read-number) table))
>>      (pcase-dolist (`(,c . ,rest) table)
>>        (push `((eq ,char ,c) (,@rest)) res))
>> -    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
>> +    `(cond ,@res (t (signal 'json-readtable-error ,(list char))))))
>
> Actually, shouldn't this be (list ,char)?
>
> Otherwise, I get this warning when compiling:
>
> In end of data:
> ../../lisp/json.el:766:1:Warning: the function ‘char’ is not known to be
>     defined.

Right you are; thanks for catching this embarrassing macro mistake.

I attach an updated patch including your fix and the bug number.

Thanks again,

-- 
Basil

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Noam Postavsky]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> I attach an updated patch including your fix and the bug number.

Thanks, could you add a regression test for this too, please?  Since
we've already had two mistakes independently generated, it seems likely
we'll make more if this code is updated again.

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Basil L. Contovounesios]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Pass-json-readtable-error-data-as-a-list-bug-30489.patch --]
[-- Type: text/x-diff, Size: 1596 bytes --]

From bed6240056d6ca8f47c5709fbe7033c490c8738c Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob@tcd.ie>
Date: Fri, 16 Feb 2018 17:11:49 +0000
Subject: [PATCH] Pass json-readtable-error data as a list (bug#30489)

* lisp/json.el (json-readtable-dispatch): Fix error data.
* test/lisp/json-tests.el (test-json-read): Check error data is a
list.
---
 lisp/json.el            | 2 +-
 test/lisp/json-tests.el | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/json.el b/lisp/json.el
index b03a482ca6..d374f452e6 100644
--- a/lisp/json.el
+++ b/lisp/json.el
@@ -685,7 +685,7 @@ json-readtable-dispatch
       (push (list c 'json-read-number) table))
     (pcase-dolist (`(,c . ,rest) table)
       (push `((eq ,char ,c) (,@rest)) res))
-    `(cond ,@res (t (signal 'json-readtable-error ,char)))))
+    `(cond ,@res (t (signal 'json-readtable-error (list ,char))))))
 
 (defun json-read ()
   "Parse and return the JSON object following point.
diff --git a/test/lisp/json-tests.el b/test/lisp/json-tests.el
index 47f8047b34..ea562e8b13 100644
--- a/test/lisp/json-tests.el
+++ b/test/lisp/json-tests.el
@@ -309,7 +309,8 @@ json-tests--with-temp-buffer
   (json-tests--with-temp-buffer ""
     (should-error (json-read) :type 'json-end-of-file))
   (json-tests--with-temp-buffer "xxx"
-    (should-error (json-read) :type 'json-readtable-error)))
+    (let ((err (should-error (json-read) :type 'json-readtable-error)))
+      (should (equal (cdr err) '(?x))))))
 
 (ert-deftest test-json-read-from-string ()
   (let ((json-string "{ \"a\": 1 }"))
-- 
2.16.2


[-- Attachment #2: Type: text/plain, Size: 304 bytes --]


Noam Postavsky <npostavs@gmail.com> writes:

> Thanks, could you add a regression test for this too, please?  Since
> we've already had two mistakes independently generated, it seems likely
> we'll make more if this code is updated again.

Of course.  Is the attached up to scratch?

Thanks,

-- 
Basil

bug#30489: 26.0.91; [PATCH] Data associated with json-readtable-error not a list

[Noam Postavsky]

tags 30489 fixed
close 30489 26.1
quit

"Basil L. Contovounesios" <contovob@tcd.ie> writes:

> Noam Postavsky <npostavs@gmail.com> writes:
>
>> Thanks, could you add a regression test for this too, please?  Since
>> we've already had two mistakes independently generated, it seems likely
>> we'll make more if this code is updated again.
>
> Of course.  Is the attached up to scratch?

Perfect, pushed to emacs-26.

[1: 7f27d42f0b]: 2018-03-18 23:13:29 -0400
  Pass json-readtable-error data as a list (bug#30489)
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=7f27d42f0b64c74625e45909a0bf389d68eddc62