Subject: Re: bug#51173: 28.0.60; gnus-article-describe-key doesn't work

From: Juri Linkov <[email-masked]>
Date: Mon, 18 Oct 2021 19:20:00 +0300

>> 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:

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))))