From f08e6538dca6d9cd1457ba1129afe1e56ee286f4 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Sun, 15 Nov 2020 00:53:32 +0100
Subject: [PATCH] Run menu-item :filter function before showing binding

* lisp/help.el (describe-map): Fix running `menu-item' :filter
functions.  This fixes a mistake in the previous conversion of this
defun from the old C function describe_map.  See the discussion in
Bug#39149.
* test/src/keymap-tests.el
(keymap---get-keyelt/runs-menu-item-filter)
(describe-buffer-bindings/menu-item-filter-show-binding)
(describe-buffer-bindings/menu-item-filter-hide-binding):
New tests.
(keymap-tests--test-menu-item-filter): New defun.
---
 lisp/help.el             |  1 +
 test/src/keymap-tests.el | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/lisp/help.el b/lisp/help.el
index 32ee84b5f92..ac5c2f1311b 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1310,6 +1310,7 @@ TRANSL, PARTIAL, SHADOW, NOMENU, MENTION-SHADOW are as in
                                 ((and mention-shadow (not (eq tem definition)))
                                  (setq this-shadowed t))
                                 (t nil))))
+                    (eq definition (lookup-key tail (vector event) t))
                     (push (list event definition this-shadowed) vect))))
             ((eq (car tail) 'keymap)
              ;; The same keymap might be in the structure twice, if
diff --git a/test/src/keymap-tests.el b/test/src/keymap-tests.el
index e3dd8420d7b..610234c5a13 100644
--- a/test/src/keymap-tests.el
+++ b/test/src/keymap-tests.el
@@ -54,6 +54,16 @@
 (ert-deftest keymap-copy-keymap/is-not-eq ()
   (should-not (eq (copy-keymap help-mode-map) help-mode-map)))
 
+(ert-deftest keymap---get-keyelt/runs-menu-item-filter ()
+  (let* (menu-item-filter-ran
+         (object `(menu-item "2" identity
+                             :filter ,(lambda (cmd)
+                                        (message "foo")
+                                        (setq menu-item-filter-ran t)
+                                        cmd))))
+    (keymap--get-keyelt object t)
+    (should menu-item-filter-ran)))
+
 (ert-deftest keymap-lookup-key ()
   (let ((map (make-keymap)))
     (define-key map [?a] 'foo)
@@ -72,6 +82,26 @@ https://debbugs.gnu.org/39149#31"
   (with-temp-buffer
     (should (eq (describe-buffer-bindings (current-buffer)) nil))))
 
+(defun keymap-tests--test-menu-item-filter (show filter-fun)
+  (unwind-protect
+      (progn
+        (define-key global-map (kbd "C-c C-l r")
+          `(menu-item "2" identity :filter ,filter-fun))
+        (with-temp-buffer
+          (describe-buffer-bindings (current-buffer))
+          (goto-char (point-min))
+          (if (eq show 'show)
+              (should (search-forward "C-c C-l r" nil t))
+            (should-not (search-forward "C-c C-l r" nil t)))))
+    (define-key global-map (kbd "C-c C-l r") nil)
+    (define-key global-map (kbd "C-c C-l") nil)))
+
+(ert-deftest describe-buffer-bindings/menu-item-filter-show-binding ()
+  (keymap-tests--test-menu-item-filter 'show (lambda (cmd) cmd)))
+
+(ert-deftest describe-buffer-bindings/menu-item-filter-hide-binding ()
+  (keymap-tests--test-menu-item-filter 'hide (lambda (_) nil)))
+
 (ert-deftest keymap-store_in_keymap-XFASTINT-on-non-characters ()
   "Check for bug fixed in \"Fix assertion violation in define-key\",
 commit 86c19714b097aa477d339ed99ffb5136c755a046."