[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: |
Eli Zaretskii |
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 12:22:44 +0300 |
> Cc: mail@xuchunyang.me, 32793@debbugs.gnu.org
> From: Dmitry Gutov <dgutov@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.
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Dmitry Gutov, 2019/04/11
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Eli Zaretskii, 2019/04/11
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Dmitry Gutov, 2019/04/11
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type,
Eli Zaretskii <=
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Dmitry Gutov, 2019/04/12
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Eli Zaretskii, 2019/04/12
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Dmitry Gutov, 2019/04/12
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Eli Zaretskii, 2019/04/12
- bug#32793: 27.0.50; json-parse-string doesn't have the equivalent of json.el's json-array-type, Dmitry Gutov, 2019/04/12