GNU bug report logs - #36588
Unable to revert M-x apropos help buffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36588 in the body.
You can then email your comments to 36588 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: bug-gnu-emacs <at> gnu.org
Subject: Unable to revert M-x apropos help buffer
Date: Thu, 11 Jul 2019 05:31:59 +0200

I get an error trying to revert the apropos help buffer on Emacs 27
(current master).  I could also reproduce this on 26.1.

Steps to reproduce:
0. emacs -Q
1. M-x apropos RET foo RET
2. C-x o
3. g yes RET
4. Error:
apply: Symbol’s function definition is void: nil

Backtrace:
Debugger entered--Lisp error: (void-function nil)
  nil()
  apply(nil nil)
  help-mode-revert-buffer(t nil)
  revert-buffer(t)
  funcall-interactively(revert-buffer t)
  #<subr call-interactively>(revert-buffer nil nil)
  apply(#<subr call-interactively> revert-buffer (nil nil))
  call-interactively <at> ido-cr+-record-current-command(#<subr
call-interactively> revert-buffer nil nil)
  apply(call-interactively <at> ido-cr+-record-current-command #<subr
call-interactively> (revert-buffer nil nil))
  call-interactively(revert-buffer nil nil)
  command-execute(revert-buffer)

GNU Emacs 27.0.50 (build 5, x86_64-apple-darwin15.6.0, NS
appkit-1404.47 Version 10.11.6 (Build 15G22010)) of 2019-07-11

Thanks,
Stefan Kangas

Message #8 received at 36588 <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36588 <at> debbugs.gnu.org
Subject: Re: bug#36588: Unable to revert M-x apropos help buffer
Date: Thu, 11 Jul 2019 15:53:56 +0100

[Message part 1 (text/plain, inline)]

severity 36588 minor
tags 36588 + patch
quit

Stefan Kangas <stefan <at> marxist.se> writes:

> I get an error trying to revert the apropos help buffer on Emacs 27
> (current master).  I could also reproduce this on 26.1.
>
> Steps to reproduce:
> 0. emacs -Q
> 1. M-x apropos RET foo RET
> 2. C-x o
> 3. g yes RET
> 4. Error:
> apply: Symbol’s function definition is void: nil
>
> Backtrace:
> Debugger entered--Lisp error: (void-function nil)
>   nil()
>   apply(nil nil)
>   help-mode-revert-buffer(t nil)
>   revert-buffer(t)
>   funcall-interactively(revert-buffer t)
>   #<subr call-interactively>(revert-buffer nil nil)
>   apply(#<subr call-interactively> revert-buffer (nil nil))
>   call-interactively <at> ido-cr+-record-current-command(#<subr
> call-interactively> revert-buffer nil nil)
>   apply(call-interactively <at> ido-cr+-record-current-command #<subr
> call-interactively> (revert-buffer nil nil))
>   call-interactively(revert-buffer nil nil)
>   command-execute(revert-buffer)

This is because Apropos buffers are set up in apropos-print using
with-output-to-temp-buffer, which by default calls help-mode, which sets
revert-buffer-function permanently-locally to help-mode-revert-buffer.

[Aside: Why is revert-buffer-function permanent-local?]

help-mode-revert-buffer expects the command which generated the Help
buffer to have previously called help-setup-xref.  This should be
possible to do in Apropos, but it would be quite ugly as help-setup-xref
needs to be called with the Help buffer current, so Apropos commands
would need to set their own version of help-xref-stack-item before
apropos-print performs the help-setup-xref dance.  Besides, Apropos
buffers are not (currently) really Help buffers.

Instead, I propose emulating a simpler version of help-setup-xref
specific to Apropos:

[0001-Support-reverting-Apropos-buffers-bug-36588.patch (text/x-diff, inline)]

From 890d2d6c1447f207892be734ab7412b4b471ddb7 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Date: Thu, 11 Jul 2019 15:11:35 +0100
Subject: [PATCH] Support reverting Apropos buffers (bug#36588)

* lisp/apropos.el (apropos--current): New variable akin to
help-xref-stack-item storing information for revert-buffer.
(apropos--revert-buffer): New function.
(apropos-mode): Use it as revert-buffer-function.
(apropos-command, apropos, apropos-library, apropos-value)
(apropos-local-value, apropos-documentation): Set apropos--current
in low-level commands, i.e., those which do not call other commands.
---
 lisp/apropos.el | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/lisp/apropos.el b/lisp/apropos.el
index 1b86f5bcde..79e5a1518f 100644
--- a/lisp/apropos.el
+++ b/lisp/apropos.el
@@ -212,6 +212,12 @@ apropos-synonyms
 Each element is a list of words where the first word is the standard Emacs
 term, and the rest of the words are alternative terms.")
 
+(defvar apropos--current nil
+  "List of current Apropos function followed by its arguments.
+Used by `apropos--revert-buffer' to regenerate the current
+Apropos buffer.  Each Apropos command should ensure it is set
+before `apropos-mode' makes it buffer-local.")
+
 
 ;;; Button types used by apropos
 
@@ -472,10 +478,18 @@ apropos-true-hit-doc
   "Return t if DOC is really matched by the current keywords."
   (apropos-true-hit doc apropos-all-words))
 
+(defun apropos--revert-buffer (_ignore-auto noconfirm)
+  "Regenerate current Apropos buffer using `apropos--current'.
+Intended as a value for `revert-buffer-function'."
+  (when (or noconfirm (yes-or-no-p "Revert apropos buffer? "))
+    (apply #'funcall apropos--current)))
+
 (define-derived-mode apropos-mode special-mode "Apropos"
   "Major mode for following hyperlinks in output of apropos commands.
 
-\\{apropos-mode-map}")
+\\{apropos-mode-map}"
+  (make-local-variable 'apropos--current)
+  (setq-local revert-buffer-function #'apropos--revert-buffer))
 
 (defvar apropos-multi-type t
   "If non-nil, this apropos query concerns multiple types.
@@ -550,6 +564,7 @@ apropos-command
 		      (if (or current-prefix-arg apropos-do-all)
 			  "command or function" "command"))
 		     current-prefix-arg))
+  (setq apropos--current (list #'apropos-command pattern do-all var-predicate))
   (apropos-parse-pattern pattern)
   (let ((message
 	 (let ((standard-output (get-buffer-create "*Apropos*")))
@@ -628,6 +643,7 @@ apropos
 Returns list of symbols and documentation found."
   (interactive (list (apropos-read-pattern "symbol")
 		     current-prefix-arg))
+  (setq apropos--current (list #'apropos pattern do-all))
   (apropos-parse-pattern pattern)
   (apropos-symbols-internal
    (apropos-internal apropos-regexp
@@ -670,6 +686,7 @@ apropos-library
                          libs))
                   libs)))
      (list (completing-read "Describe library: " libs nil t))))
+  (setq apropos--current (list #'apropos-library file))
   (let ((symbols nil)
 	;; (autoloads nil)
 	(provides nil)
@@ -776,6 +793,7 @@ apropos-value
 Returns list of symbols and values found."
   (interactive (list (apropos-read-pattern "value")
 		     current-prefix-arg))
+  (setq apropos--current (list #'apropos-value pattern do-all))
   (apropos-parse-pattern pattern)
   (or do-all (setq do-all apropos-do-all))
   (setq apropos-accumulator ())
@@ -815,6 +833,7 @@ apropos-local-value
 Optional arg BUFFER (default: current buffer) is the buffer to check."
   (interactive (list (apropos-read-pattern "value of buffer-local variable")))
   (unless buffer (setq buffer  (current-buffer)))
+  (setq apropos--current (list #'apropos-local-value pattern buffer))
   (apropos-parse-pattern pattern)
   (setq apropos-accumulator  ())
   (let ((var             nil))
@@ -856,6 +875,7 @@ apropos-documentation
   ;; output, but I cannot see that that is true.
   (interactive (list (apropos-read-pattern "documentation")
 		     current-prefix-arg))
+  (setq apropos--current (list #'apropos-documentation pattern do-all))
   (apropos-parse-pattern pattern)
   (or do-all (setq do-all apropos-do-all))
   (setq apropos-accumulator () apropos-files-scanned ())
-- 
2.20.1

[Message part 3 (text/plain, inline)]

I don't consider this duplication because, unless Apropos were
completely refactored, setting something like apropos--current would be
necessary either way.  This has the added perk of not coupling Apropos
with Help any more than it currently needs to be.

WDYT?

Thanks,

-- 
Basil

Message #15 received at 36588 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: "Basil L. Contovounesios" <contovob <at> tcd.ie>
Cc: 36588 <at> debbugs.gnu.org
Subject: Re: bug#36588: Unable to revert M-x apropos help buffer
Date: Fri, 12 Jul 2019 07:03:31 +0200

[Message part 1 (text/plain, inline)]

Basil L. Contovounesios <contovob <at> tcd.ie> writes:

> This is because Apropos buffers are set up in apropos-print using
> with-output-to-temp-buffer, which by default calls help-mode, which sets
> revert-buffer-function permanently-locally to help-mode-revert-buffer.
>
> [Aside: Why is revert-buffer-function permanent-local?]
>
> help-mode-revert-buffer expects the command which generated the Help
> buffer to have previously called help-setup-xref.  This should be
> possible to do in Apropos, but it would be quite ugly as help-setup-xref
> needs to be called with the Help buffer current, so Apropos commands
> would need to set their own version of help-xref-stack-item before
> apropos-print performs the help-setup-xref dance.  Besides, Apropos
> buffers are not (currently) really Help buffers.
>
> Instead, I propose emulating a simpler version of help-setup-xref
> specific to Apropos:
>
>
> I don't consider this duplication because, unless Apropos were
> completely refactored, setting something like apropos--current would be
> necessary either way.  This has the added perk of not coupling Apropos
> with Help any more than it currently needs to be.
>
> WDYT?

Not familiar with this code, so I can't speak to your solution.  I can confirm
it fixes the problem though.  Thanks for looking at it so promptly.

I've attached a small patch with a test for this.

Thanks,
Stefan Kangas

[0001-test-lisp-apropos-tests.el-New-file.patch (application/octet-stream, attachment)]

Message #22 received at 36588-done <at> debbugs.gnu.org (full text, mbox):

From: "Basil L. Contovounesios" <contovob <at> tcd.ie>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36588-done <at> debbugs.gnu.org
Subject: Re: bug#36588: Unable to revert M-x apropos help buffer
Date: Mon, 05 Aug 2019 01:45:53 +0300

tags 36588 fixed
close 36588 27.1
quit

Stefan Kangas <stefan <at> marxist.se> writes:

> Not familiar with this code, so I can't speak to your solution.  I can confirm
> it fixes the problem though.  Thanks for looking at it so promptly.

Thanks, I've now pushed the patch to master[1] and am therefore closing
this report.

[1]: Support reverting Apropos buffers (bug#36588)
4cd41ba8de 2019-08-05 01:19:51 +0300
https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=4cd41ba8def704ce3bd2f3806176815fd696fa57

> I've attached a small patch with a test for this.

Thanks, but I'm not sure it's applicable here; see below.

> From 562d2622048be8ac5509d8ccd4b9fd18244fc3a4 Mon Sep 17 00:00:00 2001
> From: Stefan Kangas <stefankangas <at> gmail.com>
> Date: Fri, 12 Jul 2019 06:57:29 +0200
> Subject: [PATCH] * test/lisp/apropos-tests.el: New file.

[...]

> +(ert-deftest apropos-tests-revert-buffer ()
> +  (with-temp-buffer
> +    (apropos "foo")
> +    (revert-buffer nil t)
> +    (should (looking-at "Type RET on a type label"))))

[Nit: Use looking-at-p unless subsequently using the match data.]

I don't currently see a clean way to test this behaviour
non-interactively, as the apropos command takes over the hard-coded
*Apropos* buffer and can pop up new windows and frames (which also
renders the call to with-temp-buffer a no-op, AFAICT).  This is far too
intrusive for the ERT case.

Alternatives include refactoring apropos.el to make it more testable,
and adding manual tests under test/manual/, but either of these deserves
its own bug report.

Thanks,

-- 
Basil

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.