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

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.

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

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> 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.

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.

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

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 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.

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.

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

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 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?

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.