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

[Stefan Monnier]

> This patch adds 5 docstrings to abbrev.el where there were none before.

Thanks.  See some comments below.


        Stefan


>  (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.
- 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.

> +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".

> +TYPE of abbrev includes \"Global\", \"Mode\".

Here you made the same mistake of documenting what the callers do rather
than what the function does.

> +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.

>  (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? ...).

>  (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.


        Stefan