Subject: Re: bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type

From: Eli Zaretskii <[email-masked]>
Date: Fri, 12 Apr 2019 18:26:03 +0300

> Cc: [email-masked], [email-masked]
> From: Dmitry Gutov <[email-masked]>
> 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.