bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of jso


From: Dmitry Gutov
Subject: 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
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1

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.

Attachment: json-array-type.diff
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]