Subject: Re: bug#73637: [PATCH] New hook 'help-setup-hook'

From: Eshel Yaron <[email-masked]>
Date: Sat, 05 Oct 2024 10:46:06 +0200

Hi,

Eli Zaretskii <[email-masked]> writes:

>> Date: Sat, 05 Oct 2024 09:14:54 +0200
>> From:  Eshel Yaron via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <[email-masked]>
>
> Stefan, any comments?
>
>> --- a/lisp/help-fns.el
>> +++ b/lisp/help-fns.el
>> @@ -280,7 +280,12 @@ xref-backend-references
>>  
>>  (defun help-fns--setup-xref-backend ()
>>    (add-hook 'xref-backend-functions #'elisp--xref-backend nil t)
>> -  (setq-local semantic-symref-filepattern-alist '((help-mode "*.el"))))
>> +  (setq-local semantic-symref-filepattern-alist '((help-mode "*.el")))
>> +  (add-hook 'help-setup-hook
>> +            (lambda ()
>> +              (kill-local-variable 'xref-backend-functions)
>> +              (kill-local-variable 'semantic-symref-filepattern-alist))
>> +            nil t))
>
> Please add comments here to explain why the hook does these things.

OK, see updated patch below.

>> --- a/lisp/help-mode.el
>> +++ b/lisp/help-mode.el
>> @@ -492,6 +492,13 @@ help-xref-url-regexp
>>    (purecopy "\\<[Uu][Rr][Ll][ \t\n]+['`‘]\\([^'’]+\\)['’]")
>>    "Regexp matching doc string references to a URL.")
>>  
>> +(defvar-local help-setup-hook nil
>> +  "Functions to call with no arguments when populating \"*Help*\" buffers.
>
> Please mention in the doc string which function(s) call(s) this hook.

Done.

>> -    ;; Disable `outline-minor-mode' in a reused Help buffer
>> -    ;; created by `describe-bindings' that enables this mode.
>
> This useful comment is lost as result of your patch.  Is that on
> purpose?

Yes, this change obviates the need for this comment, as the code no
longer caters for describe-bindings specifically, it just runs a hook.

> Finally, I think this hook should be added to the list of the standard
> hooks in the ELisp manual.

Done in the patch below.  Thanks for taking a look.

>From 1a67fe980a80537da18a95119829806b031b33b4 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <[email-masked]>
Date: Sat, 5 Oct 2024 09:04:10 +0200
Subject: [PATCH v2] New hook 'help-setup-hook' (bug#73637)

* lisp/help-mode.el (help-setup-hook): New hook.
(help-setup-xref): Run it.
* lisp/help.el (describe-bindings):
* lisp/help-fns.el (help-fns--setup-xref-backend): Use it
for clean up.
* doc/lispref/help.texi (Help Functions): Document it.
* doc/lispref/hooks.texi (Standard Hooks): Mention it.
---
 doc/lispref/help.texi  |  6 ++++++
 doc/lispref/hooks.texi |  3 +++
 lisp/help-fns.el       |  8 +++++++-
 lisp/help-mode.el      | 26 ++++++++++----------------
 lisp/help.el           | 16 +++++++++++++++-
 5 files changed, 41 insertions(+), 18 deletions(-)

diff --git a/doc/lispref/help.texi b/doc/lispref/help.texi
index cc3c16f896d..7d3ae070ab2 100644
--- a/doc/lispref/help.texi
+++ b/doc/lispref/help.texi
@@ -772,6 +772,7 @@ Help Functions
 last value in @var{body}.
 @end defmac
 
+@vindex help-setup-hook
 @defun help-setup-xref item interactive-p
 This function updates the cross reference data in the @file{*Help*}
 buffer, which is used to regenerate the help information when the user
@@ -783,6 +784,11 @@ Help Functions
 The @var{interactive-p} argument is non-@code{nil} if the calling
 command was invoked interactively; in that case, the stack of items
 for the @file{*Help*} buffer's @samp{Back} buttons is cleared.
+
+This function runs the hook @code{help-setup-hook}, and resets the
+buffer-local value of that hook to its global value.  Functions that
+rely on bespoke settings in the @file{*Help*} buffer can use this hook
+to restore a clean state when another function reuses that buffer.
 @end defun
 
 @xref{describe-symbols example}, for an example of using
diff --git a/doc/lispref/hooks.texi b/doc/lispref/hooks.texi
index 9231d7573f9..645ec0f5bdf 100644
--- a/doc/lispref/hooks.texi
+++ b/doc/lispref/hooks.texi
@@ -180,6 +180,9 @@ Standard Hooks
 @item menu-bar-update-hook
 @xref{Menu Bar}.
 
+@item help-setup-hook
+@xref{Help Functions}.
+
 @item minibuffer-setup-hook
 @itemx minibuffer-exit-hook
 @xref{Minibuffer Misc}.
diff --git a/lisp/help-fns.el b/lisp/help-fns.el
index 5cc8df6497f..42463ea898b 100644
--- a/lisp/help-fns.el
+++ b/lisp/help-fns.el
@@ -280,7 +280,13 @@ xref-backend-references
 
 (defun help-fns--setup-xref-backend ()
   (add-hook 'xref-backend-functions #'elisp--xref-backend nil t)
-  (setq-local semantic-symref-filepattern-alist '((help-mode "*.el"))))
+  (setq-local semantic-symref-filepattern-alist '((help-mode "*.el")))
+  ;; Clean up after ourselves when the *Help* buffer is reused.
+  (add-hook 'help-setup-hook
+            (lambda ()
+              (kill-local-variable 'xref-backend-functions)
+              (kill-local-variable 'semantic-symref-filepattern-alist))
+            nil t))
 
 ;;;###autoload
 (defun describe-function (function)
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index 7bb69a9389f..8dece261581 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -492,6 +492,14 @@ help-xref-url-regexp
   (purecopy "\\<[Uu][Rr][Ll][ \t\n]+['`‘]\\([^'’]+\\)['’]")
   "Regexp matching doc string references to a URL.")
 
+(defvar-local help-setup-hook nil
+  "Functions to call with no arguments when populating \"*Help*\" buffers.
+
+`help-setup-xref' runs this hook.  It resets the buffer-local value of
+this variable after calling these functions, so that functions that you
+add to this hook buffer-locally are only called once, when another
+command reuses the \"*Help*\" buffer.")
+
 ;;;###autoload
 (defun help-setup-xref (item interactive-p)
   "Invoked from commands using the \"*Help*\" buffer to install some xref info.
@@ -505,22 +513,8 @@ help-setup-xref
 because we want to record the \"previous\" position of point so we can
 restore it properly when going back."
   (with-current-buffer (help-buffer)
-    ;; Disable `outline-minor-mode' in a reused Help buffer
-    ;; created by `describe-bindings' that enables this mode.
-    (when (bound-and-true-p outline-minor-mode)
-      (outline-minor-mode -1)
-      (mapc #'kill-local-variable
-            '(outline-search-function
-              outline-regexp
-              outline-heading-end-regexp
-              outline-level
-              outline-minor-mode-cycle
-              outline-minor-mode-highlight
-              outline-minor-mode-use-buttons
-              outline-default-state
-              outline-default-rules)))
-    (kill-local-variable 'xref-backend-functions)
-    (kill-local-variable 'semantic-symref-filepattern-alist)
+    (run-hooks 'help-setup-hook)
+    (kill-local-variable 'help-setup-hook)
     (when help-xref-stack-item
       (push (cons (point) help-xref-stack-item) help-xref-stack)
       (setq help-xref-forward-stack nil))
diff --git a/lisp/help.el b/lisp/help.el
index 5efe207c624..4f5646d9e93 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -806,7 +806,21 @@ describe-bindings
               ;; Hide ^Ls.
               (while (search-forward "\n\f\n" nil t)
 		(put-text-property (1+ (match-beginning 0)) (1- (match-end 0))
-                                   'invisible t)))))))))
+                                   'invisible t))))
+          ;; Clean up after ourselves when the *Help* buffer is reused.
+          (add-hook 'help-setup-hook
+                    (lambda ()
+                      (outline-minor-mode -1)
+                      (mapc #'kill-local-variable
+                            '(outline-regexp
+                              outline-heading-end-regexp
+                              outline-level
+                              outline-minor-mode-cycle
+                              outline-minor-mode-highlight
+                              outline-minor-mode-use-buttons
+                              outline-default-state
+                              outline-default-rules)))
+                    nil t))))))
 
 (defun where-is (definition &optional insert)
   "Print message listing key sequences that invoke the command DEFINITION.
-- 
2.46.2