GNU bug report logs -
#51173
28.0.60; gnus-article-describe-key doesn't work
Previous Next
Reported by: Katsumi Yamaoka <yamaoka <at> jpl.org>
Date: Wed, 13 Oct 2021 01:07:02 UTC
Severity: normal
Fixed in version 28.0.60
Done: Juri Linkov <juri <at> linkov.net>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 51173 in the body.
You can then email your comments to 51173 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 01:07:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Katsumi Yamaoka <yamaoka <at> jpl.org>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org.
(Wed, 13 Oct 2021 01:07:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
The following example program returns `sh-case' on Emacs 27 and
olders, however to make it work on Emacs 28 and 29 the third
line has to be uncommented. So does `describe-key' case.
(with-temp-buffer
(sh-mode)
;;(set-window-buffer nil (current-buffer))
(describe-key-briefly "\C-c\C-c"))
I don't know when/why those commands were changed to require the
buffer (where the keymap is) to be visited in the selected window,
but now `gnus-article-describe-key\(-briefly\)?' doesn't work
because of this. A patch to gnus-art.el is attached, though the
one that should be fixed might be help.el.
Thanks.
[Message part 2 (text/x-patch, inline)]
--- gnus-art.el~ 2021-10-06 01:11:50.777999500 +0000
+++ gnus-art.el 2021-10-13 01:04:18.073219200 +0000
@@ -6854,19 +6854,24 @@
(read-key-sequence "Describe key: ")))
gnus-article-mode)
(gnus-article-check-buffer)
- (if (memq (key-binding key t) '(gnus-article-read-summary-keys
- gnus-article-read-summary-send-keys))
- (with-current-buffer gnus-article-current-summary
- (setq unread-command-events
- (nconc
- (mapcar (lambda (x) (if (and (integerp x) (>= x 128))
- (list 'meta (- x 128))
- x))
- key)
- unread-command-events))
- (let ((cursor-in-echo-area t)
- gnus-pick-mode)
- (describe-key (read-key-sequence nil t))))
+ (if (and (memq (key-binding key t) '(gnus-article-read-summary-keys
+ gnus-article-read-summary-send-keys))
+ (buffer-live-p gnus-article-current-summary))
+ (let ((artbuf (current-buffer)))
+ (unwind-protect
+ (progn
+ (set-window-buffer nil gnus-article-current-summary)
+ (setq unread-command-events
+ (nconc
+ (mapcar (lambda (x) (if (and (integerp x) (>= x 128))
+ (list 'meta (- x 128))
+ x))
+ key)
+ unread-command-events))
+ (let ((cursor-in-echo-area t)
+ gnus-pick-mode)
+ (describe-key (read-key-sequence nil t))))
+ (set-window-buffer nil artbuf)))
(describe-key key)))
(defun gnus-article-describe-key-briefly (key &optional insert)
@@ -6877,19 +6882,24 @@
current-prefix-arg)
gnus-article-mode)
(gnus-article-check-buffer)
- (if (memq (key-binding key t) '(gnus-article-read-summary-keys
- gnus-article-read-summary-send-keys))
- (with-current-buffer gnus-article-current-summary
- (setq unread-command-events
- (nconc
- (mapcar (lambda (x) (if (and (integerp x) (>= x 128))
- (list 'meta (- x 128))
- x))
- key)
- unread-command-events))
- (let ((cursor-in-echo-area t)
- gnus-pick-mode)
- (describe-key-briefly (read-key-sequence nil t) insert)))
+ (if (and (memq (key-binding key t) '(gnus-article-read-summary-keys
+ gnus-article-read-summary-send-keys))
+ (buffer-live-p gnus-article-current-summary))
+ (let ((artbuf (current-buffer)))
+ (unwind-protect
+ (progn
+ (set-window-buffer nil gnus-article-current-summary)
+ (setq unread-command-events
+ (nconc
+ (mapcar (lambda (x) (if (and (integerp x) (>= x 128))
+ (list 'meta (- x 128))
+ x))
+ key)
+ unread-command-events))
+ (let ((cursor-in-echo-area t)
+ gnus-pick-mode)
+ (describe-key-briefly (read-key-sequence nil t) insert)))
+ (set-window-buffer nil artbuf)))
(describe-key-briefly key insert)))
;;`gnus-agent-mode' in gnus-agent.el will define it.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 12:00:03 GMT)
Full text and
rfc822 format available.
Message #8 received at 51173 <at> debbugs.gnu.org (full text, mbox):
Katsumi Yamaoka <yamaoka <at> jpl.org> writes:
> The following example program returns `sh-case' on Emacs 27 and
> olders, however to make it work on Emacs 28 and 29 the third
> line has to be uncommented. So does `describe-key' case.
>
> (with-temp-buffer
> (sh-mode)
> ;;(set-window-buffer nil (current-buffer))
> (describe-key-briefly "\C-c\C-c"))
>
> I don't know when/why those commands were changed to require the
> buffer (where the keymap is) to be visited in the selected window,
> but now `gnus-article-describe-key\(-briefly\)?' doesn't work
> because of this. A patch to gnus-art.el is attached, though the
> one that should be fixed might be help.el.
I think this sounds like a bug in describe-key*, so perhaps it should be
fixed there? Looking at the history, I'm not at all sure what caused
this regression, but perhaps it's:
commit 9d4af3e6bdfac374f6c9591566c010e6a1514751
Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
AuthorDate: Tue Jan 30 11:57:40 2018 -0500
I've added Stefan to the CCs.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 16:36:02 GMT)
Full text and
rfc822 format available.
Message #11 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> The following example program returns `sh-case' on Emacs 27 and
>> olders, however to make it work on Emacs 28 and 29 the third
>> line has to be uncommented. So does `describe-key' case.
>>
>> (with-temp-buffer
>> (sh-mode)
>> ;;(set-window-buffer nil (current-buffer))
>> (describe-key-briefly "\C-c\C-c"))
>>
>> I don't know when/why those commands were changed to require the
>> buffer (where the keymap is) to be visited in the selected window,
>> but now `gnus-article-describe-key\(-briefly\)?' doesn't work
>> because of this. A patch to gnus-art.el is attached, though the
>> one that should be fixed might be help.el.
>
> I think this sounds like a bug in describe-key*, so perhaps it should be
> fixed there? Looking at the history, I'm not at all sure what caused
> this regression, but perhaps it's:
>
> commit 9d4af3e6bdfac374f6c9591566c010e6a1514751
> Author: Stefan Monnier <monnier <at> iro.umontreal.ca>
> AuthorDate: Tue Jan 30 11:57:40 2018 -0500
>
> I've added Stefan to the CCs.
I think this is mea culpa - commit 2d1564103e.
It was changed to handle context menu clicks
in the displayed window.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 17:34:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 51173 <at> debbugs.gnu.org (full text, mbox):
> I think this is mea culpa - commit 2d1564103e.
> It was changed to handle context menu clicks
> in the displayed window.
This patch should handle only mouse events specially:
diff --git a/lisp/help.el b/lisp/help.el
index fa4eaee417..956a3d0d32 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -699,7 +699,10 @@ help--analyze-key
;; is selected from the context menu that should describe KEY
;; at the position of mouse click that opened the context menu.
;; When no mouse was involved, it defaults to window-point.
- (defn (save-excursion (mouse-set-point event) (key-binding key t))))
+ (defn (if (consp event)
+ (save-excursion
+ (mouse-set-point event) (key-binding key t))
+ (key-binding key t))))
;; Handle the case where we faked an entry in "Select and Paste" menu.
(when (and (eq defn nil)
(stringp (aref key (1- (length key))))
--
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 18:50:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 51173 <at> debbugs.gnu.org (full text, mbox):
Juri Linkov <juri <at> linkov.net> writes:
> This patch should handle only mouse events specially:
The patch makes both the with-temp-buffer test case as well as `C-h k'
work in Gnus article buffers, so it seems to fix the problem.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 19:20:02 GMT)
Full text and
rfc822 format available.
Message #20 received at 51173 <at> debbugs.gnu.org (full text, mbox):
close 51173 28.0.60
quit
>> This patch should handle only mouse events specially:
>
> The patch makes both the with-temp-buffer test case as well as `C-h k'
> work in Gnus article buffers, so it seems to fix the problem.
So now pushed to emacs-28.
bug marked as fixed in version 28.0.60, send any further explanations to
51173 <at> debbugs.gnu.org and Katsumi Yamaoka <yamaoka <at> jpl.org>
Request was from
Juri Linkov <juri <at> linkov.net>
to
control <at> debbugs.gnu.org.
(Wed, 13 Oct 2021 19:20:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 13 Oct 2021 20:03:02 GMT)
Full text and
rfc822 format available.
Message #25 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> I think this is mea culpa - commit 2d1564103e.
>> It was changed to handle context menu clicks
>> in the displayed window.
>
> This patch should handle only mouse events specially:
>
> diff --git a/lisp/help.el b/lisp/help.el
> index fa4eaee417..956a3d0d32 100644
> --- a/lisp/help.el
> +++ b/lisp/help.el
> @@ -699,7 +699,10 @@ help--analyze-key
> ;; is selected from the context menu that should describe KEY
> ;; at the position of mouse click that opened the context menu.
> ;; When no mouse was involved, it defaults to window-point.
> - (defn (save-excursion (mouse-set-point event) (key-binding key t))))
> + (defn (if (consp event)
> + (save-excursion
> + (mouse-set-point event) (key-binding key t))
> + (key-binding key t))))
> ;; Handle the case where we faked an entry in "Select and Paste" menu.
> (when (and (eq defn nil)
> (stringp (aref key (1- (length key))))
But this will still use "the wrong buffer" for mouse clicks, no?
BTW, maybe a cleaner fix would be as follows:
- Add a `buffer` argument to `describe-key(-briefly)`.
- Pass that argument from `gnus-article-describe-key`.
And to get the behavior that Juri just pushed that `buffer` argument
would default to (if (consp event) (window-buffer (posn-window
(event-start event))) (current-buffere)).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Thu, 14 Oct 2021 16:18:03 GMT)
Full text and
rfc822 format available.
Message #28 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> + (defn (if (consp event)
>> + (save-excursion
>> + (mouse-set-point event) (key-binding key t))
>> + (key-binding key t))))
>> ;; Handle the case where we faked an entry in "Select and Paste" menu.
>> (when (and (eq defn nil)
>> (stringp (aref key (1- (length key))))
>
> But this will still use "the wrong buffer" for mouse clicks, no?
It seems all mouse clicks expect the buffer where they were clicked
to be displayed in a window. I can't imagine how a mouse click
could originate from a hidden buffer.
> BTW, maybe a cleaner fix would be as follows:
> - Add a `buffer` argument to `describe-key(-briefly)`.
> - Pass that argument from `gnus-article-describe-key`.
> And to get the behavior that Juri just pushed that `buffer` argument
> would default to (if (consp event) (window-buffer (posn-window
> (event-start event))) (current-buffere)).
I don't know. All reported cases work now after Eli fixed
mouse-minibuffer-check not to raise an error when called from
mouse-set-point, so now it's safe to use it in help--analyze-key.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Thu, 14 Oct 2021 18:43:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>>> + (defn (if (consp event)
>>> + (save-excursion
>>> + (mouse-set-point event) (key-binding key t))
>>> + (key-binding key t))))
>>> ;; Handle the case where we faked an entry in "Select and Paste" menu.
>>> (when (and (eq defn nil)
>>> (stringp (aref key (1- (length key))))
>>
>> But this will still use "the wrong buffer" for mouse clicks, no?
>
> It seems all mouse clicks expect the buffer where they were clicked
> to be displayed in a window. I can't imagine how a mouse click
> could originate from a hidden buffer.
`gnus-article-describe-key` is used for those keybindings which Gnus
redirects from the buffer in which they occurred to some other buffer.
So if you use such a redirection for mouse-clicks,
`gnus-article-describe-key` would also want to look them up in the
other buffer.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Fri, 15 Oct 2021 07:03:02 GMT)
Full text and
rfc822 format available.
Message #34 received at 51173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
> But this will still use "the wrong buffer" for mouse clicks, no?
>
> BTW, maybe a cleaner fix would be as follows:
> - Add a `buffer` argument to `describe-key(-briefly)`.
> - Pass that argument from `gnus-article-describe-key`.
> And to get the behavior that Juri just pushed that `buffer` argument
> would default to (if (consp event) (window-buffer (posn-window
> (event-start event))) (current-buffere)).
Maybe something like this (but currently I have no idea how to test all cases):
[gnus-article-describe-key.patch (text/x-diff, inline)]
diff --git a/lisp/help.el b/lisp/help.el
index 9666ef9805..08a293c3dc 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -731,7 +731,7 @@ help--filter-info-list
;; If nothing left, then keep one (the last one).
(last info-list)))
-(defun describe-key-briefly (&optional key-list insert untranslated)
+(defun describe-key-briefly (&optional key-list insert buffer)
"Print the name of the functions KEY-LIST invokes.
KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
RAW-SEQ is the untranslated form of the key sequence SEQ.
@@ -739,8 +739,10 @@ describe-key-briefly
While reading KEY-LIST interactively, this command temporarily enables
menu items or tool-bar buttons that are disabled to allow getting help
-on them."
- (declare (advertised-calling-convention (key-list &optional insert) "27.1"))
+on them.
+
+BUFFER is the buffer in which to lookup those keys; it defaults to the
+current buffer."
(interactive
;; Ignore mouse movement events because it's too easy to miss the
;; message while moving the mouse.
@@ -748,15 +750,13 @@ describe-key-briefly
`(,key-list ,current-prefix-arg)))
(when (arrayp key-list)
;; Old calling convention, changed
- (setq key-list (list (cons key-list
- (if (numberp untranslated)
- (this-single-command-raw-keys)
- untranslated)))))
- (let* ((info-list (mapcar (lambda (kr)
- (help--analyze-key (car kr) (cdr kr)))
- key-list))
- (msg (mapconcat #'car (help--filter-info-list info-list 1) "\n")))
- (if insert (insert msg) (message "%s" msg))))
+ (setq key-list (list (cons key-list nil))))
+ (with-current-buffer (or buffer (current-buffer))
+ (let* ((info-list (mapcar (lambda (kr)
+ (help--analyze-key (car kr) (cdr kr)))
+ key-list))
+ (msg (mapconcat #'car (help--filter-info-list info-list 1) "\n")))
+ (if insert (insert msg) (message "%s" msg)))))
(defun help--key-binding-keymap (key &optional accept-default no-remap position)
"Return a keymap holding a binding for KEY within current keymaps.
diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index bb466b9400..bbb452279a 100644
--- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -6865,8 +6865,14 @@ gnus-article-describe-key
unread-command-events))
(let ((cursor-in-echo-area t)
gnus-pick-mode)
- (describe-key (read-key-sequence nil t))))
- (describe-key key)))
+ (let* ((key (read-key-sequence nil t))
+ (buffer (if (consp key)
+ (window-buffer (posn-window (event-start key)))
+ (current-buffer))))
+ (describe-key key buffer))))
+ (describe-key key (if (consp key)
+ (window-buffer (posn-window (event-start key)))
+ (current-buffer)))))
(defun gnus-article-describe-key-briefly (key &optional insert)
"Display documentation of the function invoked by KEY.
@@ -6888,8 +6894,15 @@ gnus-article-describe-key-briefly
unread-command-events))
(let ((cursor-in-echo-area t)
gnus-pick-mode)
- (describe-key-briefly (read-key-sequence nil t) insert)))
- (describe-key-briefly key insert)))
+ (let ((key (read-key-sequence nil t))
+ (buffer (if (consp key)
+ (window-buffer (posn-window (event-start key)))
+ (current-buffer))))
+ (describe-key-briefly key insert buffer))))
+ (describe-key-briefly key insert
+ (if (consp key)
+ (window-buffer (posn-window (event-start key)))
+ (current-buffer)))))
;;`gnus-agent-mode' in gnus-agent.el will define it.
(defvar gnus-agent-summary-mode)
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Fri, 15 Oct 2021 18:34:03 GMT)
Full text and
rfc822 format available.
Message #37 received at 51173 <at> debbugs.gnu.org (full text, mbox):
> + (let* ((key (read-key-sequence nil t))
> + (buffer (if (consp key)
> + (window-buffer (posn-window (event-start key)))
> + (current-buffer))))
> + (describe-key key buffer))))
> + (describe-key key (if (consp key)
> + (window-buffer (posn-window (event-start key)))
> + (current-buffer)))))
I think the first `describe-key` above should always use
`current-buffer` (that's the whole point of the function: to lookup the
keybinding in that other buffer).
And for the second, it's supposed to be a fallback that does whatever
`describe-key` does normally, so I don't see why we'd need/want this
(if ...) construction.
OTOH we should probably try and change the `key` arg to use the new
key-list format expected by `describe-key` (i.e. a list of (SEQ
. RAW-SEQ) pairs).
> + (let ((key (read-key-sequence nil t))
> + (buffer (if (consp key)
> + (window-buffer (posn-window (event-start key)))
> + (current-buffer))))
> + (describe-key-briefly key insert buffer))))
> + (describe-key-briefly key insert
> + (if (consp key)
> + (window-buffer (posn-window (event-start key)))
> + (current-buffer)))))
Same here.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Sat, 16 Oct 2021 18:01:02 GMT)
Full text and
rfc822 format available.
Message #40 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> + (let* ((key (read-key-sequence nil t))
>> + (buffer (if (consp key)
>> + (window-buffer (posn-window (event-start key)))
>> + (current-buffer))))
>> + (describe-key key buffer))))
>
> I think the first `describe-key` above should always use
> `current-buffer` (that's the whole point of the function: to lookup the
> keybinding in that other buffer).
There is still something missing: `gnus-article-describe-key`
already selects the required buffer with `with-current-buffer`.
Then why should it provide the same buffer as an argument
to `describe-key` to select it again in `describe-key`?
Maybe you intended to add a new argument `buffer` to pass it down
to `help--analyze-key` that could use it somehow in this condition:
(defn (if (consp event)
(save-excursion (mouse-set-point event) (key-binding key t))
(key-binding key t)))
But I have no idea how. Maybe not to set point (that also selects
another window) when the `buffer` arg is provided? E.g.
(defn (if (not buffer)
(save-excursion (mouse-set-point event) (key-binding key t))
(key-binding key t)))
IOW, the semantics of `buffer` is not clear here.
> OTOH we should probably try and change the `key` arg to use the new
> key-list format expected by `describe-key` (i.e. a list of (SEQ
> . RAW-SEQ) pairs).
`gnus-article-describe-key` just passes down the value
that `read-key-sequence` returns.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Sat, 16 Oct 2021 19:54:01 GMT)
Full text and
rfc822 format available.
Message #43 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>>> + (let* ((key (read-key-sequence nil t))
>>> + (buffer (if (consp key)
>>> + (window-buffer (posn-window (event-start key)))
>>> + (current-buffer))))
>>> + (describe-key key buffer))))
>>
>> I think the first `describe-key` above should always use
>> `current-buffer` (that's the whole point of the function: to lookup the
>> keybinding in that other buffer).
>
> There is still something missing: `gnus-article-describe-key`
> already selects the required buffer with `with-current-buffer`.
> Then why should it provide the same buffer as an argument
> to `describe-key` to select it again in `describe-key`?
Because `describe-key` otherwise uses the buffer of the window
associated with the event (this is in done in `help--analyse-key` where
we currently use `mouse-set-point`). So we need to pass an explicit
buffer to tell `describe-key` to ignore the event's window (and we need
to change this part of `describe-key/help--analyse-key` to obey such a
buffer argument).
> Maybe you intended to add a new argument `buffer` to pass it down
> to `help--analyze-key` that could use it somehow in this condition:
>
> (defn (if (consp event)
> (save-excursion (mouse-set-point event) (key-binding key t))
> (key-binding key t)))
Yes.
> But I have no idea how. Maybe not to set point (that also selects
> another window) when the `buffer` arg is provided?
Exactly.
> (defn (if (not buffer)
> (save-excursion (mouse-set-point event) (key-binding key t))
> (key-binding key t)))
>
> IOW, the semantics of `buffer` is not clear here.
If nil it means "defaults to the (window-buffer (posn-window (event-end event)))"
>> OTOH we should probably try and change the `key` arg to use the new
>> key-list format expected by `describe-key` (i.e. a list of (SEQ
>> . RAW-SEQ) pairs).
>
> `gnus-article-describe-key` just passes down the value
> that `read-key-sequence` returns.
I know. This is the old calling convention of `describe-key`; we
should move to the new one.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Mon, 18 Oct 2021 16:22:02 GMT)
Full text and
rfc822 format available.
Message #46 received at 51173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>> There is still something missing: `gnus-article-describe-key`
>> already selects the required buffer with `with-current-buffer`.
>> Then why should it provide the same buffer as an argument
>> to `describe-key` to select it again in `describe-key`?
>
> Because `describe-key` otherwise uses the buffer of the window
> associated with the event (this is in done in `help--analyse-key` where
> we currently use `mouse-set-point`). So we need to pass an explicit
> buffer to tell `describe-key` to ignore the event's window (and we need
> to change this part of `describe-key/help--analyse-key` to obey such a
> buffer argument).
>
>> (defn (if (not buffer)
>> (save-excursion (mouse-set-point event) (key-binding key t))
>> (key-binding key t)))
>>
>> IOW, the semantics of `buffer` is not clear here.
>
> If nil it means "defaults to the (window-buffer (posn-window (event-end event)))"
>
>>> OTOH we should probably try and change the `key` arg to use the new
>>> key-list format expected by `describe-key` (i.e. a list of (SEQ
>>> . RAW-SEQ) pairs).
>>
>> `gnus-article-describe-key` just passes down the value
>> that `read-key-sequence` returns.
>
> I know. This is the old calling convention of `describe-key`; we
> should move to the new one.
Is seems everything is covered by this patch:
[help--analyze-key.patch (text/x-diff, inline)]
diff --git a/lisp/gnus/gnus-art.el b/lisp/gnus/gnus-art.el
index bb466b9400..930c739a73 100644
--- a/lisp/gnus/gnus-art.el
+++ b/lisp/gnus/gnus-art.el
@@ -6865,7 +6865,9 @@ gnus-article-describe-key
unread-command-events))
(let ((cursor-in-echo-area t)
gnus-pick-mode)
- (describe-key (read-key-sequence nil t))))
+ (describe-key (cons (read-key-sequence nil t)
+ (this-single-command-raw-keys))
+ (current-buffer))))
(describe-key key)))
(defun gnus-article-describe-key-briefly (key &optional insert)
@@ -6888,7 +6890,9 @@ gnus-article-describe-key-briefly
unread-command-events))
(let ((cursor-in-echo-area t)
gnus-pick-mode)
- (describe-key-briefly (read-key-sequence nil t) insert)))
+ (describe-key-briefly (cons (read-key-sequence nil t)
+ (this-single-command-raw-keys))
+ insert (current-buffer))))
(describe-key-briefly key insert)))
;;`gnus-agent-mode' in gnus-agent.el will define it.
diff --git a/lisp/help.el b/lisp/help.el
index 9666ef9805..a7084d29ce 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -677,9 +677,11 @@ help-key-description
(defun help--binding-undefined-p (defn)
(or (null defn) (integerp defn) (equal defn 'undefined)))
-(defun help--analyze-key (key untranslated)
+(defun help--analyze-key (key untranslated &optional buffer)
"Get information about KEY its corresponding UNTRANSLATED events.
-Returns a list of the form (BRIEF-DESC DEFN EVENT MOUSE-MSG)."
+Returns a list of the form (BRIEF-DESC DEFN EVENT MOUSE-MSG).
+When BUFFER is nil, it defaults to the
+`(window-buffer (posn-window (event-end event)))'."
(if (numberp untranslated)
(error "Missing `untranslated'!"))
(let* ((event (when (> (length key) 0)
@@ -699,9 +701,8 @@ help--analyze-key
;; is selected from the context menu that should describe KEY
;; at the position of mouse click that opened the context menu.
;; When no mouse was involved, don't use `mouse-set-point'.
- (defn (if (consp event)
- (save-excursion (mouse-set-point event) (key-binding key t))
- (key-binding key t))))
+ (defn (if buffer (key-binding key t)
+ (save-excursion (mouse-set-point event) (key-binding key t)))))
;; Handle the case where we faked an entry in "Select and Paste" menu.
(when (and (eq defn nil)
(stringp (aref key (1- (length key))))
@@ -731,7 +732,7 @@ help--filter-info-list
;; If nothing left, then keep one (the last one).
(last info-list)))
-(defun describe-key-briefly (&optional key-list insert untranslated)
+(defun describe-key-briefly (&optional key-list insert buffer)
"Print the name of the functions KEY-LIST invokes.
KEY-LIST is a list of pairs (SEQ . RAW-SEQ) of key sequences, where
RAW-SEQ is the untranslated form of the key sequence SEQ.
@@ -739,8 +740,10 @@ describe-key-briefly
While reading KEY-LIST interactively, this command temporarily enables
menu items or tool-bar buttons that are disabled to allow getting help
-on them."
- (declare (advertised-calling-convention (key-list &optional insert) "27.1"))
+on them.
+
+BUFFER is the buffer in which to lookup those keys; it defaults to the
+current buffer."
(interactive
;; Ignore mouse movement events because it's too easy to miss the
;; message while moving the mouse.
@@ -748,15 +751,13 @@ describe-key-briefly
`(,key-list ,current-prefix-arg)))
(when (arrayp key-list)
;; Old calling convention, changed
- (setq key-list (list (cons key-list
- (if (numberp untranslated)
- (this-single-command-raw-keys)
- untranslated)))))
- (let* ((info-list (mapcar (lambda (kr)
- (help--analyze-key (car kr) (cdr kr)))
- key-list))
- (msg (mapconcat #'car (help--filter-info-list info-list 1) "\n")))
- (if insert (insert msg) (message "%s" msg))))
+ (setq key-list (list (cons key-list nil))))
+ (with-current-buffer (if (buffer-live-p buffer) buffer (current-buffer))
+ (let* ((info-list (mapcar (lambda (kr)
+ (help--analyze-key (car kr) (cdr kr) buffer))
+ key-list))
+ (msg (mapconcat #'car (help--filter-info-list info-list 1) "\n")))
+ (if insert (insert msg) (message "%s" msg)))))
(defun help--key-binding-keymap (key &optional accept-default no-remap position)
"Return a keymap holding a binding for KEY within current keymaps.
@@ -916,7 +917,7 @@ describe-key
(mapcar (lambda (x)
(pcase-let* ((`(,seq . ,raw-seq) x)
(`(,brief-desc ,defn ,event ,_mouse-msg)
- (help--analyze-key seq raw-seq))
+ (help--analyze-key seq raw-seq buffer))
(locus
(help--binding-locus
seq (event-start event))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 20 Oct 2021 17:56:01 GMT)
Full text and
rfc822 format available.
Message #49 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>>>> OTOH we should probably try and change the `key` arg to use the new
>>>> key-list format expected by `describe-key` (i.e. a list of (SEQ
>>>> . RAW-SEQ) pairs).
>>>
>>> `gnus-article-describe-key` just passes down the value
>>> that `read-key-sequence` returns.
>>
>> I know. This is the old calling convention of `describe-key`; we
>> should move to the new one.
>
> Is seems everything is covered by this patch:
So this is pushed now.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Thu, 21 Oct 2021 16:44:01 GMT)
Full text and
rfc822 format available.
Message #52 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>>>>> OTOH we should probably try and change the `key` arg to use the new
>>>>> key-list format expected by `describe-key` (i.e. a list of (SEQ
>>>>> . RAW-SEQ) pairs).
>>>>
>>>> `gnus-article-describe-key` just passes down the value
>>>> that `read-key-sequence` returns.
>>>
>>> I know. This is the old calling convention of `describe-key`; we
>>> should move to the new one.
>>
>> Is seems everything is covered by this patch:
>
> So this is pushed now.
Thanks, and sorry I didn't get to it earlier. FWIW, it does look good,
Stefan
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Fri, 19 Nov 2021 12:24:07 GMT)
Full text and
rfc822 format available.
bug unarchived.
Request was from
Juri Linkov <juri <at> linkov.net>
to
control <at> debbugs.gnu.org.
(Mon, 29 Nov 2021 18:25:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Mon, 29 Nov 2021 19:00:02 GMT)
Full text and
rfc822 format available.
Message #59 received at 51173 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
>>>>>> OTOH we should probably try and change the `key` arg to use the new
>>>>>> key-list format expected by `describe-key` (i.e. a list of (SEQ
>>>>>> . RAW-SEQ) pairs).
>>>>>
>>>>> `gnus-article-describe-key` just passes down the value
>>>>> that `read-key-sequence` returns.
>>>>
>>>> I know. This is the old calling convention of `describe-key`; we
>>>> should move to the new one.
>>>
>>> Is seems everything is covered by this patch:
>>
>> So this is pushed now.
>
> Thanks, and sorry I didn't get to it earlier. FWIW, it does look good,
Oh, another regression:
C-s ;; isearch-forward
C-h k ;; isearch-describe-key
then typing any key to describe in isearch-mode
leaves isearch in a broken state: it displays the
search prompt, but no isearch indicator on the mode-line.
So something exits isearch. Adding a breakpoint
in isearch-done reveals this backtrace:
isearch-done()
isearch-mouse-leave-buffer()
mouse-minibuffer-check(19)
mouse-set-point(19)
help--analyze-key("\23" [19] nil)
describe-key((("\23" . [19])))
funcall-interactively(describe-key (("\23" . [19])))
isearch-describe-key()
funcall-interactively(isearch-describe-key)
command-execute(isearch-describe-key)
I don't know if more functions using describe-key are broken,
but copying the same code from gnus-article-describe-key
that gives the buffer argument to describe-key fixes this.
I have no idea for a better way to avoid such problems.
More precautions are added to this patch:
when isearch-mouse-commands contains isearch-describe-key,
isearch-mouse-leave-buffer won't leave isearch-mode.
Also to avoid a broken state, isearch-update
should be used only when isearch-mode is active:
[isearch-describe-key.patch (text/x-diff, inline)]
diff --git a/lisp/isearch.el b/lisp/isearch.el
index 0a041b7a1e..043d2c016e 100644
--- a/lisp/isearch.el
+++ b/lisp/isearch.el
@@ -525,15 +525,17 @@ isearch-describe-key
"Display documentation of the function invoked by isearch key."
(interactive)
(let ((display-buffer-overriding-action isearch--display-help-action))
- (call-interactively 'describe-key))
- (isearch-update))
+ (describe-key (list (cons (read-key-sequence nil t)
+ (this-single-command-raw-keys)))
+ (current-buffer)))
+ (when isearch-mode (isearch-update)))
(defun isearch-describe-mode ()
"Display documentation of Isearch mode."
(interactive)
(let ((display-buffer-overriding-action isearch--display-help-action))
(describe-function 'isearch-forward))
- (isearch-update))
+ (when isearch-mode (isearch-update)))
(defalias 'isearch-mode-help 'isearch-describe-mode)
@@ -1498,7 +1500,7 @@ isearch-done
(and (not edit) isearch-recursive-edit (exit-recursive-edit)))
-(defvar isearch-mouse-commands '(mouse-minor-mode-menu)
+(defvar isearch-mouse-commands '(mouse-minor-mode-menu isearch-describe-key)
"List of mouse commands that are allowed during Isearch.")
(defun isearch-mouse-leave-buffer ()
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Mon, 29 Nov 2021 20:59:02 GMT)
Full text and
rfc822 format available.
Message #62 received at 51173 <at> debbugs.gnu.org (full text, mbox):
> mouse-minibuffer-check(19)
> mouse-set-point(19)
> help--analyze-key("\23" [19] nil)
Hpw 'bout the patch below?
Or maybe the use of `mouse-minibuffer-check` in `mouse-set-point` should
be moved to its interactive spec?
Stefan
diff --git a/lisp/help.el b/lisp/help.el
index 1917ef425d..e175421d02 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -715,7 +715,8 @@ help--analyze-key
(and (not (windowp (posn-window (event-start event))))
(not (framep (posn-window (event-start event))))))
(key-binding key t)
- (save-excursion (mouse-set-point event) (key-binding key t)))))
+ (save-excursion (posn-set-point (event-end event))
+ (key-binding key t)))))
;; Handle the case where we faked an entry in "Select and Paste" menu.
(when (and (eq defn nil)
(stringp (aref key (1- (length key))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Tue, 30 Nov 2021 03:27:02 GMT)
Full text and
rfc822 format available.
Message #65 received at 51173 <at> debbugs.gnu.org (full text, mbox):
> Cc: 51173 <at> debbugs.gnu.org
> Date: Mon, 29 Nov 2021 15:57:51 -0500
> From: Stefan Monnier via "Bug reports for GNU Emacs,
> the Swiss army knife of text editors" <bug-gnu-emacs <at> gnu.org>
>
> > mouse-minibuffer-check(19)
> > mouse-set-point(19)
> > help--analyze-key("\23" [19] nil)
>
> Hpw 'bout the patch below?
>
> Or maybe the use of `mouse-minibuffer-check` in `mouse-set-point` should
> be moved to its interactive spec?
If you want this fixed on the release branch, I'd prefer a localized
change in Gnus (with a more thorough change on master).
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Tue, 30 Nov 2021 09:17:01 GMT)
Full text and
rfc822 format available.
Message #68 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> mouse-minibuffer-check(19)
>> mouse-set-point(19)
>> help--analyze-key("\23" [19] nil)
>
> Hpw 'bout the patch below?
I confirm that the patch fixes this regression,
while not breaking other cases.
Maybe this patch also obsoletes this code in the same place:
;; Clicks on the menu bar produce "event" that
;; is just '(menu-bar)', for which
;; `mouse-set-point' is not useful.
(and (not (windowp (posn-window (event-start event))))
(not (framep (posn-window (event-start event)))))
While testing with this code removed, clicks on the menu bar still work
without errors.
> Or maybe the use of `mouse-minibuffer-check` in `mouse-set-point` should
> be moved to its interactive spec?
I agree with Eli that more cardinal changes could be tried only in master.
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Tue, 30 Nov 2021 13:07:01 GMT)
Full text and
rfc822 format available.
Message #71 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> > mouse-minibuffer-check(19)
>> > mouse-set-point(19)
>> > help--analyze-key("\23" [19] nil)
>>
>> Hpw 'bout the patch below?
>>
>> Or maybe the use of `mouse-minibuffer-check` in `mouse-set-point` should
>> be moved to its interactive spec?
>
> If you want this fixed on the release branch, I'd prefer a localized
> change in Gnus (with a more thorough change on master).
Indeed my patch is not intended for `master` (nor is my suggestion to
move the `mouse-minibuffer-check` call to the interactive spec of
`mouse-set-point`).
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org, bugs <at> gnus.org:
bug#51173; Package
emacs,gnus.
(Wed, 01 Dec 2021 17:37:01 GMT)
Full text and
rfc822 format available.
Message #74 received at 51173 <at> debbugs.gnu.org (full text, mbox):
>> Hpw 'bout the patch below?
>
> I confirm that the patch fixes this regression,
> while not breaking other cases.
>
> Maybe this patch also obsoletes this code in the same place:
>
> ;; Clicks on the menu bar produce "event" that
> ;; is just '(menu-bar)', for which
> ;; `mouse-set-point' is not useful.
> (and (not (windowp (posn-window (event-start event))))
> (not (framep (posn-window (event-start event)))))
>
> While testing with this code removed, clicks on the menu bar still work
> without errors.
Now the fix was pushed to emacs-28.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Thu, 30 Dec 2021 12:24:13 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 118 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.