* bug#36588: Unable to revert M-x apropos help buffer @ 2019-07-11 3:31 Stefan Kangas 2019-07-11 14:53 ` Basil L. Contovounesios 0 siblings, 1 reply; 4+ messages in thread From: Stefan Kangas @ 2019-07-11 3:31 UTC (permalink / raw) To: 36588 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@ido-cr+-record-current-command(#<subr call-interactively> revert-buffer nil nil) apply(call-interactively@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* bug#36588: Unable to revert M-x apropos help buffer 2019-07-11 3:31 bug#36588: Unable to revert M-x apropos help buffer Stefan Kangas @ 2019-07-11 14:53 ` Basil L. Contovounesios 2019-07-12 5:03 ` Stefan Kangas 0 siblings, 1 reply; 4+ messages in thread From: Basil L. Contovounesios @ 2019-07-11 14:53 UTC (permalink / raw) To: Stefan Kangas; +Cc: 36588 [-- Attachment #1: Type: text/plain, Size: 1856 bytes --] severity 36588 minor tags 36588 + patch quit Stefan Kangas <stefan@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@ido-cr+-record-current-command(#<subr > call-interactively> revert-buffer nil nil) > apply(call-interactively@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: [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #2: 0001-Support-reverting-Apropos-buffers-bug-36588.patch --] [-- Type: text/x-diff, Size: 4253 bytes --] From 890d2d6c1447f207892be734ab7412b4b471ddb7 Mon Sep 17 00:00:00 2001 From: "Basil L. Contovounesios" <contovob@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.") + \f ;;; 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 [-- Attachment #3: Type: text/plain, Size: 284 bytes --] 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#36588: Unable to revert M-x apropos help buffer 2019-07-11 14:53 ` Basil L. Contovounesios @ 2019-07-12 5:03 ` Stefan Kangas 2019-08-04 22:45 ` Basil L. Contovounesios 0 siblings, 1 reply; 4+ messages in thread From: Stefan Kangas @ 2019-07-12 5:03 UTC (permalink / raw) To: Basil L. Contovounesios; +Cc: 36588 [-- Attachment #1: Type: text/plain, Size: 1397 bytes --] Basil L. Contovounesios <contovob@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 [-- Attachment #2: 0001-test-lisp-apropos-tests.el-New-file.patch --] [-- Type: application/octet-stream, Size: 1681 bytes --] From 562d2622048be8ac5509d8ccd4b9fd18244fc3a4 Mon Sep 17 00:00:00 2001 From: Stefan Kangas <stefankangas@gmail.com> Date: Fri, 12 Jul 2019 06:57:29 +0200 Subject: [PATCH] * test/lisp/apropos-tests.el: New file. --- test/lisp/apropos-tests.el | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 test/lisp/apropos-tests.el diff --git a/test/lisp/apropos-tests.el b/test/lisp/apropos-tests.el new file mode 100644 index 0000000000..44737d902e --- /dev/null +++ b/test/lisp/apropos-tests.el @@ -0,0 +1,34 @@ +;;; apropos-tests.el --- Tests for apropos.el -*- lexical-binding: t; -*- + +;; Copyright (C) 2019 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see <https://www.gnu.org/licenses/>. + +;;; Commentary: + +;;; Code: + +(require 'ert) +(require 'apropos) + +(ert-deftest apropos-tests-revert-buffer () + (with-temp-buffer + (apropos "foo") + (revert-buffer nil t) + (should (looking-at "Type RET on a type label")))) + +(provide 'apropos-tests) +;;; apropos-tests.el ends here -- 2.21.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* bug#36588: Unable to revert M-x apropos help buffer 2019-07-12 5:03 ` Stefan Kangas @ 2019-08-04 22:45 ` Basil L. Contovounesios 0 siblings, 0 replies; 4+ messages in thread From: Basil L. Contovounesios @ 2019-08-04 22:45 UTC (permalink / raw) To: Stefan Kangas; +Cc: 36588-done tags 36588 fixed close 36588 27.1 quit Stefan Kangas <stefan@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@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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-04 22:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-07-11 3:31 bug#36588: Unable to revert M-x apropos help buffer Stefan Kangas 2019-07-11 14:53 ` Basil L. Contovounesios 2019-07-12 5:03 ` Stefan Kangas 2019-08-04 22:45 ` Basil L. Contovounesios
Code repositories for project(s) associated with this external index https://git.savannah.gnu.org/cgit/emacs.git https://git.savannah.gnu.org/cgit/emacs/org-mode.git This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.