Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Dmitry Gutov
Hi Eli,

On 13.04.2019 10:09, Eli Zaretskii wrote:

>          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.
> +Move point after the end of the object if parsing was successful.
> +On error, don't move point.
> +
> +The returned object will be a vector, list, hashtable, alist, or
> +plist.  Its elements will be the JSON null value, the JSON false
> +value, t, numbers, strings, or further vectors, lists, hashtables,
> +alists, or plists.  If there are duplicate keys in an object, all
> +but the last one are ignored.
> +
> +If the current buffer doesn't contain a valid JSON object, the
> +function signals an error of type `json-parse-error'.
> +
> +The arguments ARGS are a list of keyword/argument pairs:
> +
> +The keyword argument `:object-type' specifies which Lisp type is used
> +to represent objects; it can be `hash-table', `alist' or `plist'.  It
> +defaults to `hash-table'.
> +
> +The keyword argument `:array-type' specifies which Lisp type is used
> +to represent arrays; it can be `array' (the default) or `list'.
> +
> +The keyword argument `:null-object' specifies which object to use
> +to represent a JSON null value.  It defaults to `:null'.
> +
> +The keyword argument `:false-object' specifies which object to use to
> +represent a JSON false value.  It defaults to `:false'.

FTR, I quite dislike this kind of duplication in docstrings.

Didn't we discuss the difference between docstrings and manuals some
time ago, where you expressed the opinion that docstrings are allowed
the kind of "see XX for more detail" references, and it's the manuals
where information sometimes has to be reiterated for the convenience of
the reader? Here you seem to have made the reverse choice.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Eli Zaretskii
> From: Dmitry Gutov <[hidden email]>
> Date: Sun, 14 Apr 2019 13:34:51 +0300
>
> FTR, I quite dislike this kind of duplication in docstrings.

Yes, you've said that in the past, and I think we agreed to disagree
on it.  My opinion is that it's a judgment call: sometimes duplication
is for the better, and sometimes for the worse.  In this case, the
original doc string of json-parse-buffer said almost nothing useful,
leaving almost everything to look up in another function's
documentation, which IME is annoying.  Referring to another doc string
is OK for some secondary details, or perhaps for some very complicated
issues.  Not for the main part of the doc.  Especially for a function
whose name doesn't necessarily speak volumes about its purpose.

> Didn't we discuss the difference between docstrings and manuals some
> time ago, where you expressed the opinion that docstrings are allowed
> the kind of "see XX for more detail" references, and it's the manuals
> where information sometimes has to be reiterated for the convenience of
> the reader?

Doesn't sound like something I'd say, not to that effect.  "Allowed",
yes; but not "required".  If anything, it is easier to refer to a
previous function in the manual, when two or more functions are
described one after another.  By contrast, doc strings are never
"near" one another.

> Here you seem to have made the reverse choice.

I didn't like the original doc string, yes.  The manual I didn't
change, it was written that way to begin with.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Dmitry Gutov
On 14.04.2019 17:40, Eli Zaretskii wrote:

> Yes, you've said that in the past, and I think we agreed to disagree
> on it.  My opinion is that it's a judgment call: sometimes duplication
> is for the better, and sometimes for the worse.

I hope you are taking in consideration the increased overhead of
maintaining it when adding further changes to either of these functions.

> In this case, the
> original doc string of json-parse-buffer said almost nothing useful,

It gave the gist, described the behavior unique to the current function,
and directed the reader for more details about the behavior.

Which was quite enough for me to learn how to use it, and where the
improvement should be.

> leaving almost everything to look up in another function's
> documentation, which IME is annoying.  Referring to another doc string
> is OK for some secondary details, or perhaps for some very complicated
> issues.  Not for the main part of the doc.

I would maybe add "returns a Lisp structure corresponding to the JSON
text contained in the buffer" to the original docstring. Enumerating the
possible return values means creating more places where the doc can
become out of date. And we don't have a linter for those mistakes.

And enumeration of the optional keyword arguments sounds one more step
removed from "the main part of the doc" to me.

> Especially for a function
> whose name doesn't necessarily speak volumes about its purpose.

Which of the two functions? Both of them seem to have pretty apt names.

>> Didn't we discuss the difference between docstrings and manuals some
>> time ago, where you expressed the opinion that docstrings are allowed
>> the kind of "see XX for more detail" references, and it's the manuals
>> where information sometimes has to be reiterated for the convenience of
>> the reader?
>
> Doesn't sound like something I'd say, not to that effect.  "Allowed",
> yes; but not "required".

Even if it said "Allowed", I'd interpret it like "I'm allowed to
structure the documentation this way, without expecting somebody else to
come later and rewrite it with increased duplication".

> If anything, it is easier to refer to a
> previous function in the manual, when two or more functions are
> described one after another.  By contrast, doc strings are never
> "near" one another.

When one references another? It's always fast for the user to navigate
to the other docstring.

Whereas in the case of the manual they might have to go back a page, for
instance.

Anyway, this is my opinion. Sorry if you heard all of this before already.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Eli Zaretskii
> Cc: [hidden email]
> From: Dmitry Gutov <[hidden email]>
> Date: Sun, 14 Apr 2019 23:34:25 +0300
>
> On 14.04.2019 17:40, Eli Zaretskii wrote:
>
> > Yes, you've said that in the past, and I think we agreed to disagree
> > on it.  My opinion is that it's a judgment call: sometimes duplication
> > is for the better, and sometimes for the worse.
>
> I hope you are taking in consideration the increased overhead of
> maintaining it when adding further changes to either of these functions.

I did.  We have enough similar doc strings already; one more or less
won't change anything.

> > Especially for a function
> > whose name doesn't necessarily speak volumes about its purpose.
>
> Which of the two functions? Both of them seem to have pretty apt names.

Not IMO.  "Parse" is ambiguous, and doesn't hint on the fact that
these functions produce a Lisp representation of a JSON object.

> > Doesn't sound like something I'd say, not to that effect.  "Allowed",
> > yes; but not "required".
>
> Even if it said "Allowed", I'd interpret it like "I'm allowed to
> structure the documentation this way, without expecting somebody else to
> come later and rewrite it with increased duplication".

Please don't take my changes as some kind of indirect accusation
against you.  It was just a routine maintenance job, something I do
almost every day when I see documentation that can be improved.

> > If anything, it is easier to refer to a
> > previous function in the manual, when two or more functions are
> > described one after another.  By contrast, doc strings are never
> > "near" one another.
>
> When one references another? It's always fast for the user to navigate
> to the other docstring.

It's at least one more keystroke.  More importantly, the doc strings
are slightly different, because some of the things one function does
make no sense for the other.  So the reader will also have to decide
which parts are not relevant.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Dmitry Gutov
On 15.04.2019 17:57, Eli Zaretskii wrote:

> I did.  We have enough similar doc strings already; one more or less
> won't change anything.

I admit to being a bit nervous for the remaining "dry" docstrings.

>> Which of the two functions? Both of them seem to have pretty apt names.
>
> Not IMO.  "Parse" is ambiguous, and doesn't hint on the fact that
> these functions produce a Lisp representation of a JSON object.

Erm. Well, I thought it's a given. It is something we can explain with
just one sentence, though (see an example in my previous message).

> Please don't take my changes as some kind of indirect accusation
> against you.  It was just a routine maintenance job, something I do
> almost every day when I see documentation that can be improved.

It's not personal, but it's something I'd have to deal with next time
I'm making a change there (if that happens). I also liked the previous
version better, or at least some aspects of it.

Speaking of routine maintenance, since you're asking for documentation
changes together with the patches now, it might be worth it to
standardize the approach more.

> It's at least one more keystroke.  More importantly, the doc strings
> are slightly different, because some of the things one function does
> make no sense for the other.  So the reader will also have to decide
> which parts are not relevant.

It's not like I disagree with the whole commit altogether. That part
could be improved. But "refer to that other docstring for the
description of the optional keyword arguments" is something that made
sense to me. Enumerating all the possible return types in just one place
is another thing.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Eli Zaretskii
> Cc: [hidden email]
> From: Dmitry Gutov <[hidden email]>
> Date: Mon, 15 Apr 2019 18:42:50 +0300
>
> Speaking of routine maintenance, since you're asking for documentation
> changes together with the patches now, it might be worth it to
> standardize the approach more.

I usually convey that through review comments, as much as
"standardize" is attainable when talking about something like our
documentation, which is basically slightly formalized free text.

> It's not like I disagree with the whole commit altogether. That part
> could be improved. But "refer to that other docstring for the
> description of the optional keyword arguments" is something that made
> sense to me. Enumerating all the possible return types in just one place
> is another thing.

I cannot convince you, but I do feel that my change was for the
better.  Just trust me on that, OK?

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] master 2475687: Improve documentation changes of a recent commit

Dmitry Gutov
On 15.04.2019 19:06, Eli Zaretskii wrote:

> I usually convey that through review comments, as much as
> "standardize" is attainable when talking about something like our
> documentation, which is basically slightly formalized free text.

Apparently, I prefer documentation that is more like code. :-)

> I cannot convince you, but I do feel that my change was for the
> better.  Just trust me on that, OK?

OK. I will stop arguing for a while, at least.