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

[Jeremy Bryant via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Add-5-docstrings-to-abbrev.el.patch --]
[-- Type: text/x-diff, Size: 3638 bytes --]

From a8a1b01cab209d73dabf34fe7fcebe9075bc3ca0 Mon Sep 17 00:00:00 2001
From: Jeremy Bryant <jb@jeremybryant.net>
Date: Wed, 15 Nov 2023 23:15:46 +0000
Subject: [PATCH] Add 5 docstrings to abbrev.el

* lisp/abbrev.el (prepare-abbrev-list-buffer)
(add-abbrev)
(inverse-add-abbrev)
(abbrev--describe)
(abbrev--possibly-save): Add docstrings.
---
 lisp/abbrev.el | 39 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 36 insertions(+), 3 deletions(-)

diff --git a/lisp/abbrev.el b/lisp/abbrev.el
index e1311dbc83b..6269fd50adf 100644
--- a/lisp/abbrev.el
+++ b/lisp/abbrev.el
@@ -122,6 +122,9 @@ abbrev-table-name
     found))
 
 (defun prepare-abbrev-list-buffer (&optional local)
+  "Return buffer listing abbreviations and expansions for each abbrev table.
+
+If LOCAL is non-nil, include in the buffer only the local abbrevs."
   (let ((local-table local-abbrev-table))
     (with-current-buffer (get-buffer-create "*Abbrevs*")
       (erase-buffer)
@@ -333,6 +336,20 @@ add-global-abbrev
   (add-abbrev global-abbrev-table "Global" arg))
 
 (defun add-abbrev (table type arg)
+  "Define abbrev in TABLE, whose expansion is ARG words before point.
+Read the abbreviation from the minibuffer, with prompt TYPE.
+
+ARG of zero means the entire region is the expansion.
+
+A negative ARG means to undefine the specified abbrev.
+
+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 `inverse-add-abbrev' for the opposite task.
+
+Don't use this function in a Lisp program; use `define-abbrev' instead."
   (let ((exp
          (cond
           ((or (and (null arg) (use-region-p))
@@ -353,7 +370,7 @@ add-abbrev
     (if (or (null exp)
 	    (not (abbrev-expansion name table))
 	    (y-or-n-p (format "%s expands into \"%s\"; redefine? "
-			      name (abbrev-expansion name table))))
+                              name (abbrev-expansion name table))))
 	(define-abbrev table (downcase name) exp))))
 
 (defun inverse-add-mode-abbrev (n)
@@ -393,6 +410,19 @@ inverse-add-global-abbrev
   (inverse-add-abbrev global-abbrev-table "Global" n))
 
 (defun inverse-add-abbrev (table type arg)
+  "Define the word before point as an abbrev in TABLE.
+Read the expansion from the minibuffer, using prompt TYPE, define
+the abbrev, and then expand 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."
   (let (name exp start end)
     (save-excursion
       (forward-word (1+ (- arg)))
@@ -1102,6 +1132,8 @@ abbrev--write
   (insert ")\n"))
 
 (defun abbrev--describe (sym)
+  "Describe abbrev SYM.
+Print on `standard-output' the abbrev, count of use, expansion."
   (when (symbol-value sym)
     (prin1 (symbol-name sym))
     (if (null (abbrev-get sym :system))
@@ -1243,11 +1275,12 @@ edit-abbrevs-mode
   (setq font-lock-multiline nil))
 
 (defun abbrev--possibly-save (query &optional arg)
+  "Hook function for use by `save-some-buffer-functions'.
+
+Maybe save abbrevs, and record whether we either saved them or asked to."
   ;; Query mode.
   (if (eq query 'query)
       (and save-abbrevs abbrevs-changed)
-    ;; Maybe save abbrevs, and record whether we either saved them or
-    ;; asked to.
     (and save-abbrevs
          abbrevs-changed
          (prog1
-- 
2.40.1


[-- Attachment #2: Type: text/plain, Size: 3149 bytes --]


Thank you for your patience in explaining what is required.
New patch attached, with comments below.


Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: gerd@gnu.org, 67153@debbugs.gnu.org, rms@gnu.org
>> 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" <bug-gnu-emacs@gnu.org>
>> 
>>  (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.)

Noted, fixed

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

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

Fixed by removing sentence
>>  (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.

Reworded; region comment N/A.
>
>>  (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.

Reworded by moving comment.

>
>> +Associated meaning for QUERY and ARG."
>
> I couldn't parse this sentence.
>
> Thanks.

Initially I read Stefan's suggestion

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

and left this as guide to arguments, but have now removed arguments
entirely.

Please let me know if this is good to install or any other changes needed.