Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

[Eshel Yaron]

Hi,

Sean Whitton <spwhitton@spwhitton.name> writes:

> branch: master
> commit c3e989ca9d786e001a9801fdd95fcbcb0d73fddc
> Author: Sean Whitton <spwhitton@spwhitton.name>
> Commit: Sean Whitton <spwhitton@spwhitton.name>
>
>     New minor mode find-function-mode replaces find-function-setup-keys
>     
>     * lisp/emacs-lisp/find-func.el (find-function-mode-map):
>     (find-function-mode): New minor mode.
>     (find-function-setup-keys): Replace with stub function that just
>     enables the new minor mode.  Mark as obsolete.

[...]

> +(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."
> +  :global t :lighter nil   ; compat. with old `find-function-setup-keys'
> +  :group 'find-function :version "31.1")
> +
>  ;;;###autoload
>  (defun find-function-setup-keys ()
> -  "Define some key bindings for the `find-function' family of functions."
> -  (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))
> +  "Turn on `find-function-mode', which see."
> +  (find-function-mode 1))
> +(make-obsolete 'find-function-setup-keys 'find-function-mode "31.1")
>  
>  (provide 'find-func)

Hmm this changes the precedence of these bindings from low (global map)
to very high (minor mode map).  So it's actually quite a breaking change
for those of us that use find-function-setup-keys and override these
bindings in certain major mode maps :/

How about keeping the behavior of find-function-setup-keys as it was?
We can keep that minor mode too, of course.


Best,

Eshel

Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

[Sean Whitton]

Hello,

On Fri 04 Oct 2024 at 11:56am +02, Eshel Yaron wrote:

> Hmm this changes the precedence of these bindings from low (global map)
> to very high (minor mode map).  So it's actually quite a breaking change
> for those of us that use find-function-setup-keys and override these
> bindings in certain major mode maps :/
>
> How about keeping the behavior of find-function-setup-keys as it was?
> We can keep that minor mode too, of course.

Ah, that was not intentional.  Thanks for pointing this out.

One option is just to keep find-function-setup-keys as it was.
Another would be to make the minor mode add the keys to the global map,
and remove them from it when deactivated.  How about that?

-- 
Sean Whitton

Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

[Eshel Yaron]

Sean Whitton <spwhitton@spwhitton.name> writes:

> On Fri 04 Oct 2024 at 11:56am +02, Eshel Yaron wrote:
>
>> Hmm this changes the precedence of these bindings from low (global map)
>> to very high (minor mode map).  So it's actually quite a breaking change
>> for those of us that use find-function-setup-keys and override these
>> bindings in certain major mode maps :/
>>
>> How about keeping the behavior of find-function-setup-keys as it was?
>> We can keep that minor mode too, of course.
>
> Ah, that was not intentional.  Thanks for pointing this out.
>
> One option is just to keep find-function-setup-keys as it was.
> Another would be to make the minor mode add the keys to the global map,
> and remove them from it when deactivated.  How about that?

The first option might be slightly simpler, but either way works for my
use case, I think.

Thanks,

Eshel

Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

[Sean Whitton]

Hello,

On Fri 04 Oct 2024 at 01:57pm +02, Eshel Yaron wrote:

> Sean Whitton <spwhitton@spwhitton.name> writes:
>
>> On Fri 04 Oct 2024 at 11:56am +02, Eshel Yaron wrote:
>>
>>> Hmm this changes the precedence of these bindings from low (global map)
>>> to very high (minor mode map).  So it's actually quite a breaking change
>>> for those of us that use find-function-setup-keys and override these
>>> bindings in certain major mode maps :/
>>>
>>> How about keeping the behavior of find-function-setup-keys as it was?
>>> We can keep that minor mode too, of course.
>>
>> Ah, that was not intentional.  Thanks for pointing this out.
>>
>> One option is just to keep find-function-setup-keys as it was.
>> Another would be to make the minor mode add the keys to the global map,
>> and remove them from it when deactivated.  How about that?
>
> The first option might be slightly simpler, but either way works for my
> use case, I think.

I prefer the second, so installed that.

-- 
Sean Whitton

Re: master c3e989ca9d7: New minor mode find-function-mode replaces find-function-setup-keys

[Howard Melman]

Sean Whitton <spwhitton@spwhitton.name> writes:

> One option is just to keep find-function-setup-keys as it
> was.  Another would be to make the minor mode add the keys
> to the global map, and remove them from it when
> deactivated.  How about that?

I never understood why these commands were not just bound
like many other commands.  They've been around for 25+ years
and AFAIK nothing else is competing with the bindings.

If you want to bind these (useful) commands, both
find-function-setup-keys and a minor mode require
the find-function library to be loaded from the user's init,
slowing down startup. (I duplicate the code from
find-function-setup-keys in my init to avoid this).

If there are users that bind these keys to other commands
their customizations would be unaffected by having these
commands bound.

Now that other-window-prefix and other-frame-prefix exist,
even binding the base commands to C-x [FKVL] would help.
Those that would prefer to type C-x 4 F instead of C-x 4 4
C-x F can bind it themselves (though I'd prefer if all of
these were bound by default).

-- 

Howard