GNU bug report logs -
#72915
Docstrings of add-hook and remove-hook improvement?
Previous Next
Reported by: Tomas Nordin <tomasn <at> posteo.net>
Date: Sat, 31 Aug 2024 12:38:01 UTC
Severity: normal
Tags: patch
Fixed in version 30.1
Done: Stefan Kangas <stefankangas <at> gmail.com>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
[Message part 1 (text/plain, inline)]
Your message dated Thu, 27 Feb 2025 22:15:09 +0000
with message-id <CADwFkmk5n_uvASr1DxuknXckZ4QR64Wc59PPJHUv8Y_XVEJvFA <at> mail.gmail.com>
and subject line Re: bug#72915: Docstrings of add-hook and remove-hook improvement?
has caused the debbugs.gnu.org bug report #72915,
regarding Docstrings of add-hook and remove-hook improvement?
to be marked as done.
(If you believe you have received this mail in error, please contact
help-debbugs <at> gnu.org.)
--
72915: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=72915
GNU Bug Tracking System
Contact help-debbugs <at> gnu.org with problems
[Message part 2 (message/rfc822, inline)]
Hello List
In bug#70820 August 14, Stefan mentions that it is a common confusion to
think of the functions in a hook as hooks. It got my attention because I
belong to the confused ones every second year or so adding or removing
functions from a hook.
I suggest the provided patch as a small improvement of the function
documentation of add-hook and remove-hook. Maybe it doesn't mitigate the
confusion mentioned that much, but it seem to align better with the
manual as I read it.
What do you think?
In add-hook doc, lift up the paragraph about HOOK and FUNCTION and
remove the mention about first setting the HOOK to nil. I think that is
something internal to the add-hook function and not relevant to the
programmer calling the add-hook function? And then say that the
resulting hook will be a list both when the HOOK symbol is void or a
single function.
In remove-hook, stick to the notion that a hook contain functions to
run, not hooks.
This notion though is a bit confusing in relation to the names of those
functions, but that's another story I guess.
The following on top of emacs-30.
diff --git a/lisp/subr.el b/lisp/subr.el
index 28ba30f584e..e60c4119c60 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2090,6 +2090,10 @@ add-hook
"Add to the value of HOOK the function FUNCTION.
FUNCTION is not added if already present.
+HOOK should be a symbol. If HOOK is void, or if HOOK's value is a
+single function, it is changed to a list of functions (containing only
+FUNCTION in the void case).
+
The place where the function is added depends on the DEPTH
parameter. DEPTH defaults to 0. By convention, it should be
a number between -100 and 100 where 100 means that the function
@@ -2108,10 +2112,6 @@ add-hook
buffer-local value. That acts as a flag to run the hook
functions of the global value as well as in the local value.
-HOOK should be a symbol. If HOOK is void, it is first set to
-nil. If HOOK's value is a single function, it is changed to a
-list of functions.
-
FUNCTION may be any valid function, but it's recommended to use a
function symbol and not a lambda form. Using a symbol will
ensure that the function is not re-added if the function is
@@ -2179,7 +2179,7 @@ remove-hook
"Remove from the value of HOOK the function FUNCTION.
HOOK should be a symbol, and FUNCTION may be any valid function. If
FUNCTION isn't the value of HOOK, or, if FUNCTION doesn't appear in the
-list of hooks to run in HOOK, then nothing is done. See `add-hook'.
+list of functions to run in HOOK, then nothing is done. See `add-hook'.
The optional third argument, LOCAL, if non-nil, says to modify
the hook's buffer-local value rather than its default value.
[Message part 3 (message/rfc822, inline)]
Version: 30.1
Tomas Nordin <tomasn <at> posteo.net> writes:
> Stefan Kangas <stefankangas <at> gmail.com> writes:
>
>> Tomas Nordin <tomasn <at> posteo.net> writes:
>>
>>> Tomas Nordin <tomasn <at> posteo.net> writes:
>>>
>>>> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>>>
>>>>> The `add-hook` part was OK for me, indeed.
>>>>>
>>>>> [ I'm no great fan of that paragraph (neither the original nor the one
>>>>> you replace it with), tho, because it's a bit "too detailed" for my
>>>>> taste. E.g. the value *always* ends up being a list of functions, and
>>>>> the parenthesis states something which sounds to me like it should be
>>>>> inferrable from the rest of the docstring. ]
>>>>
>>>> I think I agree, the parenthesized part could be discarded, shortening
>>>> the paragraph to
>>>>
>>>> "HOOK should be a symbol. If HOOK is void, or if HOOK's value is a
>>>> single function, it is changed to a list of functions."
>>>
>>> I think nothing was installed along this discussion so far. Attached is
>>> an attempt to summarize the suggestions as patch.
>>
>> Thanks. I think this contribution is bigger than what we can accept
>> without a copyright assignment from you to the FSF.
>>
>> Please see etc/copyright-assign.txt in emacs.git to get started.
>
> Actually I got my assignment confirmed in November last year. It took
> some time, but it should be there on file somewhere. Should I provide
> the assignment number?
No need, thanks. I just pushed your patch to emacs-30 with the below
small change to use the imperative mood.
Thanks for the patch! Closing this bug now.
diff --git a/lisp/subr.el b/lisp/subr.el
index d0faff73e4a..7af21125b33 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -2177,8 +2177,8 @@ add-hook
(defun remove-hook (hook function &optional local)
"Remove FUNCTION from HOOK's functions.
HOOK should be a symbol, and FUNCTION may be any valid function.
-Does nothing if HOOK does not currently contain FUNCTION.
-Compares functions with `equal`, which means that it can be
+Do nothing if HOOK does not currently contain FUNCTION.
+Compare functions with `equal`, which means that it can be
slow if FUNCTION is not a symbol. See `add-hook'.
The optional third argument, LOCAL, if non-nil, says to modify
This bug report was last modified 106 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.