JSON->lisp Mapping: Hash vs AList

JSON->lisp Mapping: Hash vs AList

[raman]

Build Emacs from @Head and started playing with the native
implementation of JSON parsing -- it works well and is much faster
than the lisp version as expected.

After writing some code with it, I have a feature request --- could we
set it up so that the caller can specify that json-hashes map to lisp
alists -- rather than lisp hash-tables? 

Justification:

1. JSON dicts tend to be deeply nested -- but rarely have a large
number of entries -- so  lisp  hash-tables may be overkill.

2. The deeply nested nature of JSON dicts makes accessing things at
deeper levels  require nested  calls to  gethash -- leads to code that
is opaque.

3. JS code succinctly accesses such nested data as a.b.c --- mapping
json dicts to lisp alists gives the same expressiveness when using
let-alist -- for an example see 
http://emacspeak.blogspot.com/2017/07/data-binding-in-emacs-lisp-let-alist.html

--raman 


-- 

-- 

-- 

Re: JSON->lisp Mapping: Hash vs AList

[Nicolas Petton]

[-- Attachment #1: Type: text/plain, Size: 614 bytes --]

raman <raman@google.com> writes:

> 2. The deeply nested nature of JSON dicts makes accessing things at
> deeper levels  require nested  calls to  gethash -- leads to code that
> is opaque.

I agree that an option for alists would be nice.

However, you could use `map-nested-elt' to traverse any map (hash-table
or alist):

    (map-nested-elt my-table '(key1 key2 key3))

map.el also provides `map-let' and a "map" `pcase' pattern, which can be
used to traverse nested hash-tables (and also works on alists, or a mix):

    (pcase-let (((map ('key1 (map ('key2 (map key3))))) my-table))
      ...)

Cheers,
Nico

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[raman]

Thanks -- Hadn't used those patterns before -- they're nice
-- 

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 465 bytes --]

raman <raman@google.com> schrieb am Di., 12. Dez. 2017 um 02:40 Uhr:

> Build Emacs from @Head and started playing with the native
> implementation of JSON parsing -- it works well and is much faster
> than the lisp version as expected.
>
> After writing some code with it, I have a feature request --- could we
> set it up so that the caller can specify that json-hashes map to lisp
> alists -- rather than lisp hash-tables?
>

Sounds reasonable, here is a patch.

[-- Attachment #1.2: Type: text/html, Size: 756 bytes --]

[-- Attachment #2: 0001-Allow-JSON-parser-functions-to-return-alists.txt --]
[-- Type: text/plain, Size: 11879 bytes --]

From eb1162893fadd928a893a79fe0ddeaf827e5dfc5 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Wed, 13 Dec 2017 23:35:07 +0100
Subject: [PATCH] Allow JSON parser functions to return alists

* src/json.c (Fjson_parse_string, Fjson_parse_buffer): Give these
functions a keyword argument to specify the return type for JSON
objects.
(json_to_lisp): Convert objects to alists if requested.
(json_parse_object_type): New helper function to parse keyword
arguments.

* test/src/json-tests.el (json-parse-string/object): Add a unit test.

* doc/lispref/text.texi (Parsing JSON): Document new functionality.
---
 doc/lispref/text.texi  |  20 ++++----
 src/json.c             | 128 +++++++++++++++++++++++++++++++++++++------------
 test/src/json-tests.el |  16 ++++---
 3 files changed, 119 insertions(+), 45 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 5b288d9750..7517dbcfb6 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -4965,14 +4965,13 @@ Parsing JSON
 
 @item
 JSON has only one map type, the object.  JSON objects are represented
-using Lisp hashtables.
+using Lisp hashtables or alists.
 
 @end itemize
 
 @noindent
-Note that @code{nil} doesn't represent any JSON values: this is to
-avoid confusion, because @code{nil} could either represent
-@code{null}, @code{false}, or an empty array, all of which are
+Note that @code{nil} represents the empty JSON object, @code{@{@}},
+not @code{null}, @code{false}, or an empty array, all of which are
 different JSON values.
 
   If some Lisp object can't be represented in JSON, the serialization
@@ -4995,8 +4994,13 @@ Parsing JSON
 
   Only top-level values (arrays and objects) can be serialized to
 JSON.  The subobjects within these top-level values can be of any
-type.  Likewise, the parsing functions will only return vectors and
-hashtables.
+type.  Likewise, the parsing functions will only return vectors,
+hashtables, and alists.
+
+  The parsing functions accept keyword arguments.  Currently only one
+keyword argument, @code{:object-type}, is recognized; its value can be
+either @code{hash-table} to parse JSON objects as hashtables with
+string keys (the default) or @code{alist} to parse them as alists.
 
 @defun json-serialize object
 This function returns a new Lisp string which contains the JSON
@@ -5008,12 +5012,12 @@ Parsing JSON
 current buffer before point.
 @end defun
 
-@defun json-parse-string string
+@defun json-parse-string string &key (object-type 'hash-table)
 This function parses the JSON value in @var{string}, which must be a
 Lisp string.
 @end defun
 
-@defun json-parse-buffer
+@defun json-parse-buffer &key (object-type 'hash-table)
 This function reads the next JSON value from the current buffer,
 starting at point.  It moves point to the position immediately after
 the value if a value could be read and converted to Lisp; otherwise it
diff --git a/src/json.c b/src/json.c
index 7025ae165c..34c568d76f 100644
--- a/src/json.c
+++ b/src/json.c
@@ -507,10 +507,15 @@ OBJECT.  */)
   return unbind_to (count, Qnil);
 }
 
+enum json_object_type {
+  json_object_hashtable,
+  json_object_alist,
+};
+
 /* Convert a JSON object to a Lisp object.  */
 
 static _GL_ARG_NONNULL ((1)) Lisp_Object
-json_to_lisp (json_t *json)
+json_to_lisp (json_t *json, enum json_object_type object_type)
 {
   switch (json_typeof (json))
     {
@@ -544,7 +549,7 @@ json_to_lisp (json_t *json)
         Lisp_Object result = Fmake_vector (make_natnum (size), Qunbound);
         for (ptrdiff_t i = 0; i < size; ++i)
           ASET (result, i,
-                json_to_lisp (json_array_get (json, i)));
+                json_to_lisp (json_array_get (json, i), object_type));
         --lisp_eval_depth;
         return result;
       }
@@ -552,23 +557,49 @@ json_to_lisp (json_t *json)
       {
         if (++lisp_eval_depth > max_lisp_eval_depth)
           xsignal0 (Qjson_object_too_deep);
-        size_t size = json_object_size (json);
-        if (FIXNUM_OVERFLOW_P (size))
-          xsignal0 (Qoverflow_error);
-        Lisp_Object result = CALLN (Fmake_hash_table, QCtest, Qequal,
-                                    QCsize, make_natnum (size));
-        struct Lisp_Hash_Table *h = XHASH_TABLE (result);
-        const char *key_str;
-        json_t *value;
-        json_object_foreach (json, key_str, value)
+        Lisp_Object result;
+        switch (object_type)
           {
-            Lisp_Object key = json_build_string (key_str);
-            EMACS_UINT hash;
-            ptrdiff_t i = hash_lookup (h, key, &hash);
-            /* Keys in JSON objects are unique, so the key can’t be
-               present yet.  */
-            eassert (i < 0);
-            hash_put (h, key, json_to_lisp (value), hash);
+          case json_object_hashtable:
+            {
+              size_t size = json_object_size (json);
+              if (FIXNUM_OVERFLOW_P (size))
+                xsignal0 (Qoverflow_error);
+              result = CALLN (Fmake_hash_table, QCtest, Qequal, QCsize,
+                              make_natnum (size));
+              struct Lisp_Hash_Table *h = XHASH_TABLE (result);
+              const char *key_str;
+              json_t *value;
+              json_object_foreach (json, key_str, value)
+                {
+                  Lisp_Object key = json_build_string (key_str);
+                  EMACS_UINT hash;
+                  ptrdiff_t i = hash_lookup (h, key, &hash);
+                  /* Keys in JSON objects are unique, so the key can’t
+                     be present yet.  */
+                  eassert (i < 0);
+                  hash_put (h, key, json_to_lisp (value, object_type), hash);
+                }
+              break;
+            }
+          case json_object_alist:
+            {
+              result = Qnil;
+              const char *key_str;
+              json_t *value;
+              json_object_foreach (json, key_str, value)
+                {
+                  Lisp_Object key = Fintern (json_build_string (key_str), Qnil);
+                  result
+                    = Fcons (Fcons (key, json_to_lisp (value, object_type)),
+                             result);
+                }
+              result = Fnreverse (result);
+              break;
+            }
+          default:
+            /* Can’t get here.  */
+            emacs_abort ();
           }
         --lisp_eval_depth;
         return result;
@@ -578,15 +609,43 @@ json_to_lisp (json_t *json)
   emacs_abort ();
 }
 
-DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, 1, NULL,
+static enum json_object_type
+json_parse_object_type (ptrdiff_t nargs, Lisp_Object *args)
+{
+  switch (nargs)
+    {
+    case 0:
+      return json_object_hashtable;
+    case 2: {
+      Lisp_Object key = args[0];
+      Lisp_Object value = args[1];
+      if (!EQ (key, QCobject_type))
+        wrong_choice (list1 (QCobject_type), key);
+      if (EQ (value, Qhash_table))
+        return json_object_hashtable;
+      else if (EQ (value, Qalist))
+        return json_object_alist;
+      else
+        wrong_choice (list2 (Qhash_table, Qalist), value);
+    }
+    default:
+      wrong_type_argument (Qplistp, Flist (nargs, args));
+    }
+}
+
+DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, MANY,
+       NULL,
        doc: /* Parse the JSON STRING into a Lisp object.
 This is essentially the reverse operation of `json-serialize', which
-see.  The returned object will be a vector or hashtable.  Its elements
-will be `:null', `:false', t, numbers, strings, or further vectors and
-hashtables.  If there are duplicate keys in an object, all but the
-last one are ignored.  If STRING doesn't contain a valid JSON object,
-an error of type `json-parse-error' is signaled.  */)
-  (Lisp_Object string)
+see.  The returned object will be a vector, hashtable, or alist.  Its
+elements will be `:null', `:false', t, numbers, strings, or further
+vectors, hashtables, and alists.  If there are duplicate keys in an
+object, all but the last one are ignored.  If STRING doesn't contain a
+valid JSON object, an error of type `json-parse-error' is signaled.
+The keyword argument OBJECT-TYPE specifies which Lisp type is used to
+represent objects; it can be `hash-table' or `alist'.
+usage: (string &key (OBJECT-TYPE \\='hash-table))  */)
+  (ptrdiff_t nargs, Lisp_Object *args)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -605,8 +664,11 @@ an error of type `json-parse-error' is signaled.  */)
     }
 #endif
 
+  Lisp_Object string = args[0];
   Lisp_Object encoded = json_encode (string);
   check_string_without_embedded_nulls (encoded);
+  enum json_object_type object_type
+    = json_parse_object_type (nargs - 1, args + 1);
 
   json_error_t error;
   json_t *object = json_loads (SSDATA (encoded), 0, &error);
@@ -617,7 +679,7 @@ an error of type `json-parse-error' is signaled.  */)
   if (object != NULL)
     record_unwind_protect_ptr (json_release_object, object);
 
-  return unbind_to (count, json_to_lisp (object));
+  return unbind_to (count, json_to_lisp (object, object_type));
 }
 
 struct json_read_buffer_data
@@ -650,12 +712,13 @@ json_read_buffer_callback (void *buffer, size_t buflen, void *data)
 }
 
 DEFUN ("json-parse-buffer", Fjson_parse_buffer, Sjson_parse_buffer,
-       0, 0, NULL,
+       0, MANY, NULL,
        doc: /* Read JSON object from current buffer starting at point.
 This is similar to `json-parse-string', which see.  Move point after
 the end of the object if parsing was successful.  On error, point is
-not moved.  */)
-  (void)
+not moved.
+usage: (&key (OBJECT-TYPE \\='hash-table))  */)
+  (ptrdiff_t nargs, Lisp_Object *args)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -674,6 +737,8 @@ not moved.  */)
     }
 #endif
 
+  enum json_object_type object_type = json_parse_object_type (nargs, args);
+
   ptrdiff_t point = PT_BYTE;
   struct json_read_buffer_data data = {.point = point};
   json_error_t error;
@@ -687,7 +752,7 @@ not moved.  */)
   record_unwind_protect_ptr (json_release_object, object);
 
   /* Convert and then move point only if everything succeeded.  */
-  Lisp_Object lisp = json_to_lisp (object);
+  Lisp_Object lisp = json_to_lisp (object, object_type);
 
   /* Adjust point by how much we just read.  */
   point += error.position;
@@ -750,6 +815,9 @@ syms_of_json (void)
   Fput (Qjson_parse_string, Qpure, Qt);
   Fput (Qjson_parse_string, Qside_effect_free, Qt);
 
+  DEFSYM (QCobject_type, ":object-type");
+  DEFSYM (Qalist, "alist");
+
   defsubr (&Sjson_serialize);
   defsubr (&Sjson_insert);
   defsubr (&Sjson_parse_string);
diff --git a/test/src/json-tests.el b/test/src/json-tests.el
index 07eb41d093..da51aac8c8 100644
--- a/test/src/json-tests.el
+++ b/test/src/json-tests.el
@@ -52,13 +52,15 @@
 
 (ert-deftest json-parse-string/object ()
   (skip-unless (fboundp 'json-parse-string))
-  (let ((actual
-         (json-parse-string
-          "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n")))
-    (should (hash-table-p actual))
-    (should (equal (hash-table-count actual) 2))
-    (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
-                   '(("abc" . [9 :false]) ("def" . :null))))))
+  (let ((input
+         "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n"))
+    (let ((actual (json-parse-string input)))
+      (should (hash-table-p actual))
+      (should (equal (hash-table-count actual) 2))
+      (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
+                     '(("abc" . [9 :false]) ("def" . :null)))))
+    (should (equal (json-parse-string input :object-type 'alist)
+                   '((abc . [9 :false]) (def . :null))))))
 
 (ert-deftest json-parse-string/string ()
   (skip-unless (fboundp 'json-parse-string))
-- 
2.15.1

Re: JSON->lisp Mapping: Hash vs AList

[T.V Raman]

Nice!
Could you check it into master once others have had a chance to comment/

Philipp Stephani writes:
 >    raman <[1]raman@google.com> schrieb am Di., 12. Dez. 2017 um 02:40 Uhr:
 > 
 >      Build Emacs from @Head and started playing with the native
 >      implementation of JSON parsing -- it works well and is much faster
 >      than the lisp version as expected.
 >      After writing some code with it, I have a feature request --- could
 >      we
 >      set it up so that the caller can specify that json-hashes map to
 >      lisp
 >      alists -- rather than lisp hash-tables?
 > 
 >    Sounds reasonable, here is a patch.
 > 
 > References
 > 
 >    1. mailto:raman@google.com
 > 
 > ----------------------------------------------------------------------
 > From eb1162893fadd928a893a79fe0ddeaf827e5dfc5 Mon Sep 17 00:00:00 2001
 > From: Philipp Stephani <phst@google.com>
 > Date: Wed, 13 Dec 2017 23:35:07 +0100
 > Subject: [PATCH] Allow JSON parser functions to return alists
 > 
 > * src/json.c (Fjson_parse_string, Fjson_parse_buffer): Give these
 > functions a keyword argument to specify the return type for JSON
 > objects.
 > (json_to_lisp): Convert objects to alists if requested.
 > (json_parse_object_type): New helper function to parse keyword
 > arguments.
 > 
 > * test/src/json-tests.el (json-parse-string/object): Add a unit test.
 > 
 > * doc/lispref/text.texi (Parsing JSON): Document new functionality.
 > ---
 >  doc/lispref/text.texi  |  20 ++++----
 >  src/json.c             | 128 +++++++++++++++++++++++++++++++++++++------------
 >  test/src/json-tests.el |  16 ++++---
 >  3 files changed, 119 insertions(+), 45 deletions(-)
 > 
 > diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
 > index 5b288d9750..7517dbcfb6 100644
 > --- a/doc/lispref/text.texi
 > +++ b/doc/lispref/text.texi
 > @@ -4965,14 +4965,13 @@ Parsing JSON
 >  
 >  @item
 >  JSON has only one map type, the object.  JSON objects are represented
 > -using Lisp hashtables.
 > +using Lisp hashtables or alists.
 >  
 >  @end itemize
 >  
 >  @noindent
 > -Note that @code{nil} doesn't represent any JSON values: this is to
 > -avoid confusion, because @code{nil} could either represent
 > -@code{null}, @code{false}, or an empty array, all of which are
 > +Note that @code{nil} represents the empty JSON object, @code{@{@}},
 > +not @code{null}, @code{false}, or an empty array, all of which are
 >  different JSON values.
 >  
 >    If some Lisp object can't be represented in JSON, the serialization
 > @@ -4995,8 +4994,13 @@ Parsing JSON
 >  
 >    Only top-level values (arrays and objects) can be serialized to
 >  JSON.  The subobjects within these top-level values can be of any
 > -type.  Likewise, the parsing functions will only return vectors and
 > -hashtables.
 > +type.  Likewise, the parsing functions will only return vectors,
 > +hashtables, and alists.
 > +
 > +  The parsing functions accept keyword arguments.  Currently only one
 > +keyword argument, @code{:object-type}, is recognized; its value can be
 > +either @code{hash-table} to parse JSON objects as hashtables with
 > +string keys (the default) or @code{alist} to parse them as alists.
 >  
 >  @defun json-serialize object
 >  This function returns a new Lisp string which contains the JSON
 > @@ -5008,12 +5012,12 @@ Parsing JSON
 >  current buffer before point.
 >  @end defun
 >  
 > -@defun json-parse-string string
 > +@defun json-parse-string string &key (object-type 'hash-table)
 >  This function parses the JSON value in @var{string}, which must be a
 >  Lisp string.
 >  @end defun
 >  
 > -@defun json-parse-buffer
 > +@defun json-parse-buffer &key (object-type 'hash-table)
 >  This function reads the next JSON value from the current buffer,
 >  starting at point.  It moves point to the position immediately after
 >  the value if a value could be read and converted to Lisp; otherwise it
 > diff --git a/src/json.c b/src/json.c
 > index 7025ae165c..34c568d76f 100644
 > --- a/src/json.c
 > +++ b/src/json.c
 > @@ -507,10 +507,15 @@ OBJECT.  */)
 >    return unbind_to (count, Qnil);
 >  }
 >  
 > +enum json_object_type {
 > +  json_object_hashtable,
 > +  json_object_alist,
 > +};
 > +
 >  /* Convert a JSON object to a Lisp object.  */
 >  
 >  static _GL_ARG_NONNULL ((1)) Lisp_Object
 > -json_to_lisp (json_t *json)
 > +json_to_lisp (json_t *json, enum json_object_type object_type)
 >  {
 >    switch (json_typeof (json))
 >      {
 > @@ -544,7 +549,7 @@ json_to_lisp (json_t *json)
 >          Lisp_Object result = Fmake_vector (make_natnum (size), Qunbound);
 >          for (ptrdiff_t i = 0; i < size; ++i)
 >            ASET (result, i,
 > -                json_to_lisp (json_array_get (json, i)));
 > +                json_to_lisp (json_array_get (json, i), object_type));
 >          --lisp_eval_depth;
 >          return result;
 >        }
 > @@ -552,23 +557,49 @@ json_to_lisp (json_t *json)
 >        {
 >          if (++lisp_eval_depth > max_lisp_eval_depth)
 >            xsignal0 (Qjson_object_too_deep);
 > -        size_t size = json_object_size (json);
 > -        if (FIXNUM_OVERFLOW_P (size))
 > -          xsignal0 (Qoverflow_error);
 > -        Lisp_Object result = CALLN (Fmake_hash_table, QCtest, Qequal,
 > -                                    QCsize, make_natnum (size));
 > -        struct Lisp_Hash_Table *h = XHASH_TABLE (result);
 > -        const char *key_str;
 > -        json_t *value;
 > -        json_object_foreach (json, key_str, value)
 > +        Lisp_Object result;
 > +        switch (object_type)
 >            {
 > -            Lisp_Object key = json_build_string (key_str);
 > -            EMACS_UINT hash;
 > -            ptrdiff_t i = hash_lookup (h, key, &hash);
 > -            /* Keys in JSON objects are unique, so the key can’t be
 > -               present yet.  */
 > -            eassert (i < 0);
 > -            hash_put (h, key, json_to_lisp (value), hash);
 > +          case json_object_hashtable:
 > +            {
 > +              size_t size = json_object_size (json);
 > +              if (FIXNUM_OVERFLOW_P (size))
 > +                xsignal0 (Qoverflow_error);
 > +              result = CALLN (Fmake_hash_table, QCtest, Qequal, QCsize,
 > +                              make_natnum (size));
 > +              struct Lisp_Hash_Table *h = XHASH_TABLE (result);
 > +              const char *key_str;
 > +              json_t *value;
 > +              json_object_foreach (json, key_str, value)
 > +                {
 > +                  Lisp_Object key = json_build_string (key_str);
 > +                  EMACS_UINT hash;
 > +                  ptrdiff_t i = hash_lookup (h, key, &hash);
 > +                  /* Keys in JSON objects are unique, so the key can’t
 > +                     be present yet.  */
 > +                  eassert (i < 0);
 > +                  hash_put (h, key, json_to_lisp (value, object_type), hash);
 > +                }
 > +              break;
 > +            }
 > +          case json_object_alist:
 > +            {
 > +              result = Qnil;
 > +              const char *key_str;
 > +              json_t *value;
 > +              json_object_foreach (json, key_str, value)
 > +                {
 > +                  Lisp_Object key = Fintern (json_build_string (key_str), Qnil);
 > +                  result
 > +                    = Fcons (Fcons (key, json_to_lisp (value, object_type)),
 > +                             result);
 > +                }
 > +              result = Fnreverse (result);
 > +              break;
 > +            }
 > +          default:
 > +            /* Can’t get here.  */
 > +            emacs_abort ();
 >            }
 >          --lisp_eval_depth;
 >          return result;
 > @@ -578,15 +609,43 @@ json_to_lisp (json_t *json)
 >    emacs_abort ();
 >  }
 >  
 > -DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, 1, NULL,
 > +static enum json_object_type
 > +json_parse_object_type (ptrdiff_t nargs, Lisp_Object *args)
 > +{
 > +  switch (nargs)
 > +    {
 > +    case 0:
 > +      return json_object_hashtable;
 > +    case 2: {
 > +      Lisp_Object key = args[0];
 > +      Lisp_Object value = args[1];
 > +      if (!EQ (key, QCobject_type))
 > +        wrong_choice (list1 (QCobject_type), key);
 > +      if (EQ (value, Qhash_table))
 > +        return json_object_hashtable;
 > +      else if (EQ (value, Qalist))
 > +        return json_object_alist;
 > +      else
 > +        wrong_choice (list2 (Qhash_table, Qalist), value);
 > +    }
 > +    default:
 > +      wrong_type_argument (Qplistp, Flist (nargs, args));
 > +    }
 > +}
 > +
 > +DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, MANY,
 > +       NULL,
 >         doc: /* Parse the JSON STRING into a Lisp object.
 >  This is essentially the reverse operation of `json-serialize', which
 > -see.  The returned object will be a vector or hashtable.  Its elements
 > -will be `:null', `:false', t, numbers, strings, or further vectors and
 > -hashtables.  If there are duplicate keys in an object, all but the
 > -last one are ignored.  If STRING doesn't contain a valid JSON object,
 > -an error of type `json-parse-error' is signaled.  */)
 > -  (Lisp_Object string)
 > +see.  The returned object will be a vector, hashtable, or alist.  Its
 > +elements will be `:null', `:false', t, numbers, strings, or further
 > +vectors, hashtables, and alists.  If there are duplicate keys in an
 > +object, all but the last one are ignored.  If STRING doesn't contain a
 > +valid JSON object, an error of type `json-parse-error' is signaled.
 > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
 > +represent objects; it can be `hash-table' or `alist'.
 > +usage: (string &key (OBJECT-TYPE \\='hash-table))  */)
 > +  (ptrdiff_t nargs, Lisp_Object *args)
 >  {
 >    ptrdiff_t count = SPECPDL_INDEX ();
 >  
 > @@ -605,8 +664,11 @@ an error of type `json-parse-error' is signaled.  */)
 >      }
 >  #endif
 >  
 > +  Lisp_Object string = args[0];
 >    Lisp_Object encoded = json_encode (string);
 >    check_string_without_embedded_nulls (encoded);
 > +  enum json_object_type object_type
 > +    = json_parse_object_type (nargs - 1, args + 1);
 >  
 >    json_error_t error;
 >    json_t *object = json_loads (SSDATA (encoded), 0, &error);
 > @@ -617,7 +679,7 @@ an error of type `json-parse-error' is signaled.  */)
 >    if (object != NULL)
 >      record_unwind_protect_ptr (json_release_object, object);
 >  
 > -  return unbind_to (count, json_to_lisp (object));
 > +  return unbind_to (count, json_to_lisp (object, object_type));
 >  }
 >  
 >  struct json_read_buffer_data
 > @@ -650,12 +712,13 @@ json_read_buffer_callback (void *buffer, size_t buflen, void *data)
 >  }
 >  
 >  DEFUN ("json-parse-buffer", Fjson_parse_buffer, Sjson_parse_buffer,
 > -       0, 0, NULL,
 > +       0, MANY, NULL,
 >         doc: /* Read JSON object from current buffer starting at point.
 >  This is similar to `json-parse-string', which see.  Move point after
 >  the end of the object if parsing was successful.  On error, point is
 > -not moved.  */)
 > -  (void)
 > +not moved.
 > +usage: (&key (OBJECT-TYPE \\='hash-table))  */)
 > +  (ptrdiff_t nargs, Lisp_Object *args)
 >  {
 >    ptrdiff_t count = SPECPDL_INDEX ();
 >  
 > @@ -674,6 +737,8 @@ not moved.  */)
 >      }
 >  #endif
 >  
 > +  enum json_object_type object_type = json_parse_object_type (nargs, args);
 > +
 >    ptrdiff_t point = PT_BYTE;
 >    struct json_read_buffer_data data = {.point = point};
 >    json_error_t error;
 > @@ -687,7 +752,7 @@ not moved.  */)
 >    record_unwind_protect_ptr (json_release_object, object);
 >  
 >    /* Convert and then move point only if everything succeeded.  */
 > -  Lisp_Object lisp = json_to_lisp (object);
 > +  Lisp_Object lisp = json_to_lisp (object, object_type);
 >  
 >    /* Adjust point by how much we just read.  */
 >    point += error.position;
 > @@ -750,6 +815,9 @@ syms_of_json (void)
 >    Fput (Qjson_parse_string, Qpure, Qt);
 >    Fput (Qjson_parse_string, Qside_effect_free, Qt);
 >  
 > +  DEFSYM (QCobject_type, ":object-type");
 > +  DEFSYM (Qalist, "alist");
 > +
 >    defsubr (&Sjson_serialize);
 >    defsubr (&Sjson_insert);
 >    defsubr (&Sjson_parse_string);
 > diff --git a/test/src/json-tests.el b/test/src/json-tests.el
 > index 07eb41d093..da51aac8c8 100644
 > --- a/test/src/json-tests.el
 > +++ b/test/src/json-tests.el
 > @@ -52,13 +52,15 @@
 >  
 >  (ert-deftest json-parse-string/object ()
 >    (skip-unless (fboundp 'json-parse-string))
 > -  (let ((actual
 > -         (json-parse-string
 > -          "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n")))
 > -    (should (hash-table-p actual))
 > -    (should (equal (hash-table-count actual) 2))
 > -    (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
 > -                   '(("abc" . [9 :false]) ("def" . :null))))))
 > +  (let ((input
 > +         "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n"))
 > +    (let ((actual (json-parse-string input)))
 > +      (should (hash-table-p actual))
 > +      (should (equal (hash-table-count actual) 2))
 > +      (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
 > +                     '(("abc" . [9 :false]) ("def" . :null)))))
 > +    (should (equal (json-parse-string input :object-type 'alist)
 > +                   '((abc . [9 :false]) (def . :null))))))
 >  
 >  (ert-deftest json-parse-string/string ()
 >    (skip-unless (fboundp 'json-parse-string))
 > -- 
 > 2.15.1
 > 

-- 

--

Re: JSON->lisp Mapping: Hash vs AList

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Wed, 13 Dec 2017 22:37:46 +0000
> Cc: emacs-devel@gnu.org
> 
> Sounds reasonable, here is a patch. 

Thanks.

> -@defun json-parse-string string
> +@defun json-parse-string string &key (object-type 'hash-table)

Hmm.. why is there an apostrophe before "hash-table"?  What do you
want to get in the output there?

And btw, I don't see "&key" mentioned anywhere in the ELisp manual, so
I wonder whether the reader will understand what it means.

> +          case json_object_alist:
> +            {
> +              result = Qnil;
> +              const char *key_str;
> +              json_t *value;
> +              json_object_foreach (json, key_str, value)
> +                {
> +                  Lisp_Object key = Fintern (json_build_string (key_str), Qnil);
> +                  result
> +                    = Fcons (Fcons (key, json_to_lisp (value, object_type)),
> +                             result);
> +                }
> +              result = Fnreverse (result);

Is there a reason for calling nreverse here?

> +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
                        ^^^^^^^^^^^
Shouldn't that be `:object-type' (including quotes)?

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1993 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Do., 14. Dez. 2017 um 17:28 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Wed, 13 Dec 2017 22:37:46 +0000
> > Cc: emacs-devel@gnu.org
> >
> > Sounds reasonable, here is a patch.
>
> Thanks.
>
> > -@defun json-parse-string string
> > +@defun json-parse-string string &key (object-type 'hash-table)
>
> Hmm.. why is there an apostrophe before "hash-table"?  What do you
> want to get in the output there?
>

An apostrophe? It seems to work as expected.


>
> And btw, I don't see "&key" mentioned anywhere in the ELisp manual, so
> I wonder whether the reader will understand what it means.
>

This is the Common Lisp syntax, from cl-defun etc. It's a bit unfortunate
that it's not used in Emacs core, even for functions that take keyword
arguments such as `make-process'. I can switch to '&rest args' if you
prefer that.


>
> > +          case json_object_alist:
> > +            {
> > +              result = Qnil;
> > +              const char *key_str;
> > +              json_t *value;
> > +              json_object_foreach (json, key_str, value)
> > +                {
> > +                  Lisp_Object key = Fintern (json_build_string
> (key_str), Qnil);
> > +                  result
> > +                    = Fcons (Fcons (key, json_to_lisp (value,
> object_type)),
> > +                             result);
> > +                }
> > +              result = Fnreverse (result);
>
> Is there a reason for calling nreverse here?
>

It puts the elements in the same order as the original JSON. (The Jansson
parser also retains the original order.)
This isn't very important, just a bit nicer and less surprising.


>
> > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
>                         ^^^^^^^^^^^
> Shouldn't that be `:object-type' (including quotes)?
>

Depending on whether we can use &key in a docstring in core. If so, then
this one is correct, see e.g. the docstring of should-error.

[-- Attachment #2: Type: text/html, Size: 3200 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sat, 16 Dec 2017 22:24:35 +0000
> Cc: raman@google.com, emacs-devel@gnu.org
> 
>  > -@defun json-parse-string string
>  > +@defun json-parse-string string &key (object-type 'hash-table)
> 
>  Hmm.. why is there an apostrophe before "hash-table"?  What do you
>  want to get in the output there?
> 
> An apostrophe? It seems to work as expected.

That's not what I meant.  I meant we never use a bare apostrophe in
Texinfo, we use markup instead.  So I asked what you want to get there
in the Info and printed output, so I could suggest a proper markup.

>  And btw, I don't see "&key" mentioned anywhere in the ELisp manual, so
>  I wonder whether the reader will understand what it means.
> 
> This is the Common Lisp syntax, from cl-defun etc. It's a bit unfortunate that it's not used in Emacs core, even
> for functions that take keyword arguments such as `make-process'. I can switch to '&rest args' if you prefer
> that.

Let's wait until the discussion of using &key in the code reaches its
conclusion.  If &key will stay in the source, I do prefer &rest in the
manual.

>  > +              result = Fnreverse (result);
> 
>  Is there a reason for calling nreverse here?
> 
> It puts the elements in the same order as the original JSON. (The Jansson parser also retains the original
> order.)
> This isn't very important, just a bit nicer and less surprising.

It's a potential performance hit, but if you think it's worthwhile,
it's fine with me.

>  > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
>                          ^^^^^^^^^^^
>  Shouldn't that be `:object-type' (including quotes)?
> 
> Depending on whether we can use &key in a docstring in core. If so, then this one is correct, see e.g. the
> docstring of should-error. 

IMO, the doc string of should-error is no less confusing than this
one, because it expects something like ":type 'foo".

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 2741 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am So., 17. Dez. 2017 um 16:53 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sat, 16 Dec 2017 22:24:35 +0000
> > Cc: raman@google.com, emacs-devel@gnu.org
> >
> >  > -@defun json-parse-string string
> >  > +@defun json-parse-string string &key (object-type 'hash-table)
> >
> >  Hmm.. why is there an apostrophe before "hash-table"?  What do you
> >  want to get in the output there?
> >
> > An apostrophe? It seems to work as expected.
>
> That's not what I meant.  I meant we never use a bare apostrophe in
> Texinfo, we use markup instead.  So I asked what you want to get there
> in the Info and printed output, so I could suggest a proper markup.
>

My goal was to specify the default value the same way that cl-lib does.
With cl-lib you'd write the function as

(cl-defun json-parse-string (string &key (object-type 'hash-table)))

We can't do that in C, but we can keep the same syntax.


>
> >  And btw, I don't see "&key" mentioned anywhere in the ELisp manual, so
> >  I wonder whether the reader will understand what it means.
> >
> > This is the Common Lisp syntax, from cl-defun etc. It's a bit
> unfortunate that it's not used in Emacs core, even
> > for functions that take keyword arguments such as `make-process'. I can
> switch to '&rest args' if you prefer
> > that.
>
> Let's wait until the discussion of using &key in the code reaches its
> conclusion.  If &key will stay in the source, I do prefer &rest in the
> manual.
>

I think the discussion will reach a conclusion if and when you or John as
maintainers make a decision :-)


>
> >  > +              result = Fnreverse (result);
> >
> >  Is there a reason for calling nreverse here?
> >
> > It puts the elements in the same order as the original JSON. (The
> Jansson parser also retains the original
> > order.)
> > This isn't very important, just a bit nicer and less surprising.
>
> It's a potential performance hit, but if you think it's worthwhile,
> it's fine with me.
>

I don't care much. For now I'd leave it in, we can take it out later if it
hurts performance too much. (Though people that care about performance
should probably use hashtables anyway.)


>
> >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
> >                          ^^^^^^^^^^^
> >  Shouldn't that be `:object-type' (including quotes)?
> >
> > Depending on whether we can use &key in a docstring in core. If so, then
> this one is correct, see e.g. the
> > docstring of should-error.
>
> IMO, the doc string of should-error is no less confusing than this
> one, because it expects something like ":type 'foo".
>

Arguably yes. Though that has been the convention for cl-lib functions for
a while.

[-- Attachment #2: Type: text/html, Size: 4107 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Sun, 17 Dec 2017 17:44:34 +0000
> Cc: raman@google.com, emacs-devel@gnu.org
> 
>  >  > -@defun json-parse-string string
>  >  > +@defun json-parse-string string &key (object-type 'hash-table)
>  >
>  >  Hmm.. why is there an apostrophe before "hash-table"?  What do you
>  >  want to get in the output there?
>  >
>  > An apostrophe? It seems to work as expected.
> 
>  That's not what I meant.  I meant we never use a bare apostrophe in
>  Texinfo, we use markup instead.  So I asked what you want to get there
>  in the Info and printed output, so I could suggest a proper markup.
> 
> My goal was to specify the default value the same way that cl-lib does. With cl-lib you'd write the function as 
> 
> (cl-defun json-parse-string (string &key (object-type 'hash-table)))
> 
> We can't do that in C, but we can keep the same syntax.

We are miscommunicating.  I'm talking about Texinfo markup, not about
Lisp or C code.  In Texinfo manuals dedicated to Emacs we use
@code{foo} where in Lisp you'd write 'foo.  Why not in this case?
IOW, why not use

  @defun json-parse-string string &key (object-type @code{hash-table})

?

>  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
>  >                          ^^^^^^^^^^^
>  >  Shouldn't that be `:object-type' (including quotes)?
>  >
>  > Depending on whether we can use &key in a docstring in core. If so, then this one is correct, see e.g.
>  the
>  > docstring of should-error.
> 
>  IMO, the doc string of should-error is no less confusing than this
>  one, because it expects something like ":type 'foo".
> 
> Arguably yes. Though that has been the convention for cl-lib functions for a while. 

cl-lib enjoyed being in the shadows for too long.  I don't think we
should let that continue any longer, we should fix that.

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 2187 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am So., 17. Dez. 2017 um 21:07 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Sun, 17 Dec 2017 17:44:34 +0000
> > Cc: raman@google.com, emacs-devel@gnu.org
> >
> >  >  > -@defun json-parse-string string
> >  >  > +@defun json-parse-string string &key (object-type 'hash-table)
> >  >
> >  >  Hmm.. why is there an apostrophe before "hash-table"?  What do you
> >  >  want to get in the output there?
> >  >
> >  > An apostrophe? It seems to work as expected.
> >
> >  That's not what I meant.  I meant we never use a bare apostrophe in
> >  Texinfo, we use markup instead.  So I asked what you want to get there
> >  in the Info and printed output, so I could suggest a proper markup.
> >
> > My goal was to specify the default value the same way that cl-lib does.
> With cl-lib you'd write the function as
> >
> > (cl-defun json-parse-string (string &key (object-type 'hash-table)))
> >
> > We can't do that in C, but we can keep the same syntax.
>
> We are miscommunicating.  I'm talking about Texinfo markup, not about
> Lisp or C code.  In Texinfo manuals dedicated to Emacs we use
> @code{foo} where in Lisp you'd write 'foo.  Why not in this case?
> IOW, why not use
>
>   @defun json-parse-string string &key (object-type @code{hash-table})
>
> ?
>

If that's the right thing, sure. OTOH, cl.texi uses
@defun cl-fill seq item @t{&key :start :end}
so probably we should use that?


>
> >  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is
> used to
> >  >                          ^^^^^^^^^^^
> >  >  Shouldn't that be `:object-type' (including quotes)?
> >  >
> >  > Depending on whether we can use &key in a docstring in core. If so,
> then this one is correct, see e.g.
> >  the
> >  > docstring of should-error.
> >
> >  IMO, the doc string of should-error is no less confusing than this
> >  one, because it expects something like ":type 'foo".
> >
> > Arguably yes. Though that has been the convention for cl-lib functions
> for a while.
>
> cl-lib enjoyed being in the shadows for too long.  I don't think we
> should let that continue any longer, we should fix that.
>

OK, what's your suggestion?

[-- Attachment #2: Type: text/html, Size: 3229 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 18 Dec 2017 19:55:52 +0000
> Cc: raman@google.com, emacs-devel@gnu.org
> 
>    @defun json-parse-string string &key (object-type @code{hash-table})
> 
>  ?
> 
> If that's the right thing, sure. OTOH, cl.texi uses
> @defun cl-fill seq item @t{&key :start :end}
> so probably we should use that?

No, @t is almost never right, certainly not in code sequences.

>  >  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is used to
>  >  >                          ^^^^^^^^^^^
>  >  >  Shouldn't that be `:object-type' (including quotes)?
>  >  >
>  >  > Depending on whether we can use &key in a docstring in core. If so, then this one is correct, see
>  e.g.
>  >  the
>  >  > docstring of should-error.
>  >
>  >  IMO, the doc string of should-error is no less confusing than this
>  >  one, because it expects something like ":type 'foo".
>  >
>  > Arguably yes. Though that has been the convention for cl-lib functions for a while.
> 
>  cl-lib enjoyed being in the shadows for too long.  I don't think we
>  should let that continue any longer, we should fix that.
> 
> OK, what's your suggestion? 

I thought I wrote that above.

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1.1: Type: text/plain, Size: 1443 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 18. Dez. 2017 um 21:19 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Mon, 18 Dec 2017 19:55:52 +0000
> > Cc: raman@google.com, emacs-devel@gnu.org
> >
> >    @defun json-parse-string string &key (object-type @code{hash-table})
> >
> >  ?
> >
> > If that's the right thing, sure. OTOH, cl.texi uses
> > @defun cl-fill seq item @t{&key :start :end}
> > so probably we should use that?
>
> No, @t is almost never right, certainly not in code sequences.
>

At least cl.texi uses it consistently, though.


>
> >  >  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is
> used to
> >  >  >                          ^^^^^^^^^^^
> >  >  >  Shouldn't that be `:object-type' (including quotes)?
> >  >  >
> >  >  > Depending on whether we can use &key in a docstring in core. If
> so, then this one is correct, see
> >  e.g.
> >  >  the
> >  >  > docstring of should-error.
> >  >
> >  >  IMO, the doc string of should-error is no less confusing than this
> >  >  one, because it expects something like ":type 'foo".
> >  >
> >  > Arguably yes. Though that has been the convention for cl-lib
> functions for a while.
> >
> >  cl-lib enjoyed being in the shadows for too long.  I don't think we
> >  should let that continue any longer, we should fix that.
> >
> > OK, what's your suggestion?
>
> I thought I wrote that above.
>

I've attached a new version of the patch.

[-- Attachment #1.2: Type: text/html, Size: 2366 bytes --]

[-- Attachment #2: 0001-Allow-JSON-parser-functions-to-return-alists.txt --]
[-- Type: text/plain, Size: 11909 bytes --]

From 37e352b628289fa61f9ef9d52ef6c4555c5525f7 Mon Sep 17 00:00:00 2001
From: Philipp Stephani <phst@google.com>
Date: Wed, 13 Dec 2017 23:35:07 +0100
Subject: [PATCH] Allow JSON parser functions to return alists

* src/json.c (Fjson_parse_string, Fjson_parse_buffer): Give these
functions a keyword argument to specify the return type for JSON
objects.
(json_to_lisp): Convert objects to alists if requested.
(json_parse_object_type): New helper function to parse keyword
arguments.

* test/src/json-tests.el (json-parse-string/object): Add a unit test.

* doc/lispref/text.texi (Parsing JSON): Document new functionality.
---
 doc/lispref/text.texi  |  20 +++++---
 src/json.c             | 129 +++++++++++++++++++++++++++++++++++++------------
 test/src/json-tests.el |  16 +++---
 3 files changed, 120 insertions(+), 45 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 5b288d9750..4fbd84a2f1 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -4965,14 +4965,13 @@ Parsing JSON
 
 @item
 JSON has only one map type, the object.  JSON objects are represented
-using Lisp hashtables.
+using Lisp hashtables or alists.
 
 @end itemize
 
 @noindent
-Note that @code{nil} doesn't represent any JSON values: this is to
-avoid confusion, because @code{nil} could either represent
-@code{null}, @code{false}, or an empty array, all of which are
+Note that @code{nil} represents the empty JSON object, @code{@{@}},
+not @code{null}, @code{false}, or an empty array, all of which are
 different JSON values.
 
   If some Lisp object can't be represented in JSON, the serialization
@@ -4995,8 +4994,13 @@ Parsing JSON
 
   Only top-level values (arrays and objects) can be serialized to
 JSON.  The subobjects within these top-level values can be of any
-type.  Likewise, the parsing functions will only return vectors and
-hashtables.
+type.  Likewise, the parsing functions will only return vectors,
+hashtables, and alists.
+
+  The parsing functions accept keyword arguments.  Currently only one
+keyword argument, @code{:object-type}, is recognized; its value can be
+either @code{hash-table} to parse JSON objects as hashtables with
+string keys (the default) or @code{alist} to parse them as alists.
 
 @defun json-serialize object
 This function returns a new Lisp string which contains the JSON
@@ -5008,12 +5012,12 @@ Parsing JSON
 current buffer before point.
 @end defun
 
-@defun json-parse-string string
+@defun json-parse-string string &key (object-type 'hash-table)
 This function parses the JSON value in @var{string}, which must be a
 Lisp string.
 @end defun
 
-@defun json-parse-buffer
+@defun json-parse-buffer &key (object-type @code{hash-table})
 This function reads the next JSON value from the current buffer,
 starting at point.  It moves point to the position immediately after
 the value if a value could be read and converted to Lisp; otherwise it
diff --git a/src/json.c b/src/json.c
index 72ca93f621..f9396b4e72 100644
--- a/src/json.c
+++ b/src/json.c
@@ -507,10 +507,15 @@ OBJECT.  */)
   return unbind_to (count, Qnil);
 }
 
+enum json_object_type {
+  json_object_hashtable,
+  json_object_alist,
+};
+
 /* Convert a JSON object to a Lisp object.  */
 
 static _GL_ARG_NONNULL ((1)) Lisp_Object
-json_to_lisp (json_t *json)
+json_to_lisp (json_t *json, enum json_object_type object_type)
 {
   switch (json_typeof (json))
     {
@@ -544,7 +549,7 @@ json_to_lisp (json_t *json)
         Lisp_Object result = Fmake_vector (make_natnum (size), Qunbound);
         for (ptrdiff_t i = 0; i < size; ++i)
           ASET (result, i,
-                json_to_lisp (json_array_get (json, i)));
+                json_to_lisp (json_array_get (json, i), object_type));
         --lisp_eval_depth;
         return result;
       }
@@ -552,23 +557,49 @@ json_to_lisp (json_t *json)
       {
         if (++lisp_eval_depth > max_lisp_eval_depth)
           xsignal0 (Qjson_object_too_deep);
-        size_t size = json_object_size (json);
-        if (FIXNUM_OVERFLOW_P (size))
-          xsignal0 (Qoverflow_error);
-        Lisp_Object result = CALLN (Fmake_hash_table, QCtest, Qequal,
-                                    QCsize, make_natnum (size));
-        struct Lisp_Hash_Table *h = XHASH_TABLE (result);
-        const char *key_str;
-        json_t *value;
-        json_object_foreach (json, key_str, value)
+        Lisp_Object result;
+        switch (object_type)
           {
-            Lisp_Object key = json_build_string (key_str);
-            EMACS_UINT hash;
-            ptrdiff_t i = hash_lookup (h, key, &hash);
-            /* Keys in JSON objects are unique, so the key can't be
-               present yet.  */
-            eassert (i < 0);
-            hash_put (h, key, json_to_lisp (value), hash);
+          case json_object_hashtable:
+            {
+              size_t size = json_object_size (json);
+              if (FIXNUM_OVERFLOW_P (size))
+                xsignal0 (Qoverflow_error);
+              result = CALLN (Fmake_hash_table, QCtest, Qequal, QCsize,
+                              make_natnum (size));
+              struct Lisp_Hash_Table *h = XHASH_TABLE (result);
+              const char *key_str;
+              json_t *value;
+              json_object_foreach (json, key_str, value)
+                {
+                  Lisp_Object key = json_build_string (key_str);
+                  EMACS_UINT hash;
+                  ptrdiff_t i = hash_lookup (h, key, &hash);
+                  /* Keys in JSON objects are unique, so the key can't
+                     be present yet.  */
+                  eassert (i < 0);
+                  hash_put (h, key, json_to_lisp (value, object_type), hash);
+                }
+              break;
+            }
+          case json_object_alist:
+            {
+              result = Qnil;
+              const char *key_str;
+              json_t *value;
+              json_object_foreach (json, key_str, value)
+                {
+                  Lisp_Object key = Fintern (json_build_string (key_str), Qnil);
+                  result
+                    = Fcons (Fcons (key, json_to_lisp (value, object_type)),
+                             result);
+                }
+              result = Fnreverse (result);
+              break;
+            }
+          default:
+            /* Can't get here.  */
+            emacs_abort ();
           }
         --lisp_eval_depth;
         return result;
@@ -578,15 +609,44 @@ json_to_lisp (json_t *json)
   emacs_abort ();
 }
 
-DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, 1, NULL,
+static enum json_object_type
+json_parse_object_type (ptrdiff_t nargs, Lisp_Object *args)
+{
+  switch (nargs)
+    {
+    case 0:
+      return json_object_hashtable;
+    case 2:
+      {
+        Lisp_Object key = args[0];
+        Lisp_Object value = args[1];
+        if (!EQ (key, QCobject_type))
+          wrong_choice (list1 (QCobject_type), key);
+        if (EQ (value, Qhash_table))
+          return json_object_hashtable;
+        else if (EQ (value, Qalist))
+          return json_object_alist;
+        else
+          wrong_choice (list2 (Qhash_table, Qalist), value);
+      }
+    default:
+      wrong_type_argument (Qplistp, Flist (nargs, args));
+    }
+}
+
+DEFUN ("json-parse-string", Fjson_parse_string, Sjson_parse_string, 1, MANY,
+       NULL,
        doc: /* Parse the JSON STRING into a Lisp object.
 This is essentially the reverse operation of `json-serialize', which
-see.  The returned object will be a vector or hashtable.  Its elements
-will be `:null', `:false', t, numbers, strings, or further vectors and
-hashtables.  If there are duplicate keys in an object, all but the
-last one are ignored.  If STRING doesn't contain a valid JSON object,
-an error of type `json-parse-error' is signaled.  */)
-  (Lisp_Object string)
+see.  The returned object will be a vector, hashtable, or alist.  Its
+elements will be `:null', `:false', t, numbers, strings, or further
+vectors, hashtables, and alists.  If there are duplicate keys in an
+object, all but the last one are ignored.  If STRING doesn't contain a
+valid JSON object, an error of type `json-parse-error' is signaled.
+The keyword argument `:object-type' specifies which Lisp type is used
+to represent objects; it can be `hash-table' or `alist'.
+usage: (string &key (OBJECT-TYPE \\='hash-table)) */)
+  (ptrdiff_t nargs, Lisp_Object *args)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -605,8 +665,11 @@ an error of type `json-parse-error' is signaled.  */)
     }
 #endif
 
+  Lisp_Object string = args[0];
   Lisp_Object encoded = json_encode (string);
   check_string_without_embedded_nulls (encoded);
+  enum json_object_type object_type
+    = json_parse_object_type (nargs - 1, args + 1);
 
   json_error_t error;
   json_t *object = json_loads (SSDATA (encoded), 0, &error);
@@ -617,7 +680,7 @@ an error of type `json-parse-error' is signaled.  */)
   if (object != NULL)
     record_unwind_protect_ptr (json_release_object, object);
 
-  return unbind_to (count, json_to_lisp (object));
+  return unbind_to (count, json_to_lisp (object, object_type));
 }
 
 struct json_read_buffer_data
@@ -650,12 +713,13 @@ json_read_buffer_callback (void *buffer, size_t buflen, void *data)
 }
 
 DEFUN ("json-parse-buffer", Fjson_parse_buffer, Sjson_parse_buffer,
-       0, 0, NULL,
+       0, MANY, NULL,
        doc: /* Read JSON object from current buffer starting at point.
 This is similar to `json-parse-string', which see.  Move point after
 the end of the object if parsing was successful.  On error, point is
-not moved.  */)
-  (void)
+not moved.
+usage: (&key (OBJECT-TYPE \\='hash-table))  */)
+  (ptrdiff_t nargs, Lisp_Object *args)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
 
@@ -674,6 +738,8 @@ not moved.  */)
     }
 #endif
 
+  enum json_object_type object_type = json_parse_object_type (nargs, args);
+
   ptrdiff_t point = PT_BYTE;
   struct json_read_buffer_data data = {.point = point};
   json_error_t error;
@@ -687,7 +753,7 @@ not moved.  */)
   record_unwind_protect_ptr (json_release_object, object);
 
   /* Convert and then move point only if everything succeeded.  */
-  Lisp_Object lisp = json_to_lisp (object);
+  Lisp_Object lisp = json_to_lisp (object, object_type);
 
   /* Adjust point by how much we just read.  */
   point += error.position;
@@ -750,6 +816,9 @@ syms_of_json (void)
   Fput (Qjson_parse_string, Qpure, Qt);
   Fput (Qjson_parse_string, Qside_effect_free, Qt);
 
+  DEFSYM (QCobject_type, ":object-type");
+  DEFSYM (Qalist, "alist");
+
   defsubr (&Sjson_serialize);
   defsubr (&Sjson_insert);
   defsubr (&Sjson_parse_string);
diff --git a/test/src/json-tests.el b/test/src/json-tests.el
index 07eb41d093..da51aac8c8 100644
--- a/test/src/json-tests.el
+++ b/test/src/json-tests.el
@@ -52,13 +52,15 @@
 
 (ert-deftest json-parse-string/object ()
   (skip-unless (fboundp 'json-parse-string))
-  (let ((actual
-         (json-parse-string
-          "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n")))
-    (should (hash-table-p actual))
-    (should (equal (hash-table-count actual) 2))
-    (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
-                   '(("abc" . [9 :false]) ("def" . :null))))))
+  (let ((input
+         "{ \"abc\" : [1, 2, true], \"def\" : null, \"abc\" : [9, false] }\n"))
+    (let ((actual (json-parse-string input)))
+      (should (hash-table-p actual))
+      (should (equal (hash-table-count actual) 2))
+      (should (equal (cl-sort (map-pairs actual) #'string< :key #'car)
+                     '(("abc" . [9 :false]) ("def" . :null)))))
+    (should (equal (json-parse-string input :object-type 'alist)
+                   '((abc . [9 :false]) (def . :null))))))
 
 (ert-deftest json-parse-string/string ()
   (skip-unless (fboundp 'json-parse-string))
-- 
2.15.1

Re: JSON->lisp Mapping: Hash vs AList

[Vibhav Pant]

On Tue, Dec 19, 2017 at 2:29 AM, Philipp Stephani <p.stephani2@gmail.com> wrote:
>
>
> Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 18. Dez. 2017 um 21:19 Uhr:
>>
>> >  >  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type is
>> > used to
>> >  >  >                          ^^^^^^^^^^^
>> >  >  >  Shouldn't that be `:object-type' (including quotes)?
>> >  >  >
>> >  >  > Depending on whether we can use &key in a docstring in core. If
>> > so, then this one is correct, see
>> >  e.g.
>> >  >  the
>> >  >  > docstring of should-error.
>> >  >
>> >  >  IMO, the doc string of should-error is no less confusing than this
>> >  >  one, because it expects something like ":type 'foo".
>> >  >
>> >  > Arguably yes. Though that has been the convention for cl-lib
>> > functions for a while.
>> >
>> >  cl-lib enjoyed being in the shadows for too long.  I don't think we
>> >  should let that continue any longer, we should fix that.
>> >
>> > OK, what's your suggestion?
>>
>> I thought I wrote that above.
>
>
> I've attached a new version of the patch.

fns.c has `get_key_arg', which accomplishes what json_parse_object_type is doing
here, and allows the list of keyword arguments to be extended (like in
`make-hash-table`). Do you think that could be used here instead?

Thanks,
Vibhav

-- 
Vibhav Pant
vibhavp@gmail.com

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1582 bytes --]

Vibhav Pant <vibhavp@gmail.com> schrieb am Di., 19. Dez. 2017 um 04:51 Uhr:

> On Tue, Dec 19, 2017 at 2:29 AM, Philipp Stephani <p.stephani2@gmail.com>
> wrote:
> >
> >
> > Eli Zaretskii <eliz@gnu.org> schrieb am Mo., 18. Dez. 2017 um 21:19 Uhr:
> >>
> >> >  >  >  > +The keyword argument OBJECT-TYPE specifies which Lisp type
> is
> >> > used to
> >> >  >  >                          ^^^^^^^^^^^
> >> >  >  >  Shouldn't that be `:object-type' (including quotes)?
> >> >  >  >
> >> >  >  > Depending on whether we can use &key in a docstring in core. If
> >> > so, then this one is correct, see
> >> >  e.g.
> >> >  >  the
> >> >  >  > docstring of should-error.
> >> >  >
> >> >  >  IMO, the doc string of should-error is no less confusing than this
> >> >  >  one, because it expects something like ":type 'foo".
> >> >  >
> >> >  > Arguably yes. Though that has been the convention for cl-lib
> >> > functions for a while.
> >> >
> >> >  cl-lib enjoyed being in the shadows for too long.  I don't think we
> >> >  should let that continue any longer, we should fix that.
> >> >
> >> > OK, what's your suggestion?
> >>
> >> I thought I wrote that above.
> >
> >
> > I've attached a new version of the patch.
>
> fns.c has `get_key_arg', which accomplishes what json_parse_object_type is
> doing
> here, and allows the list of keyword arguments to be extended (like in
> `make-hash-table`). Do you think that could be used here instead?
>
>
Yes, it would be usable. Probably it should be made 'extern', and
Fmake_progress should also use it.
It's not a terribly big deal, though.

[-- Attachment #2: Type: text/html, Size: 2460 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Eli Zaretskii]

> From: Philipp Stephani <p.stephani2@gmail.com>
> Date: Mon, 18 Dec 2017 20:59:39 +0000
> Cc: raman@google.com, emacs-devel@gnu.org
> 
> -@defun json-parse-string string
> +@defun json-parse-string string &key (object-type 'hash-table)
>  This function parses the JSON value in @var{string}, which must be a
>  Lisp string.
>  @end defun
>  
> -@defun json-parse-buffer
> +@defun json-parse-buffer &key (object-type @code{hash-table})

The first @defun needs the same change as you did in the second.

Otherwise, LGTM; thanks.

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 672 bytes --]

Eli Zaretskii <eliz@gnu.org> schrieb am Di., 19. Dez. 2017 um 18:08 Uhr:

> > From: Philipp Stephani <p.stephani2@gmail.com>
> > Date: Mon, 18 Dec 2017 20:59:39 +0000
> > Cc: raman@google.com, emacs-devel@gnu.org
> >
> > -@defun json-parse-string string
> > +@defun json-parse-string string &key (object-type 'hash-table)
> >  This function parses the JSON value in @var{string}, which must be a
> >  Lisp string.
> >  @end defun
> >
> > -@defun json-parse-buffer
> > +@defun json-parse-buffer &key (object-type @code{hash-table})
>
> The first @defun needs the same change as you did in the second.
>
> Otherwise, LGTM; thanks.
>

Thanks, pushed to master as db4f12e93f.

[-- Attachment #2: Type: text/html, Size: 1224 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Clément Pit-Claudel]

On 2017-12-17 12:44, Philipp Stephani wrote:
>     >  > +              result = Fnreverse (result);
>     >
>     >  Is there a reason for calling nreverse here?
>     >
>     > It puts the elements in the same order as the original JSON. (The Jansson parser also retains the original
>     > order.)
>     > This isn't very important, just a bit nicer and less surprising.
> 
>     It's a potential performance hit, but if you think it's worthwhile,
>     it's fine with me.
> 
> I don't care much. For now I'd leave it in, we can take it out later if it hurts performance too much. (Though people that care about performance should probably use hashtables anyway.)

Would it make it faster to construct the list in order, instead of constructing it in reverse and then reversing it?

Something like the following, but in C:

;; the direct version
(defun map-fn-over-list (fn list)
  (when list
    (let* ((out (cons (funcall fn (pop list)) nil))
           (lastcdr out))
      (while list
        (setcdr lastcdr (cons (funcall fn (pop list)) nil))
        (setq lastcdr (cdr lastcdr)))
      out)))
 Clément.

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

Clément Pit-Claudel <cpitclaudel@gmail.com> schrieb am Mo., 18. Dez. 2017
um 17:15 Uhr:

> On 2017-12-17 12:44, Philipp Stephani wrote:
> >     >  > +              result = Fnreverse (result);
> >     >
> >     >  Is there a reason for calling nreverse here?
> >     >
> >     > It puts the elements in the same order as the original JSON. (The
> Jansson parser also retains the original
> >     > order.)
> >     > This isn't very important, just a bit nicer and less surprising.
> >
> >     It's a potential performance hit, but if you think it's worthwhile,
> >     it's fine with me.
> >
> > I don't care much. For now I'd leave it in, we can take it out later if
> it hurts performance too much. (Though people that care about performance
> should probably use hashtables anyway.)
>
> Would it make it faster to construct the list in order, instead of
> constructing it in reverse and then reversing it?
>

I haven't measured it. Feel free to measure whether one of the three
methods (the current one, yours, inverted order) is significantly faster
than the others, and send a patch if yes.

[-- Attachment #2: Type: text/html, Size: 1531 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Vibhav Pant]

On Thu, Dec 14, 2017 at 4:07 AM, Philipp Stephani <p.stephani2@gmail.com> wrote:
>
> Sounds reasonable, here is a patch.

Can json-serialize be extended in a similar fashion?

-- 
Vibhav Pant
vibhavp@gmail.com

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 404 bytes --]

Vibhav Pant <vibhavp@gmail.com> schrieb am Fr., 15. Dez. 2017 um 05:14 Uhr:

> On Thu, Dec 14, 2017 at 4:07 AM, Philipp Stephani <p.stephani2@gmail.com>
> wrote:
> >
> > Sounds reasonable, here is a patch.
>
> Can json-serialize be extended in a similar fashion?
>
>
>
Yes, I have another patch ready for that. I'll wait for the first one to
get committed, though, because otherwise they would conflict.

[-- Attachment #2: Type: text/html, Size: 768 bytes --]

[PATCH] Accept alists when serializing JSON

[Philipp Stephani]

* src/json.c (lisp_to_json_toplevel_1): Also accept alists
representing objects.

* src/json.c (Fjson_serialize): Update docstring.

* test/src/json-tests.el (json-serialize/object): Add unit tests for
serializing alists.

* doc/lispref/text.texi (Parsing JSON): Document that serialization
functions accept alists.
---
 doc/lispref/text.texi  | 10 +++++----
 src/json.c             | 57 +++++++++++++++++++++++++++++++++++++++++---------
 test/src/json-tests.el | 14 ++++++++++++-
 3 files changed, 66 insertions(+), 15 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 7517dbcfb6..fbc4352fec 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -4965,14 +4965,16 @@ Parsing JSON
 
 @item
 JSON has only one map type, the object.  JSON objects are represented
-using Lisp hashtables or alists.
+using Lisp hashtables or alists.  When an alist contains several
+elements with the same key, Emacs uses only the first element for
+serialization, in accordance with the behavior of @code{assq}.
 
 @end itemize
 
 @noindent
-Note that @code{nil} represents the empty JSON object, @code{@{@}},
-not @code{null}, @code{false}, or an empty array, all of which are
-different JSON values.
+Note that @code{nil} is a valid alist and represents the empty JSON
+object, @code{@{@}}, not @code{null}, @code{false}, or an empty array,
+all of which are different JSON values.
 
   If some Lisp object can't be represented in JSON, the serialization
 functions will signal an error of type @code{wrong-type-argument}.
diff --git a/src/json.c b/src/json.c
index 6b8387ff2f..747b402f96 100644
--- a/src/json.c
+++ b/src/json.c
@@ -335,12 +335,48 @@ lisp_to_json_toplevel_1 (Lisp_Object lisp, json_t **json)
       clear_unwind_protect (count);
       return unbind_to (count, Qnil);
     }
+  else if (NILP (lisp))
+    {
+      *json = json_check (json_object ());
+      return Qnil;
+    }
+  else if (CONSP (lisp))
+    {
+      Lisp_Object tail = lisp;
+      *json = json_check (json_object ());
+      ptrdiff_t count = SPECPDL_INDEX ();
+      record_unwind_protect_ptr (json_release_object, *json);
+      FOR_EACH_TAIL (tail)
+        {
+          Lisp_Object pair = XCAR (tail);
+          CHECK_CONS (pair);
+          Lisp_Object key_symbol = XCAR (pair);
+          Lisp_Object value = XCDR (pair);
+          CHECK_SYMBOL (key_symbol);
+          Lisp_Object key = SYMBOL_NAME (key_symbol);
+          /* We can't specify the length, so the string must be
+             null-terminated.  */
+          check_string_without_embedded_nulls (key);
+          const char *key_data = SSDATA (key);
+          /* Only add element if key is not already present.  */
+          if (json_object_get (*json, key_data) == NULL)
+            {
+              int status
+                = json_object_set_new (*json, key_data, lisp_to_json (value));
+              if (status == -1)
+                json_out_of_memory ();
+            }
+        }
+      CHECK_LIST_END (tail, lisp);
+      clear_unwind_protect (count);
+      return unbind_to (count, Qnil);
+    }
   wrong_type_argument (Qjson_value_p, lisp);
 }
 
 /* Convert LISP to a toplevel JSON object (array or object).  Signal
-   an error of type `wrong-type-argument' if LISP is not a vector or
-   hashtable.  */
+   an error of type `wrong-type-argument' if LISP is not a vector,
+   hashtable, or alist.  */
 
 static json_t *
 lisp_to_json_toplevel (Lisp_Object lisp)
@@ -380,19 +416,20 @@ lisp_to_json (Lisp_Object lisp)
       return json_check (json_stringn (SSDATA (encoded), size));
     }
 
-  /* LISP now must be a vector or hashtable.  */
+  /* LISP now must be a vector, hashtable, or alist.  */
   return lisp_to_json_toplevel (lisp);
 }
 
 DEFUN ("json-serialize", Fjson_serialize, Sjson_serialize, 1, 1, NULL,
        doc: /* Return the JSON representation of OBJECT as a string.
-OBJECT must be a vector or hashtable, and its elements can recursively
-contain `:null', `:false', t, numbers, strings, or other vectors and
-hashtables.  `:null', `:false', and t will be converted to JSON null,
-false, and true values, respectively.  Vectors will be converted to
-JSON arrays, and hashtables to JSON objects.  Hashtable keys must be
-strings without embedded null characters and must be unique within
-each object.  */)
+OBJECT must be a vector, hashtable, or alist, and its elements can
+recursively contain `:null', `:false', t, numbers, strings, or other
+vectors and hashtables.  `:null', `:false', and t will be converted to
+JSON null, false, and true values, respectively.  Vectors will be
+converted to JSON arrays, and hashtables to JSON objects.  Hashtable
+keys must be strings without embedded null characters and must be
+unique within each object.  Alist keys must be symbols; if a key is
+duplicate, the first instance is used.  */)
   (Lisp_Object object)
 {
   ptrdiff_t count = SPECPDL_INDEX ();
diff --git a/test/src/json-tests.el b/test/src/json-tests.el
index da51aac8c8..e857753cdc 100644
--- a/test/src/json-tests.el
+++ b/test/src/json-tests.el
@@ -48,7 +48,19 @@
     (puthash "abc" [1 2 t] table)
     (puthash "def" :null table)
     (should (equal (json-serialize table)
-                   "{\"abc\":[1,2,true],\"def\":null}"))))
+                   "{\"abc\":[1,2,true],\"def\":null}")))
+  (should (equal (json-serialize '((abc . [1 2 t]) (def . :null)))
+                 "{\"abc\":[1,2,true],\"def\":null}"))
+  (should (equal (json-serialize nil) "{}"))
+  (should (equal (json-serialize '((abc))) "{\"abc\":{}}"))
+  (should (equal (json-serialize '((a . 1) (b . 2) (a . 3)))
+                 "{\"a\":1,\"b\":2}"))
+  (should-error (json-serialize '(abc)) :type 'wrong-type-argument)
+  (should-error (json-serialize '((a 1))) :type 'wrong-type-argument)
+  (should-error (json-serialize '((1 . 2))) :type 'wrong-type-argument)
+  (should-error (json-serialize '((a . 1) . b)) :type 'wrong-type-argument)
+  (should-error (json-serialize '#1=((a . 1) . #1#)) :type 'circular-list)
+  (should-error (json-serialize '(#1=(a #1#)))))
 
 (ert-deftest json-parse-string/object ()
   (skip-unless (fboundp 'json-parse-string))
-- 
2.15.1

Re: [PATCH] Accept alists when serializing JSON

[Vibhav Pant]

On Tue, Dec 19, 2017 at 1:56 AM, Philipp Stephani <p.stephani2@gmail.com> wrote:
> * src/json.c (lisp_to_json_toplevel_1): Also accept alists
> representing objects.
>
> * src/json.c (Fjson_serialize): Update docstring.
>
> * test/src/json-tests.el (json-serialize/object): Add unit tests for
> serializing alists.
>
> * doc/lispref/text.texi (Parsing JSON): Document that serialization
> functions accept alists.

LGTM, do you think that we can add plist support in the future?

Thanks,
Vibhav

-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] Accept alists when serializing JSON

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

Vibhav Pant <vibhavp@gmail.com> schrieb am Mi., 20. Dez. 2017 um 06:58 Uhr:

>
> do you think that we can add plist support in the future?
>
> In theory yes, but I don't think it's worth the additional complexity.
Plists are not a general-purpose data structure; they are mostly used in a
specialized way, e.g. for keyword arguments or symbol properties.
Supporting alists make sense, they are easy to work with due to let-alist
and other alist-specific functionality. But that's not the case for plists.
Or do you have a specific use case where plist support would provide a
significant advantage?

[-- Attachment #2: Type: text/html, Size: 880 bytes --]

Re: [PATCH] Accept alists when serializing JSON

[Vibhav Pant]

My motivation behind plist support was to achieve feature parity with json.el,
so that `json-serialize` could be used as a drop in replacement in the future.
I also had a patch to add support for functionality similar to what json.el
variables `json-false` and `json-null` apply, using keyword arguments to
`json-serialize` and `json-parse-*` (which is why I submitted the get_key_arg
patch earlier).

On Fri, Dec 22, 2017 at 7:25 PM, Philipp Stephani <p.stephani2@gmail.com> wrote:
>
>
> Vibhav Pant <vibhavp@gmail.com> schrieb am Mi., 20. Dez. 2017 um 06:58 Uhr:
>>
>>
>> do you think that we can add plist support in the future?
>>
> In theory yes, but I don't think it's worth the additional complexity.
> Plists are not a general-purpose data structure; they are mostly used in a
> specialized way, e.g. for keyword arguments or symbol properties. Supporting
> alists make sense, they are easy to work with due to let-alist and other
> alist-specific functionality. But that's not the case for plists.
> Or do you have a specific use case where plist support would provide a
> significant advantage?



-- 
Vibhav Pant
vibhavp@gmail.com

Re: [PATCH] Accept alists when serializing JSON

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 870 bytes --]

Vibhav Pant <vibhavp@gmail.com> schrieb am So., 24. Dez. 2017 um 18:16 Uhr:

> My motivation behind plist support was to achieve feature parity with
> json.el,

so that `json-serialize` could be used as a drop in replacement in the
> future.
>

I don't think that should be a goal. Interfaces should be kept minimal and
simple. The json.el interface already does "too much", even allowing to
generate invalid JSON (e.g. by setting json-encoding-separator to something
meaningless). We should only implement features if there's a clear,
demonstratable benefit, not just because some other library has them.
Libraries parsing or generating JSON do so to conform to some API and need
to process the respective Lisp objects anyway to adapt them to whatever
they want to do with the data. It's OK to make them use :false etc.
explicitly without possibility of customization.

[-- Attachment #2: Type: text/html, Size: 1284 bytes --]

Re: [PATCH] Accept alists when serializing JSON

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 6619 bytes --]

Pushed to master as f552a957ada23a7ff182fc1ab94221ced3ed1713.

Philipp Stephani <p.stephani2@gmail.com> schrieb am Mo., 18. Dez. 2017 um
21:26 Uhr:

> * src/json.c (lisp_to_json_toplevel_1): Also accept alists
> representing objects.
>
> * src/json.c (Fjson_serialize): Update docstring.
>
> * test/src/json-tests.el (json-serialize/object): Add unit tests for
> serializing alists.
>
> * doc/lispref/text.texi (Parsing JSON): Document that serialization
> functions accept alists.
> ---
>  doc/lispref/text.texi  | 10 +++++----
>  src/json.c             | 57
> +++++++++++++++++++++++++++++++++++++++++---------
>  test/src/json-tests.el | 14 ++++++++++++-
>  3 files changed, 66 insertions(+), 15 deletions(-)
>
> diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
> index 7517dbcfb6..fbc4352fec 100644
> --- a/doc/lispref/text.texi
> +++ b/doc/lispref/text.texi
> @@ -4965,14 +4965,16 @@ Parsing JSON
>
>  @item
>  JSON has only one map type, the object.  JSON objects are represented
> -using Lisp hashtables or alists.
> +using Lisp hashtables or alists.  When an alist contains several
> +elements with the same key, Emacs uses only the first element for
> +serialization, in accordance with the behavior of @code{assq}.
>
>  @end itemize
>
>  @noindent
> -Note that @code{nil} represents the empty JSON object, @code{@{@}},
> -not @code{null}, @code{false}, or an empty array, all of which are
> -different JSON values.
> +Note that @code{nil} is a valid alist and represents the empty JSON
> +object, @code{@{@}}, not @code{null}, @code{false}, or an empty array,
> +all of which are different JSON values.
>
>    If some Lisp object can't be represented in JSON, the serialization
>  functions will signal an error of type @code{wrong-type-argument}.
> diff --git a/src/json.c b/src/json.c
> index 6b8387ff2f..747b402f96 100644
> --- a/src/json.c
> +++ b/src/json.c
> @@ -335,12 +335,48 @@ lisp_to_json_toplevel_1 (Lisp_Object lisp, json_t
> **json)
>        clear_unwind_protect (count);
>        return unbind_to (count, Qnil);
>      }
> +  else if (NILP (lisp))
> +    {
> +      *json = json_check (json_object ());
> +      return Qnil;
> +    }
> +  else if (CONSP (lisp))
> +    {
> +      Lisp_Object tail = lisp;
> +      *json = json_check (json_object ());
> +      ptrdiff_t count = SPECPDL_INDEX ();
> +      record_unwind_protect_ptr (json_release_object, *json);
> +      FOR_EACH_TAIL (tail)
> +        {
> +          Lisp_Object pair = XCAR (tail);
> +          CHECK_CONS (pair);
> +          Lisp_Object key_symbol = XCAR (pair);
> +          Lisp_Object value = XCDR (pair);
> +          CHECK_SYMBOL (key_symbol);
> +          Lisp_Object key = SYMBOL_NAME (key_symbol);
> +          /* We can't specify the length, so the string must be
> +             null-terminated.  */
> +          check_string_without_embedded_nulls (key);
> +          const char *key_data = SSDATA (key);
> +          /* Only add element if key is not already present.  */
> +          if (json_object_get (*json, key_data) == NULL)
> +            {
> +              int status
> +                = json_object_set_new (*json, key_data, lisp_to_json
> (value));
> +              if (status == -1)
> +                json_out_of_memory ();
> +            }
> +        }
> +      CHECK_LIST_END (tail, lisp);
> +      clear_unwind_protect (count);
> +      return unbind_to (count, Qnil);
> +    }
>    wrong_type_argument (Qjson_value_p, lisp);
>  }
>
>  /* Convert LISP to a toplevel JSON object (array or object).  Signal
> -   an error of type `wrong-type-argument' if LISP is not a vector or
> -   hashtable.  */
> +   an error of type `wrong-type-argument' if LISP is not a vector,
> +   hashtable, or alist.  */
>
>  static json_t *
>  lisp_to_json_toplevel (Lisp_Object lisp)
> @@ -380,19 +416,20 @@ lisp_to_json (Lisp_Object lisp)
>        return json_check (json_stringn (SSDATA (encoded), size));
>      }
>
> -  /* LISP now must be a vector or hashtable.  */
> +  /* LISP now must be a vector, hashtable, or alist.  */
>    return lisp_to_json_toplevel (lisp);
>  }
>
>  DEFUN ("json-serialize", Fjson_serialize, Sjson_serialize, 1, 1, NULL,
>         doc: /* Return the JSON representation of OBJECT as a string.
> -OBJECT must be a vector or hashtable, and its elements can recursively
> -contain `:null', `:false', t, numbers, strings, or other vectors and
> -hashtables.  `:null', `:false', and t will be converted to JSON null,
> -false, and true values, respectively.  Vectors will be converted to
> -JSON arrays, and hashtables to JSON objects.  Hashtable keys must be
> -strings without embedded null characters and must be unique within
> -each object.  */)
> +OBJECT must be a vector, hashtable, or alist, and its elements can
> +recursively contain `:null', `:false', t, numbers, strings, or other
> +vectors and hashtables.  `:null', `:false', and t will be converted to
> +JSON null, false, and true values, respectively.  Vectors will be
> +converted to JSON arrays, and hashtables to JSON objects.  Hashtable
> +keys must be strings without embedded null characters and must be
> +unique within each object.  Alist keys must be symbols; if a key is
> +duplicate, the first instance is used.  */)
>    (Lisp_Object object)
>  {
>    ptrdiff_t count = SPECPDL_INDEX ();
> diff --git a/test/src/json-tests.el b/test/src/json-tests.el
> index da51aac8c8..e857753cdc 100644
> --- a/test/src/json-tests.el
> +++ b/test/src/json-tests.el
> @@ -48,7 +48,19 @@
>      (puthash "abc" [1 2 t] table)
>      (puthash "def" :null table)
>      (should (equal (json-serialize table)
> -                   "{\"abc\":[1,2,true],\"def\":null}"))))
> +                   "{\"abc\":[1,2,true],\"def\":null}")))
> +  (should (equal (json-serialize '((abc . [1 2 t]) (def . :null)))
> +                 "{\"abc\":[1,2,true],\"def\":null}"))
> +  (should (equal (json-serialize nil) "{}"))
> +  (should (equal (json-serialize '((abc))) "{\"abc\":{}}"))
> +  (should (equal (json-serialize '((a . 1) (b . 2) (a . 3)))
> +                 "{\"a\":1,\"b\":2}"))
> +  (should-error (json-serialize '(abc)) :type 'wrong-type-argument)
> +  (should-error (json-serialize '((a 1))) :type 'wrong-type-argument)
> +  (should-error (json-serialize '((1 . 2))) :type 'wrong-type-argument)
> +  (should-error (json-serialize '((a . 1) . b)) :type
> 'wrong-type-argument)
> +  (should-error (json-serialize '#1=((a . 1) . #1#)) :type 'circular-list)
> +  (should-error (json-serialize '(#1=(a #1#)))))
>
>  (ert-deftest json-parse-string/object ()
>    (skip-unless (fboundp 'json-parse-string))
> --
> 2.15.1
>
>

[-- Attachment #2: Type: text/html, Size: 7915 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Stefan Monnier]

> -@defun json-parse-string string
> +@defun json-parse-string string &key (object-type 'hash-table)

Emacs generally avoids keyword arguments.

I consider them acceptable only for macros (where the cost of parsing
arguments is acceptable since it's done at compile time) or for
function which both need many optional arguments (so positional
arguments are unwieldy) and whose runtime is significant enough that
parsing keyword arguments is a non-issue (e.g. make-process).

Given that there's only one optional argument here, I'd rather have it
be a &optional one.


        Stefan

Re: JSON->lisp Mapping: Hash vs AList

[Philipp Stephani]

[-- Attachment #1: Type: text/plain, Size: 895 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> schrieb am Sa., 16. Dez. 2017 um
23:36 Uhr:

> > -@defun json-parse-string string
> > +@defun json-parse-string string &key (object-type 'hash-table)
>
> Emacs generally avoids keyword arguments.
>
> I consider them acceptable only for macros (where the cost of parsing
> arguments is acceptable since it's done at compile time) or for
> function which both need many optional arguments (so positional
> arguments are unwieldy) and whose runtime is significant enough that
> parsing keyword arguments is a non-issue (e.g. make-process).
>
> Given that there's only one optional argument here, I'd rather have it
> be a &optional one.
>
>
I don't like optional arguments. They make the call sites less readable,
because it becomes less clear what the meaning of an argument is.
I don't care about saving a few nanoseconds; that's premature optimization.

[-- Attachment #2: Type: text/html, Size: 1271 bytes --]

Re: JSON->lisp Mapping: Hash vs AList

[Paul Eggert]

Philipp Stephani wrote:
> I don't like optional arguments. They make the call sites less readable,
> because it becomes less clear what the meaning of an argument is.

That depends on the case, and on the reader. Requiring keywords can make callers 
less readable due to clutter, and arguably that is the case here.

Re: JSON->lisp Mapping: Hash vs AList

[Stefan Monnier]

> I don't like optional arguments.  They make the call sites less readable,
> because it becomes less clear what the meaning of an argument is.
> I don't care about saving a few nanoseconds; that's premature optimization.

I was talking about the general design of Elisp, and in that context
I do not think it can qualify as premature optimization.
But in any case, it's a bit late to change that design, I think.


        Stefan