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

From: Eli Zaretskii <[email-masked]>
Date: Thu, 16 Nov 2023 08:47:18 +0200

> Cc: [email-masked], [email-masked], [email-masked]
> Date: Wed, 15 Nov 2023 23:24:48 +0000
> From:  Jeremy Bryant via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <[email-masked]>
> 
>  (defun prepare-abbrev-list-buffer (&optional local)
> +  "Return buffer listing abbreviations and expansions for each abbrev table.
> +
> +LOCAL is a flag, if non-nil display only local abbrevs."

Our style for expressing what you say in the last sentence is

  If LOCAL is non-nil, include in the buffer only the local abbrevs.

("Display" is not appropriate here, since this function doesn't
display the buffer.)

>  (defun add-abbrev (table type arg)
> +  "Define abbrev in TABLE, whose expansion is ARG words before point.
> +This command reads the abbreviation from the minibuffer, with prompt TYPE.
        ^^^^^^^
This is not a command.

> +ARG of zero means the entire region is the expansion.
> +If there's an active region, use that as the expansion.

Is the second sentence conditioned on the first, i.e., does the
function use the active region only when ARG is zero?  If so, the
second sentence should be removed.  If use of the active region is NOT
conditioned on ARG, I would reword the above:

  If there's an active region, use the region as the expansion.
  ARG of zero means the region is the expansion, even if it is
  inactive.

>  (defun inverse-add-abbrev (table type arg)
> +  "Define the word before point as an abbrev in TABLE.
> +This command reads the expansion from the minibuffer, using prompt TYPE,
> +defines the abbrev, and then expands the abbreviation in the current buffer.
> +
> +ARG means use the ARG-th word before point as the abbreviation.
> +Negative ARG means use the ARG-th word after point.
> +
> +TYPE is an arbitrary string used to prompt user for the kind of
> +abbrev, such as \"Global\", \"Mode\".  (This has no influence on the
> +choice of the actual TABLE).
> +
> +See also `add-abbrev', which performs the opposite task."

Same comments here.

>  (defun abbrev--possibly-save (query &optional arg)
> +  "Maybe save abbrevs: Hook function for use by `save-some-buffer-functions'.

I'd say here just

  Hook function for use by `save-some-buffer-functions'.

and add the "Maybe save" bit as a separate second sentence.

> +Associated meaning for QUERY and ARG."

I couldn't parse this sentence.

Thanks.