GNU bug report logs - #72915
Docstrings of add-hook and remove-hook improvement?

Previous Next

Package: emacs;

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

From: help-debbugs <at> gnu.org (GNU bug Tracking System)
To: Stefan Kangas <stefankangas <at> gmail.com>
Cc: tracker <at> debbugs.gnu.org
Subject: bug#72915: closed (Docstrings of add-hook and remove-hook
 improvement?)
Date: Thu, 27 Feb 2025 22:16:02 +0000
[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)]
From: Tomas Nordin <tomasn <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Docstrings of add-hook and remove-hook improvement?
Date: Sat, 31 Aug 2024 12:36:22 +0000
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)]
From: Stefan Kangas <stefankangas <at> gmail.com>
To: Tomas Nordin <tomasn <at> posteo.net>, Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 72915-done <at> debbugs.gnu.org
Subject: Re: bug#72915: Docstrings of add-hook and remove-hook improvement?
Date: Thu, 27 Feb 2025 22:15:09 +0000
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.