GNU bug report logs - #32793
27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type

Previous Next

Package: emacs;

Reported by: Xu Chunyang <mail <at> xuchunyang.me>

Date: Fri, 21 Sep 2018 12:58:01 UTC

Severity: minor

Found in version 27.0.50

Done: Dmitry Gutov <dgutov <at> yandex.ru>

Bug is archived. No further changes may be made.

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

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

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 21 Sep 2018 12:58:01 GMT) Full text and rfc822 format available.

Acknowledgement sent to Xu Chunyang <mail <at> xuchunyang.me>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 21 Sep 2018 12:58:01 GMT) Full text and rfc822 format available.

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

From: Xu Chunyang <mail <at> xuchunyang.me>
To: bug-gnu-emacs <at> gnu.org
Subject: 27.0.50; json-parse-string doesn't have the equivalent of json.el's
 json-array-type
Date: Fri, 21 Sep 2018 20:56:43 +0800
Hello,

We can parse JSON Array as Lisp List with json.el, e.g.,

(let ((json-array-type 'list))
  (json-read-from-string "[1,2,3]"))
;; => (1 2 3)

but the new json-parse-string doesn't have the equivalent, thus porting
existing code to using json-parse-string might be difficult.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Thu, 11 Apr 2019 16:27:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Xu Chunyang <mail <at> xuchunyang.me>, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Thu, 11 Apr 2019 19:26:23 +0300
On 21.09.2018 15:56, Xu Chunyang wrote:

> We can parse JSON Array as Lisp List with json.el, e.g.,
> 
> (let ((json-array-type 'list))
>    (json-read-from-string "[1,2,3]"))
> ;; => (1 2 3)
> 
> but the new json-parse-string doesn't have the equivalent, thus porting
> existing code to using json-parse-string might be difficult.

I also stumbled on this problem when trying to port existing code to 
native JSON.

It's kind of surprising, considering lists are more common in Elisp, and 
there's no JSON data structure that would correspond to them (so I can't 
simply return a different format from the backing server process, I'll 
have to convert).




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Thu, 11 Apr 2019 16:47:01 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50;
 json-parse-string doesn't have the equivalent of json.el's
 json-array-type
Date: Thu, 11 Apr 2019 19:46:14 +0300
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Thu, 11 Apr 2019 19:26:23 +0300
> 
> On 21.09.2018 15:56, Xu Chunyang wrote:
> 
> > We can parse JSON Array as Lisp List with json.el, e.g.,
> > 
> > (let ((json-array-type 'list))
> >    (json-read-from-string "[1,2,3]"))
> > ;; => (1 2 3)
> > 
> > but the new json-parse-string doesn't have the equivalent, thus porting
> > existing code to using json-parse-string might be difficult.
> 
> I also stumbled on this problem when trying to port existing code to 
> native JSON.

Sounds like a simple extension of the existing code, so patches are
welcome to implement this feature.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 00:35:01 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 03:34:05 +0300
[Message part 1 (text/plain, inline)]
On 11.04.2019 19:46, Eli Zaretskii wrote:

> Sounds like a simple extension of the existing code, so patches are
> welcome to implement this feature.

Very well, patch attached.

What do you think?
[json-array-type.diff (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 09:24:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 12:22:44 +0300
> Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 12 Apr 2019 03:34:05 +0300
> 
> > Sounds like a simple extension of the existing code, so patches are
> > welcome to implement this feature.
> 
> Very well, patch attached.
> 
> What do you think?

Thanks, some comments below.

> +enum json_array_type {
> +  json_array_array,
> +  json_array_list
> +};
> +
>  struct json_configuration {
>    enum json_object_type object_type;
> +  enum json_array_type array_type;
>    Lisp_Object null_object;
>    Lisp_Object false_object;
>  };

Can't say I like this conversion of Lisp symbols into C enumerations.
I'd rather we used symbols (Qarray, Qlist, etc.), but I can understand
why you did this as json.c already did for the other keyword values.

> @@ -521,7 +527,7 @@ static void
>  json_parse_args (ptrdiff_t nargs,
>                   Lisp_Object *args,
>                   struct json_configuration *conf,
> -                 bool configure_object_type)
> +                 bool configure_types)

If we are renaming this argument, let's do a better job: I think its
name should have been parse_object_types.

Come to think of this: why do we need this boolean at all?  The
callers which don't want :object-type parsed will ignore the result
anyway, so it sounds like something we could just toss.

> +          case json_array_list:
> +            {
> +              result = Qnil;
> +              for (ptrdiff_t i = 0; i < size; ++i)
> +                result = Fcons (json_to_lisp (json_array_get (json, i), conf),
> +                                result);
> +              result = Fnreverse (result);

If you cons the list back to front, you can avoid the Fnreverse call,
which will make this faster.

Also, please insert a call to rarely_quit into the loop, as JSON
vectors could be quite large, AFAIU.

Finally, this needs documentation update: the doc strings of
json-parse-string and json-parse-buffer, NEWS, and the ELisp manual.

Thanks again for working on this.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 15:03:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 18:02:22 +0300
[Message part 1 (text/plain, inline)]
On 12.04.2019 12:22, Eli Zaretskii wrote:

> Can't say I like this conversion of Lisp symbols into C enumerations.
> I'd rather we used symbols (Qarray, Qlist, etc.), but I can understand
> why you did this as json.c already did for the other keyword values.

I vaguely suspect it might help with performance. *shrug* Or not.

>> @@ -521,7 +527,7 @@ static void
>>   json_parse_args (ptrdiff_t nargs,
>>                    Lisp_Object *args,
>>                    struct json_configuration *conf,
>> -                 bool configure_object_type)
>> +                 bool configure_types)
> 
> If we are renaming this argument, let's do a better job: I think its
> name should have been parse_object_types.

OK.

> Come to think of this: why do we need this boolean at all?  The
> callers which don't want :object-type parsed will ignore the result
> anyway, so it sounds like something we could just toss.

I'd rather we didn't accept argument we cannot handle, to avoid false 
expectations.

For example with this patch we can parse a JSON array into a Lisp list.

But there's no way to serialize a list back into a JSON array, yet.

I looked at implementing it, just for completeness (it's not necessary 
for my use case, for now). But there's an ambiguity between lists and 
alists which is not trivial to solve (i.e. when object-type is alist and 
array-type is list, how do we determinine, quickly and reliably, that a 
given list is actually an alist?). So maybe leave it for the future.

>> +          case json_array_list:
>> +            {
>> +              result = Qnil;
>> +              for (ptrdiff_t i = 0; i < size; ++i)
>> +                result = Fcons (json_to_lisp (json_array_get (json, i), conf),
>> +                                result);
>> +              result = Fnreverse (result);
> 
> If you cons the list back to front, you can avoid the Fnreverse call,
> which will make this faster.

Done. No real performance impact that I can see, but it didn't hurt either.

> Also, please insert a call to rarely_quit into the loop, as JSON
> vectors could be quite large, AFAIU.

Also done. In the json_array_array case as well, where it was missing, 
it seems.

> Finally, this needs documentation update: the doc strings of
> json-parse-string and json-parse-buffer, NEWS, and the ELisp manual.

json-parse-buffer and NEWS don't need updating, I think.

Otherwise, done, see the new patch.
[json-array-type.diff (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 15:27:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 18:26:03 +0300
> Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 12 Apr 2019 18:02:22 +0300
> 
> > Come to think of this: why do we need this boolean at all?  The
> > callers which don't want :object-type parsed will ignore the result
> > anyway, so it sounds like something we could just toss.
> 
> I'd rather we didn't accept argument we cannot handle, to avoid false 
> expectations.
> 
> For example with this patch we can parse a JSON array into a Lisp list.
> 
> But there's no way to serialize a list back into a JSON array, yet.

This argument is meaningless for serializing, I think.  But OK.

> >> +              for (ptrdiff_t i = 0; i < size; ++i)
> >> +                result = Fcons (json_to_lisp (json_array_get (json, i), conf),
> >> +                                result);
> >> +              result = Fnreverse (result);
> > 
> > If you cons the list back to front, you can avoid the Fnreverse call,
> > which will make this faster.
> 
> Done. No real performance impact that I can see, but it didn't hurt either.

You just didn't try an array big enough for this to matter.

> > Also, please insert a call to rarely_quit into the loop, as JSON
> > vectors could be quite large, AFAIU.
> 
> Also done. In the json_array_array case as well, where it was missing, 
> it seems.

I don't mind, although in that case the loop just assigns value to a
vector that was already consed.

> json-parse-buffer and NEWS don't need updating, I think.

They don't?  Why not?

> Otherwise, done, see the new patch.

LGTM, with the above question, and this one gotcha:

> @@ -918,6 +961,9 @@ a list of keyword/argument pairs:
>  The keyword argument `:object-type' specifies which Lisp type is used
>  to represent objects; it can be `hash-table', `alist' or `plist'.
>  
> +The keyword argument `:array-type' specifies which Lisp type is used
> +to represent arrays; it can be `array' or `list'.

Please say here that 'array' is the default.

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 15:46:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 18:45:46 +0300
[Message part 1 (text/plain, inline)]
On 12.04.2019 18:26, Eli Zaretskii wrote:

> This argument is meaningless for serializing, I think.  But OK.

I wouldn't be so sure. For instance, one could attempt to resolve the 
alist-list conflict using a combination of :object-type and :array-type 
parameters. This isn't going to work, at least not yet. It's better to 
fail earlier.

> You just didn't try an array big enough for this to matter.

My array was big enough for the switch from json.el to json.c to make a 
real difference.

Anyway, it depends on how "big" the elements inside the array are as 
well. In my case they're fairly complex too.

> I don't mind, although in that case the loop just assigns value to a
> vector that was already consed.

It also calls

  json_to_lisp (json_array_get (json, i), conf)

in that loop.

>> json-parse-buffer and NEWS don't need updating, I think.
> 
> They don't?  Why not?

There was no json.c in Emacs 26. NEWS describes the changes from the 
previous release, and the current entry only lists the public functions.

json-parse-buffer doesn't enumerate the keyword arguments either.

>> +The keyword argument `:array-type' specifies which Lisp type is used
>> +to represent arrays; it can be `array' or `list'.
> 
> Please say here that 'array' is the default.

OK. I've had to update the preceding paragraph as well.

Good to install?
[json-array-type.diff (text/x-patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#32793; Package emacs. (Fri, 12 Apr 2019 17:44:02 GMT) Full text and rfc822 format available.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Fri, 12 Apr 2019 20:42:43 +0300
> Cc: mail <at> xuchunyang.me, 32793 <at> debbugs.gnu.org
> From: Dmitry Gutov <dgutov <at> yandex.ru>
> Date: Fri, 12 Apr 2019 18:45:46 +0300
> 
> Good to install?

Yes, thanks.




Reply sent to Dmitry Gutov <dgutov <at> yandex.ru>:
You have taken responsibility. (Fri, 12 Apr 2019 22:37:01 GMT) Full text and rfc822 format available.

Notification sent to Xu Chunyang <mail <at> xuchunyang.me>:
bug acknowledged by developer. (Fri, 12 Apr 2019 22:37:02 GMT) Full text and rfc822 format available.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: mail <at> xuchunyang.me, 32793-done <at> debbugs.gnu.org
Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent
 of json.el's json-array-type
Date: Sat, 13 Apr 2019 01:36:39 +0300
On 12.04.2019 20:42, Eli Zaretskii wrote:

>> Good to install?
> 
> Yes, thanks.

Thank you.

Pushed to master, closing.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sat, 11 May 2019 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 323 days ago.

Previous Next


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