[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Accept plists when serializing and parsing JSON
From: |
Eli Zaretskii |
Subject: |
Re: [PATCH] Accept plists when serializing and parsing JSON |
Date: |
Fri, 01 Jun 2018 12:39:38 +0300 |
> From: João Távora <address@hidden>
> Date: Tue, 29 May 2018 15:59:06 +0100
>
> So I found this other thread
>
> https://lists.gnu.org/archive/html/emacs-devel/2017-12/msg00667.html
>
> where plist support for json.c is briefly requested and discussed, but
> there didn't seem to be an overwhelming argument against it, so I had
> another look at the file and it didn't seem very hard or problematic to
> implement.
>
> Anyway, I had a go.
Thanks, please see a few comments below.
> address@hidden json-serialize-use-plists
> address@hidden serializing plists as json
> + For the serialization function, the variable
> address@hidden controls the converse process,
json-serialize-use-plists is the literal name of a variable, so it
should have the @code markup, not @var.
> +resolving the ambiguity when a list is found in the Lisp object to
> +serialize. If @code{nil}, its default, the list is interpreted as an
^^
Two spaces between sentences, please.
> + if ( EQ (Vjson_serialize_use_plists, Qt) ) {
> + key_symbol = XCAR (tail);
> + tail = XCDR(tail);
> + CHECK_CONS (tail);
> + value = XCAR (tail);
> + if (EQ (tail, li.tortoise)) circular_list (lisp);
> + } else {
> + Lisp_Object pair = XCAR (tail);
Our style is to write
if (something)
{
then-part;
}
else
{
else-part;
}
Also, please don't leav any whitespace between the opening parenthesis
and the following character, and similarly with closing parens. IOW,
this:
if ( EQ (Vjson_serialize_use_plists, Qt) )
should be this instead:
if (EQ (Vjson_serialize_use_plists, Qt))
> + json_object_foreach (json, key_str, value)
> + {
> + /* No idea if using AUTO_STRING and Fconcat for
> + making keywords is idiomatic, but seems to work
> + nicely */
> + AUTO_STRING (colon, ":");
> + Lisp_Object key =
> + Fintern (CALLN (Fconcat, colon, json_build_string
> (key_str))
> + , Qnil);
> + result = Fcons (key, result); /* build the plist as
> + value-key since
> + we're going to
> + reverse it in the
> + end.*/
> + result = Fcons (json_to_lisp (value, object_type), result);
> + }
Here I'd use intern_1 instead, it would allow you to avoid
unnecessarily consing Lisp objects. (Yes, I realize that the same
comment applies to the existing code.)
> + /* If non-nil use plists instead of alists in
> json-serialize.*/);
^
Comma is missing there.