Package: emacs;
Reported by: Madhu <enometh <at> meer.net>
Date: Mon, 3 Mar 2025 03:07:02 UTC
Severity: normal
Found in version 31.0.50
Fixed in version 31.1
Done: Sean Whitton <spwhitton <at> spwhitton.name>
Bug is archived. No further changes may be made.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
From: Madhu <enometh <at> meer.net> To: bug-gnu-emacs <at> gnu.org Subject: 31.0.50; find-func.el: incorrect use of minor mode Date: Mon, 03 Mar 2025 08:36:32 +0530 (IST)
[Message part 1 (text/plain, inline)]
I brought up this issue on emacs-devel but the discussion did not proceed beyond https://lists.gnu.org/archive/html/emacs-devel/2025-02/msg01054.html for decades find-func.el had used `find-function-setup-keys' to set up global bindings to functions in find-func.el. Since these are global bindings these did not require a minor mode. The minor-mode was introduced in c3e989ca9d7, "New minor mode find-function-mode replaces find-function-setup-keys" which introduced a keymap via new features. This was objected to on the emacs-devel list by Eshel in +message-id:<m1ed4wfaym.fsf <at> dazzs-mbp.home> (sorry lists.gnu.org namazu apparently websearch by mid does not work or provide a link to the html mailing list) on "Fri, 04 Oct 2024 11:56:17 +0200" and the commit 1094c3f914 "find-function-mode: Define keys at a low precedence level" was introduced to restore "backward compatibility". However this is an incorrect use of define-minor-mode, The use of define-minor-mode is to explicitly use a keymap to shadow global bindings and using it to modify global bindings is a basic category error a thinko and should not be promoted in the code base. as the functions in find-func.el are always available for binding, a minor mode does not make sense, if only to mutate global bindings. global bindings set by the user when the mode is in effect are undone when the mode is removed. Emacs should not be doing this. I believe The original function find-function-setup-keys' introduced by rms explicitly avoided using a minor mode precisely for these reasons. Introducing a minor mode is a conceptual regression. If a local keymap is not desired, the minor mode should be removed and the original function undeprecated. The attached patch however restores the local keymap but sets up global bindings via the original find-function-setup-keys.
[f.patch (text/x-patch, inline)]
From 1279279580f7f15e63da9624cdefa4c842f8b24f Mon Sep 17 00:00:00 2001 From: Madhu <enometh <at> net.meer> Date: Mon, 24 Feb 2025 07:48:54 +0530 Subject: [PATCH] lisp/emacs-lisp/find-func.el: restore local keymap * lisp/emacs-lisp/find-func.el: find-function-mode-map. (restored from commit 1094c3f9147), (find-function-setup-keys), restored from commit c3e989ca9, but modified to operate on global-keymap to maintain compatibility). commit message 1094c9147 uses "low precedence level" instead of "global" --- lisp/emacs-lisp/find-func.el | 60 +++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/lisp/emacs-lisp/find-func.el b/lisp/emacs-lisp/find-func.el index 6b7b5083620..b08b8c41aa2 100644 --- a/lisp/emacs-lisp/find-func.el +++ b/lisp/emacs-lisp/find-func.el @@ -812,36 +812,48 @@ find-variable-at-point (when (and symb (not (equal symb 0))) (find-variable-other-window symb)))) +(defvar-keymap find-function-mode-map + "C-x F" #'find-function + "C-x 4 F" #'find-function-other-window + "C-x 5 F" #'find-function-other-frame + + "C-x K" #'find-function-on-key + "C-x 4 K" #'find-function-on-key-other-window + "C-x 5 K" #'find-function-on-key-other-frame + + "C-x V" #'find-variable + "C-x 4 V" #'find-variable-other-window + "C-x 5 V" #'find-variable-other-frame + + "C-x L" #'find-library + "C-x 4 L" #'find-library-other-window + "C-x 5 L" #'find-library-other-frame) + ;;;###autoload (define-minor-mode find-function-mode "Enable some key bindings for the `find-function' family of functions." - :group 'find-function :version "31.1" :global t :lighter nil - ;; For compatibility with the historical behavior of the old - ;; `find-function-setup-keys', define our bindings at the precedence - ;; level of the global map. - :keymap nil - (pcase-dolist (`(,map ,key ,cmd) - `((,ctl-x-map "F" find-function) - (,ctl-x-4-map "F" find-function-other-window) - (,ctl-x-5-map "F" find-function-other-frame) - (,ctl-x-map "K" find-function-on-key) - (,ctl-x-4-map "K" find-function-on-key-other-window) - (,ctl-x-5-map "K" find-function-on-key-other-frame) - (,ctl-x-map "V" find-variable) - (,ctl-x-4-map "V" find-variable-other-window) - (,ctl-x-5-map "V" find-variable-other-frame) - (,ctl-x-map "L" find-library) - (,ctl-x-4-map "L" find-library-other-window) - (,ctl-x-5-map "L" find-library-other-frame))) - (if find-function-mode - (keymap-set map key cmd) - (keymap-unset map key t)))) + :global t :lighter nil ; compat. with old `find-function-setup-keys' + :group 'find-function :version "31.1") ;;;###autoload (defun find-function-setup-keys () - "Turn on `find-function-mode', which see." - (find-function-mode 1)) -(make-obsolete 'find-function-setup-keys 'find-function-mode "31.1") + "Define some key bindings for the `find-function' family of functions. +and also define the keys globally" + (find-function-mode 1) + (define-key ctl-x-map "F" 'find-function) + (define-key ctl-x-4-map "F" 'find-function-other-window) + (define-key ctl-x-5-map "F" 'find-function-other-frame) + (define-key ctl-x-map "K" 'find-function-on-key) + (define-key ctl-x-4-map "K" 'find-function-on-key-other-window) + (define-key ctl-x-5-map "K" 'find-function-on-key-other-frame) + (define-key ctl-x-map "V" 'find-variable) + (define-key ctl-x-4-map "V" 'find-variable-other-window) + (define-key ctl-x-5-map "V" 'find-variable-other-frame) + (define-key ctl-x-map "L" 'find-library) + (define-key ctl-x-4-map "L" 'find-library-other-window) + (define-key ctl-x-5-map "L" 'find-library-other-frame)) + +;;(make-obsolete 'find-function-setup-keys 'find-function-mode "31.1") (provide 'find-func) -- 2.46.0.27.gfa3b914457
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.