unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: "João Távora" <joaotavora@gmail.com>
Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
Subject: Re: [PATCH] Support custom 'null' and 'false' objects when parsing JSON
Date: Sat, 09 Jun 2018 11:18:29 +0300	[thread overview]
Message-ID: <838t7otlt6.fsf@gnu.org> (raw)
In-Reply-To: <87a7s6hx0c.fsf@gmail.com> (message from João Távora on Fri, 08 Jun 2018 02:45:07 +0100)

> From: João Távora <joaotavora@gmail.com>
> Date: Fri, 08 Jun 2018 02:45:07 +0100
> Cc: emacs-devel@gnu.org

Some comments on the documentation parts:

> +@item @code{:null-object}
> +The value decides which Lisp object use to represent the JSON keyword
                                       ^^^
"to use"

> +@code{null}.  It defaults to the lisp symbol @code{:null}.
> +
> +@item @code{:false-object}
> +The value decides which Lisp object use to represent the JSON keyword
                                       ^^^
Likewise.

> +@code{false}.  It defaults to the lisp symbol @code{:false}.
>  @end defun
>  
> -@defun json-insert object
> +@end itemize

Btw, I think @table..@end table will produce a better-looking output
here.

> +Lisp string.  The arguments @var{args} are a list of keyword/argument
                                          ^^^^^^^^^^
"is a list"

> +pairs.  The following keywords are accepted:
> +
> +@itemize
> +
> +@item @code{:object-type}

Doesn't this duplicate the same list above?  If so, it would be best
to have only one copy of that.  (And you have the same issues as above
with omitting "to" in "to use".)

> +The value decides which Lisp object to use for representing the key-value
> +mappings of a JSON object.  It can be either @code{hash-table}, the
> +default, to make hashtables with strings as keys, @code{alist} to use
> +alists with symbols as keys or @code{plist} to use plists with keyword
> +symbols as keys.

the punctuation of the last sentence could be slightly confusing.  I
suggest to use the following pattern:

  It can be either A, the default, ... ; B ...; or C.

IOW, use a semi-colon between alternatives, which will then allow you
to use commas freely when describing each alternative.

> -DEFUN ("json-serialize", Fjson_serialize, Sjson_serialize, 1, 1, NULL,
> +DEFUN ("json-serialize", Fjson_serialize, Sjson_serialize, 1, MANY,
> +       NULL,
>         doc: /* Return the JSON representation of OBJECT as a string.
> +
>  OBJECT must be a vector, hashtable, alist, or plist and its elements
> -can recursively contain `:null', `:false', t, numbers, strings, or
> -other vectors hashtables, alists or plists.  `:null', `:false', and t
> -will be converted to JSON null, false, and true values, respectively.
> -Vectors will be converted to JSON arrays, whereas hashtables, alists
> -and plists are converted to JSON objects.  Hashtable keys must be
> -strings without embedded null characters and must be unique within
> -each object.  Alist and plist keys must be symbols; if a key is
> -duplicate, the first instance is used.  */)
> -  (Lisp_Object object)
> +can recursively contain the lisp equivalents to the JSON null and
                               ^^^^
"Lisp"

> +The lisp equivalents to the JSON null and false values are
       ^^^^
Likewise.

> +Note that ambiguity can arise if you specify the same value for
> +`:null-object' and `:false-object', and so this function's behaviour
> +is unspecified

Missing period at end of sentence.

Thanks.



  reply	other threads:[~2018-06-09  8:18 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-07 18:29 [PATCH] Support custom 'null' and 'false' objects when parsing JSON João Távora
2018-06-07 19:13 ` Stefan Monnier
2018-06-07 23:08   ` João Távora
2018-06-08  1:45     ` João Távora
2018-06-09  8:18       ` Eli Zaretskii [this message]
2018-06-09 14:49         ` João Távora
2018-06-09 15:18           ` Eli Zaretskii
2018-06-14 23:23       ` João Távora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=838t7otlt6.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=joaotavora@gmail.com \
    --cc=monnier@iro.umontreal.ca \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).