bug#67153: [PATCH 2/2] Add 5 docstrings to abbrev.el

[Jeremy Bryant]

0001-Add-5-docstrings-to-abbrev.el.patch
Description: Text Data

Attached is a revised patch addressing comments of Eli and Stefan.

Further clarifications below:-


Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>  (defun prepare-abbrev-list-buffer (&optional local)
>> +  "Return *Abbrevs* buffer for the caller to select or display.
>
> A few points:
>
> - I don't think the docstring should document to the name of the buffer.
> - Docstrings should usually say what the function does rather than what
>   the callers are expected to do with the result.

Initially, I had simply lifted the text from the file ChangeLog.1
1985-12-13  Richard M. Stallman  (rms@prep)
        * abbrev.el (prepare-abbrev-list-buffer, list-abbrevs,
        edit-abbrevs):
        Some cleanups.  prepare-... now does all the work and
        returns the buffer for the caller to select or display.


But I have now rewritten.


> - To me, the above description makes it sound like the function does
>   little more than `(get-buffer "*Abbrevs*")`.
>   IOW, it should say that it fills the buffer with "some" representation
>   of "some" abbrevs.
>
>> +LOCAL is a flag, if non-nil display only local abbrevs.
>> +"
>
> We don't like our docstrings to end with a newline.

Fixed.

>
>> +A negative ARG means to undefine the specified abbrev.
> [...]
>> +This command reads the abbreviation from the minibuffer.
>
> We should say this before referring to it via "the specified abbrev".

Fixed in both add-abbrev and inverse-add-abbrev.
I initially adapted the docstring from surrounding callers like
add-global-abbrev, but have now rewritten.

>
>> +TYPE of abbrev includes \"Global\", \"Mode\".
>
> Here you made the same mistake of documenting what the callers do rather
> than what the function does.

Fixed, rewritten, it turns out the ARG string is purely descriptive.

>
>> +See `add-global-abbrev', `add-mode-abbrev' for caller examples.
>
> We usually don't include this in docstrings.
> We have `xref` and `grep` for those kinds of things.

Fixed

>>  (defun abbrev--describe (sym)
>> +  "Describe abbrev SYM.
>> +Used to generate a table by `insert-abbrev-table-description'."
>
> Similarly here I wouldn't mention the caller.  Instead I'd try and
> explain what kind of description is generated and where it's placed
> (at point in the current buffer?  As a return values? ...).

Fixed with this detail after examining the code.
>
>>  (defun abbrev--possibly-save (query &optional arg)
>> +  "Hook function for use by `save-some-buffer-functions'.
>> +Associated meaning for QUERY and ARG."
>
> Hook functions are one of the exceptions where it's often OK to refer to
> how it's used in the docstring, like you do here (e.g. so we don't have
> to repeat what `query` and `arg` are for).
> But it should also say what it actually does.

Fixed.

>
>
>         Stefan