Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[Stefan Monnier]

ELPA Syncer [2021-10-28 21:57:54] wrote:
>     Fix checkdoc warn.
[...]
> -  "Advice function of `minibuffer-message'"
> +  "Advice function of `minibuffer-message'.
> +Argument MESSAGE ."

I think this fix is worse than the checkdoc warning.
In my experience checkdoc's warning about arguments not mentioned in the
docstring aren't very helpful (it's quite common that the arguments
aren't mentioned.  Here for example, it's the same args as those of
`minibuffer-message`, so there's no strong need to repeat the doc here.
Also the function could come without docstring in which case
checkdoc wouldn't complain about that missing arg doc).

Maybe we should change checkdoc not to request args be mentioned?
Or at least not for non-interactive functions?


        Stefan

Re:Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[tumashu]

At 2021-10-29 11:07:37, "Stefan Monnier" <monnier@iro.umontreal.ca> wrote:
>ELPA Syncer [2021-10-28 21:57:54] wrote:
>>     Fix checkdoc warn.
>[...]
>> -  "Advice function of `minibuffer-message'"
>> +  "Advice function of `minibuffer-message'.
>> +Argument MESSAGE ."
>
>I think this fix is worse than the checkdoc warning.
>In my experience checkdoc's warning about arguments not mentioned in the
>docstring aren't very helpful (it's quite common that the arguments
>aren't mentioned.  Here for example, it's the same args as those of
>`minibuffer-message`, so there's no strong need to repeat the doc here.
>Also the function could come without docstring in which case
>checkdoc wouldn't complain about that missing arg doc).
>
>Maybe we should change checkdoc not to request args be mentioned?
>Or at least not for non-interactive functions?

agree :-)

>
>
>        Stefan

Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Feng Shu <tumashu@163.com>
> Date: Thu, 28 Oct 2021 23:07:37 -0400
> 
> In my experience checkdoc's warning about arguments not mentioned in the
> docstring aren't very helpful (it's quite common that the arguments
> aren't mentioned.  Here for example, it's the same args as those of
> `minibuffer-message`, so there's no strong need to repeat the doc here.
> Also the function could come without docstring in which case
> checkdoc wouldn't complain about that missing arg doc).
> 
> Maybe we should change checkdoc not to request args be mentioned?
> Or at least not for non-interactive functions?

I'm not sure such a change will be for the better.  Many times people
don't reference the arguments there because they are unaware of our
style conventions, or because they cannot find the wording which would
mention the arguments and still stay within the 80-char limitation.
IME, it's a constant source of patch review comments, and having
checkdoc look at this is useful.  (I understand that none of the above
is the case for code that you, Stefan, write, but you are one of the
few exceptions in this regard.)

I'd like to remind people why we have this convention: it's because
some Help commands, such as apropos, only display the first line of
the doc string.  So it's important for that line to be as informant as
possible.

Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> Maybe we should change checkdoc not to request args be mentioned?
>> Or at least not for non-interactive functions?
[...]
> I'd like to remind people why we have this convention: it's because
> some Help commands, such as apropos, only display the first line of
> the doc string.  So it's important for that line to be as informant as
> possible.

It is also used in e.g. company-mode to show documentation for
completions.  The increasingly popular marginalia package also uses
it.[1]

To me, the important question is not "does this docstring include all
arguments", but "does the first line of the docstring work by itself".

For example, if the function name is `message-with-foo-method' and the
arguments are "&rest args", I can reasonably assume that ARGS are A)
strings that B) will be shown.  But the "foo-method" part might be is
the thing that really needs explaining, and deserves all space it can
get on the first line.

IOW, as Eli points out, the checkdoc warning is one heuristic that can
help detect possible problems, but it is IMO not the end all and be all
of writing docstrings.  I think Stefan Monnier makes an important point
about that.

My idea for how to improve it would to be for checkdoc to grow some kind
of way to disable warnings for individual docstrings (with a comment
before/after ";; disable checkdoc warning #123" or similar).  Such
annotations are not uncommon in other linters.

Footnotes:
[1] I recently started using marginalia, and I really recommend everyone
     to give it a spin.  It is on GNU ELPA and works OOTB, just install
     it from GNU ELPA and say `M-x marginalia-mode'.

Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[Stefan Monnier]

Eli Zaretskii [2021-10-29 09:29:15] wrote:
>> In my experience checkdoc's warning about arguments not mentioned in the
>> docstring aren't very helpful (it's quite common that the arguments
>> aren't mentioned.  Here for example, it's the same args as those of
>> `minibuffer-message`, so there's no strong need to repeat the doc here.
>> Also the function could come without docstring in which case
>> checkdoc wouldn't complain about that missing arg doc).
>> 
>> Maybe we should change checkdoc not to request args be mentioned?
>> Or at least not for non-interactive functions?
>
> I'm not sure such a change will be for the better.

To be honest, I'm not sure either.  I do find most that the warning
about args not mentioned in the docstring to be by far the one that
creates the largest number of "false positives" (in most cases, they're
actually not false in he sense that indeed the args aren't mentioned,
but their lack is not a serious problem).

IME, this warning corresponds to a WIBNI rather than a FIXME.

It's the main reason why I never try to reach "0 warnings" from checkdoc.

> Many times people don't reference the arguments there because they are
> unaware of our style conventions, or because they cannot find the
> wording which would mention the arguments and still stay within the
> 80-char limitation.

Hmm... AFAIK this warning is unrelated to the first line or the 80
columns conventions.  It just says things like:

    Argument ‘prompt’ should appear (as PROMPT) in the doc string

> (I understand that none of the above is the case for code that you,
> Stefan, write, but you are one of the few exceptions in this regard.)

I know I'm perfect, but actually I do find this (usually annoying)
warning occasionally useful, to prod me into completing a doc string.

> I'd like to remind people why we have this convention: it's because
> some Help commands, such as apropos, only display the first line of
> the doc string.  So it's important for that line to be as informant as
> possible.

I think you're talking about a different convention than the one I'm
talking about (and the patch I quoted fixes the one I'm talking about
but doesn't touche the first line of the docstring).


        Stefan

Re: [elpa] externals/vertico-posframe 39dbdfe 6/7: Fix checkdoc warn.

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org,  tumashu@163.com
> Date: Fri, 29 Oct 2021 09:42:14 -0400
> 
> > Many times people don't reference the arguments there because they are
> > unaware of our style conventions, or because they cannot find the
> > wording which would mention the arguments and still stay within the
> > 80-char limitation.
> 
> Hmm... AFAIK this warning is unrelated to the first line or the 80
> columns conventions.  It just says things like:
> 
>     Argument ‘prompt’ should appear (as PROMPT) in the doc string

People sometimes give up mentioning the arguments because they cannot
find good words to describe them.  This pressure is higher for the
first line.