unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
       [not found]               ` <875ytn8ufp.fsf@mail.linkov.net>
@ 2021-10-24 14:20                 ` Robert Pluim
  2021-10-24 19:12                   ` Juri Linkov
  0 siblings, 1 reply; 17+ messages in thread
From: Robert Pluim @ 2021-10-24 14:20 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, Drew Adams, emacs-devel

>>>>> On Sat, 23 Oct 2021 21:31:06 +0300, Juri Linkov <juri@linkov.net> said:

    >> Something like this for master? emacs-28 would be nice, but this is
    >> not a bugfix.
    >> 
    >> +            (princ (format-message " `%s' (bound to %s)\n" command
    >> +                                   (mapconcat
    >> +                                    (lambda (key)
    >> +                                      (format "'%s'" (key-description key)))
    >> + (where-is-internal command (symbol-value (car keymap)))
    >> +                                    " ,"))))

    Juri> Thanks, I tried this, but it shows global keybindings,
    Juri> not shorter repeatable keybindings from the repeat-map.
    Juri> For example,

    Juri>   ‘undo-repeat-map’ keymap is repeatable by these commands:
    Juri>    ‘undo’ (bound to 'u' ,'C-_' ,'<undo>' ,'C-/' ,'C-x u' ,'<menu-bar> <edit> <undo>')

    Juri> I expected that it will show only 'u' from ‘undo-repeat-map’.

Since somebody has liberally used 'defalias' in tab-bar.el, it becomes
this :-)

diff --git a/lisp/repeat.el b/lisp/repeat.el
index ac08952eaa..931615ed4c 100644
--- a/lisp/repeat.el
+++ b/lisp/repeat.el
@@ -516,7 +516,12 @@ describe-repeat-maps
           (princ (format-message "`%s' keymap is repeatable by these commands:\n"
                                  (car keymap)))
           (dolist (command (sort (cdr keymap) 'string-lessp))
-            (princ (format-message " `%s'\n" command)))
+            (let* ((info (help-fns--analyze-function command))
+                   (map (list (symbol-value (car keymap))))
+                   (desc (key-description
+                          (or (where-is-internal command map t)
+                              (where-is-internal (nth 3 info) map t)))))
+              (princ (format-message " `%s' (bound to '%s')\n" command desc))))
           (princ "\n"))))))
 
 (provide 'repeat)

Note that this doesnʼt find the bindings for 'O' in
other-window-repeat-map nor 'M' in tab-bar-move-repeat map, because
the commands theyʼre bound to donʼt have the 'repeat-map' property,
which we might want to fix.

Juri> PS: the long lines could be refilled, but this is a minor problem.

The lines should be short enough with the above.

Robert
-- 



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-24 14:20                 ` bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request Robert Pluim
@ 2021-10-24 19:12                   ` Juri Linkov
  2021-10-24 20:02                     ` Robert Pluim
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2021-10-24 19:12 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Ergus, Drew Adams, emacs-devel

>     >> Something like this for master? emacs-28 would be nice, but this is
>     >> not a bugfix.
>     >>
>     >> +            (princ (format-message " `%s' (bound to %s)\n" command
>     >> +                                   (mapconcat
>     >> +                                    (lambda (key)
>     >> +                                      (format "'%s'" (key-description key)))
>     >> + (where-is-internal command (symbol-value (car keymap)))
>     >> +                                    " ,"))))
>
>     Juri> Thanks, I tried this, but it shows global keybindings,
>     Juri> not shorter repeatable keybindings from the repeat-map.
>     Juri> For example,
>
>     Juri>   ‘undo-repeat-map’ keymap is repeatable by these commands:
>     Juri>    ‘undo’ (bound to 'u' ,'C-_' ,'<undo>' ,'C-/' ,'C-x u' ,'<menu-bar> <edit> <undo>')
>
>     Juri> I expected that it will show only 'u' from ‘undo-repeat-map’.
>
> Since somebody has liberally used 'defalias' in tab-bar.el, it becomes
> this :-)

I fixed aliases for tab-bar.el some time ago, but was busy with fixing
other problems in repeat.el, so sorry for not pushing the fixes earlier.
Now I'll push them together with the unfinished test suite for repeat.el  :-)

> diff --git a/lisp/repeat.el b/lisp/repeat.el
> index ac08952eaa..931615ed4c 100644
> --- a/lisp/repeat.el
> +++ b/lisp/repeat.el
> @@ -516,7 +516,12 @@ describe-repeat-maps
>            (princ (format-message "`%s' keymap is repeatable by these commands:\n"
>                                   (car keymap)))
>            (dolist (command (sort (cdr keymap) 'string-lessp))
> -            (princ (format-message " `%s'\n" command)))
> +            (let* ((info (help-fns--analyze-function command))
> +                   (map (list (symbol-value (car keymap))))
> +                   (desc (key-description
> +                          (or (where-is-internal command map t)
> +                              (where-is-internal (nth 3 info) map t)))))
> +              (princ (format-message " `%s' (bound to '%s')\n" command desc))))
>            (princ "\n"))))))

Thanks, I tried this out, and everything looks nice.
I could push this in the next few days after trying to solve
remaining problems:

> Note that this doesnʼt find the bindings for 'O' in
> other-window-repeat-map nor 'M' in tab-bar-move-repeat map, because
> the commands theyʼre bound to donʼt have the 'repeat-map' property,
> which we might want to fix.

Maybe it would be sufficient just to say that a key is bound to a lambda?

> Juri> PS: the long lines could be refilled, but this is a minor problem.
>
> The lines should be short enough with the above.

I agree, a long repeat-map is too unusual.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-24 19:12                   ` Juri Linkov
@ 2021-10-24 20:02                     ` Robert Pluim
  2021-10-25  7:41                       ` Juri Linkov
  0 siblings, 1 reply; 17+ messages in thread
From: Robert Pluim @ 2021-10-24 20:02 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, Drew Adams, emacs-devel

>>>>> On Sun, 24 Oct 2021 22:12:55 +0300, Juri Linkov <juri@linkov.net> said:
    Juri> I fixed aliases for tab-bar.el some time ago, but was busy with fixing
    Juri> other problems in repeat.el, so sorry for not pushing the fixes earlier.
    Juri> Now I'll push them together with the unfinished test suite for repeat.el  :-)

    >> diff --git a/lisp/repeat.el b/lisp/repeat.el
    >> index ac08952eaa..931615ed4c 100644
    >> --- a/lisp/repeat.el
    >> +++ b/lisp/repeat.el
    >> @@ -516,7 +516,12 @@ describe-repeat-maps
    >> (princ (format-message "`%s' keymap is repeatable by these commands:\n"
    >> (car keymap)))
    >> (dolist (command (sort (cdr keymap) 'string-lessp))
    >> -            (princ (format-message " `%s'\n" command)))
    >> +            (let* ((info (help-fns--analyze-function command))
    >> +                   (map (list (symbol-value (car keymap))))
    >> +                   (desc (key-description
    >> +                          (or (where-is-internal command map t)
    >> +                              (where-is-internal (nth 3 info) map t)))))
    >> +              (princ (format-message " `%s' (bound to '%s')\n" command desc))))
    >> (princ "\n"))))))

    Juri> Thanks, I tried this out, and everything looks nice.
    Juri> I could push this in the next few days after trying to solve
    Juri> remaining problems:

Thereʼs no rush

    >> Note that this doesnʼt find the bindings for 'O' in
    >> other-window-repeat-map nor 'M' in tab-bar-move-repeat map, because
    >> the commands theyʼre bound to donʼt have the 'repeat-map' property,
    >> which we might want to fix.

    Juri> Maybe it would be sufficient just to say that a key is bound to a lambda?

Iʼd rather have a real function name to refer to than a lambda, but
itʼs up to you, I guess.

Robert
-- 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-24 20:02                     ` Robert Pluim
@ 2021-10-25  7:41                       ` Juri Linkov
  2021-10-25  8:55                         ` Robert Pluim
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2021-10-25  7:41 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Ergus, Drew Adams, emacs-devel

>     >> Note that this doesnʼt find the bindings for 'O' in
>     >> other-window-repeat-map nor 'M' in tab-bar-move-repeat map, because
>     >> the commands theyʼre bound to donʼt have the 'repeat-map' property,
>     >> which we might want to fix.
>
>     Juri> Maybe it would be sufficient just to say that a key is bound to a lambda?
>
> Iʼd rather have a real function name to refer to than a lambda, but
> itʼs up to you, I guess.

But this means adding a new command with a name e.g. 'other-window-backward'
that will cycle windows in the opposite direction.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25  7:41                       ` Juri Linkov
@ 2021-10-25  8:55                         ` Robert Pluim
  2021-10-25  9:21                           ` Stefan Kangas
  2021-10-25 17:58                           ` Juri Linkov
  0 siblings, 2 replies; 17+ messages in thread
From: Robert Pluim @ 2021-10-25  8:55 UTC (permalink / raw)
  To: Juri Linkov; +Cc: Ergus, Drew Adams, emacs-devel

>>>>> On Mon, 25 Oct 2021 10:41:44 +0300, Juri Linkov <juri@linkov.net> said:

    >> >> Note that this doesnʼt find the bindings for 'O' in
    >> >> other-window-repeat-map nor 'M' in tab-bar-move-repeat map, because
    >> >> the commands theyʼre bound to donʼt have the 'repeat-map' property,
    >> >> which we might want to fix.
    >> 
    Juri> Maybe it would be sufficient just to say that a key is
    >> bound to a lambda?
    >> 
    >> Iʼd rather have a real function name to refer to than a lambda, but
    >> itʼs up to you, I guess.

    Juri> But this means adding a new command with a name e.g. 'other-window-backward'
    Juri> that will cycle windows in the opposite direction.

As I said: itʼs up to you.

BTW, I tried using 'substitute-command-keys' to do this, since itʼs
more elegant, but it gave an unusual result for undo:

    (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
    =>#("C-x u" 0 5
      (font-lock-face help-key-binding face help-key-binding))

which is the global binding, not the repeat-map binding. Any ideas?

Robert
-- 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25  8:55                         ` Robert Pluim
@ 2021-10-25  9:21                           ` Stefan Kangas
  2021-10-25  9:59                             ` Robert Pluim
  2021-10-25 10:14                             ` Andreas Schwab
  2021-10-25 17:58                           ` Juri Linkov
  1 sibling, 2 replies; 17+ messages in thread
From: Stefan Kangas @ 2021-10-25  9:21 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Ergus, emacs-devel, Drew Adams, Juri Linkov

Robert Pluim <rpluim@gmail.com> writes:

> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
> more elegant, but it gave an unusual result for undo:
>
>     (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
>     =>#("C-x u" 0 5
>       (font-lock-face help-key-binding face help-key-binding))

That looks like a bug to me.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25  9:21                           ` Stefan Kangas
@ 2021-10-25  9:59                             ` Robert Pluim
  2021-10-25 10:14                             ` Andreas Schwab
  1 sibling, 0 replies; 17+ messages in thread
From: Robert Pluim @ 2021-10-25  9:59 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Ergus, emacs-devel, Drew Adams, Juri Linkov

>>>>> On Mon, 25 Oct 2021 11:21:50 +0200, Stefan Kangas <stefan@marxist.se> said:

    Stefan> Robert Pluim <rpluim@gmail.com> writes:
    >> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
    >> more elegant, but it gave an unusual result for undo:
    >> 
    >> (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
    >> =>#("C-x u" 0 5
    >> (font-lock-face help-key-binding face help-key-binding))

    Stefan> That looks like a bug to me.

Reported as bug#51384

Robert
-- 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25  9:21                           ` Stefan Kangas
  2021-10-25  9:59                             ` Robert Pluim
@ 2021-10-25 10:14                             ` Andreas Schwab
  2021-10-25 17:54                               ` Juri Linkov
  1 sibling, 1 reply; 17+ messages in thread
From: Andreas Schwab @ 2021-10-25 10:14 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Robert Pluim, Juri Linkov, Ergus, Drew Adams, emacs-devel

On Okt 25 2021, Stefan Kangas wrote:

> Robert Pluim <rpluim@gmail.com> writes:
>
>> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
>> more elegant, but it gave an unusual result for undo:
>>
>>     (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
>>     =>#("C-x u" 0 5
>>       (font-lock-face help-key-binding face help-key-binding))
>
> That looks like a bug to me.

This is due to :advertised-binding.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25 10:14                             ` Andreas Schwab
@ 2021-10-25 17:54                               ` Juri Linkov
  2021-10-25 18:08                                 ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2021-10-25 17:54 UTC (permalink / raw)
  To: Andreas Schwab
  Cc: Robert Pluim, Stefan Kangas, Ergus, Drew Adams, emacs-devel

>>> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
>>> more elegant, but it gave an unusual result for undo:
>>>
>>>     (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
>>>     =>#("C-x u" 0 5
>>>       (font-lock-face help-key-binding face help-key-binding))
>>
>> That looks like a bug to me.
>
> This is due to :advertised-binding.

Maybe 'where-is-internal' should check if the map contains
the advertised binding, only then return it?



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25  8:55                         ` Robert Pluim
  2021-10-25  9:21                           ` Stefan Kangas
@ 2021-10-25 17:58                           ` Juri Linkov
  1 sibling, 0 replies; 17+ messages in thread
From: Juri Linkov @ 2021-10-25 17:58 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Ergus, Drew Adams, emacs-devel

>> But this means adding a new command with a name e.g.
>> 'other-window-backward' that will cycle windows in the opposite direction.
>
> As I said: itʼs up to you.

I think this command could be added only when someone will request it as
a standalone command useful on its own, without connection to repeat-mode.



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25 17:54                               ` Juri Linkov
@ 2021-10-25 18:08                                 ` Stefan Monnier
  2021-10-25 19:59                                   ` Robert Pluim
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2021-10-25 18:08 UTC (permalink / raw)
  To: Juri Linkov
  Cc: Andreas Schwab, Robert Pluim, Stefan Kangas, Ergus, Drew Adams,
	emacs-devel

Juri Linkov [2021-10-25 20:54:03] wrote:
>>>> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
>>>> more elegant, but it gave an unusual result for undo:
>>>>
>>>>     (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
>>>>     =>#("C-x u" 0 5
>>>>       (font-lock-face help-key-binding face help-key-binding))
>>>
>>> That looks like a bug to me.
>>
>> This is due to :advertised-binding.
>
> Maybe 'where-is-internal' should check if the map contains
> the advertised binding, only then return it?

The \\<...> doesn't override all other maps, so the `undo` command is
still found to be bound to `C-x u` in the global map.

But I agree that maybe `where-is-internal` could be told here to give
precedence to bindings found in the \\<...> map.


        Stefan




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25 18:08                                 ` Stefan Monnier
@ 2021-10-25 19:59                                   ` Robert Pluim
  2021-10-25 20:57                                     ` Stefan Monnier
  0 siblings, 1 reply; 17+ messages in thread
From: Robert Pluim @ 2021-10-25 19:59 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Ergus, Stefan Kangas, emacs-devel, Andreas Schwab, Juri Linkov,
	Drew Adams

>>>>> On Mon, 25 Oct 2021 14:08:42 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    Stefan> Juri Linkov [2021-10-25 20:54:03] wrote:
    >>>>> BTW, I tried using 'substitute-command-keys' to do this, since itʼs
    >>>>> more elegant, but it gave an unusual result for undo:
    >>>>> 
    >>>>> (substitute-command-keys "\\<undo-repeat-map>\\[undo]")
    >>>>> =>#("C-x u" 0 5
    >>>>> (font-lock-face help-key-binding face help-key-binding))
    >>>> 
    >>>> That looks like a bug to me.
    >>> 
    >>> This is due to :advertised-binding.
    >> 
    >> Maybe 'where-is-internal' should check if the map contains
    >> the advertised binding, only then return it?

    Stefan> The \\<...> doesn't override all other maps, so the `undo` command is
    Stefan> still found to be bound to `C-x u` in the global map.

Right. I think it should look *only* in the specified map, otherwise
what's the point of specifying it?

    Stefan> But I agree that maybe `where-is-internal` could be told here to give
    Stefan> precedence to bindings found in the \\<...> map.

`where-is-internal' is not the issue. If you pass it (list keymap) it
will look only in 'keymap'. But substitute-command-keys passes it
'keymap', which allows it to look in the global map as well.

Perhaps something like this?

diff --git a/lisp/help.el b/lisp/help.el
index 7e2e492a36..38ecde7f8f 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1124,7 +1124,9 @@ substitute-command-keys
                 (delete-char 2)
                 (let* ((fun (intern (buffer-substring (point) (1- end-point))))
                        (key (with-current-buffer orig-buf
-                              (where-is-internal fun keymap t))))
+                              (or
+                               (where-is-internal fun (list keymap) t)
+                               (where-is-internal fun (list global-map) t)))))
                   ;; If this a command remap, we need to follow it.
                   (when (and (vectorp key)
                              (> (length key) 1)
@@ -1132,7 +1134,9 @@ substitute-command-keys
                              (symbolp (aref key 1)))
                     (setq fun (aref key 1))
                     (setq key (with-current-buffer orig-buf
-                                (where-is-internal fun keymap t))))
+                                (or
+                                 (where-is-internal fun (list keymap) t)
+                                 (where-is-internal fun (list global-map) t)))))
                   (if (not key)
                       ;; Function is not on any key.
                       (let ((op (point)))


Otherwise, extending 'substitute-command-keys' to allow '\(map)' to
mean "look only in map" is fairly trivial as well, but I donʼt see why
weʼd want to add more syntax when we could fix the existing one.

Robert
-- 



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25 19:59                                   ` Robert Pluim
@ 2021-10-25 20:57                                     ` Stefan Monnier
  2021-10-26 10:06                                       ` Robert Pluim
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2021-10-25 20:57 UTC (permalink / raw)
  To: Robert Pluim
  Cc: Juri Linkov, Andreas Schwab, Stefan Kangas, Ergus, Drew Adams,
	emacs-devel

>     Stefan> The \\<...> doesn't override all other maps, so the `undo` command is
>     Stefan> still found to be bound to `C-x u` in the global map.
> Right. I think it should look *only* in the specified map, otherwise
> what's the point of specifying it?

The common case is to specify the keymap that will likely be active when
the command is used.

Maybe you're right that we shouldn't look elsewhere, but I'd be
surprised if there aren't docstrings that rely on the current behavior.

>     Stefan> But I agree that maybe `where-is-internal` could be told
>     Stefan> here to give precedence to bindings found in the
>     Stefan> \\<...> map.
>
> `where-is-internal' is not the issue. If you pass it (list keymap) it
> will look only in 'keymap'. But substitute-command-keys passes it
> 'keymap', which allows it to look in the global map as well.
>
> Perhaps something like this?

Sounds about right, tho it disregards other keymaps than `keymap` and
`global-map`.  Maybe we should do

    (or (where-is-internal fun (list keymap) t)
        (where-is-internal fun nil t))

instead to avoid this problem.


        Stefan




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-25 20:57                                     ` Stefan Monnier
@ 2021-10-26 10:06                                       ` Robert Pluim
  2021-10-26 10:25                                         ` Robert Pluim
  2021-10-26 12:39                                         ` Stefan Monnier
  0 siblings, 2 replies; 17+ messages in thread
From: Robert Pluim @ 2021-10-26 10:06 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Ergus, Stefan Kangas, emacs-devel, Andreas Schwab, Juri Linkov,
	Drew Adams

>>>>> On Mon, 25 Oct 2021 16:57:19 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    Stefan> The \\<...> doesn't override all other maps, so the `undo` command is
    Stefan> still found to be bound to `C-x u` in the global map.
    >> Right. I think it should look *only* in the specified map, otherwise
    >> what's the point of specifying it?

    Stefan> The common case is to specify the keymap that will likely be active when
    Stefan> the command is used.

OK, so I guess that pleads for a fallback to the current behaviour.

    Stefan> Maybe you're right that we shouldn't look elsewhere, but I'd be
    Stefan> surprised if there aren't docstrings that rely on the current behavior.

That may be true, and I canʼt think of an easy way of finding them.

    Stefan> But I agree that maybe `where-is-internal` could be told
    Stefan> here to give precedence to bindings found in the
    Stefan> \\<...> map.
    >> 
    >> `where-is-internal' is not the issue. If you pass it (list keymap) it
    >> will look only in 'keymap'. But substitute-command-keys passes it
    >> 'keymap', which allows it to look in the global map as well.
    >> 
    >> Perhaps something like this?

    Stefan> Sounds about right, tho it disregards other keymaps than `keymap` and
    Stefan> `global-map`.  Maybe we should do

    Stefan>     (or (where-is-internal fun (list keymap) t)
    Stefan>         (where-is-internal fun nil t))

    Stefan> instead to avoid this problem.

I think the minimal change from the current behaviour would be this,
since 'keymap' starts out as 'overriding-local-map', but then can get
set to 'nil' or a specific keymap based on the contents of the string.

Or we could add an optional argument to 'substitute-command-keys' to
mean 'only look in MAPVAR'.

diff --git a/lisp/help.el b/lisp/help.el
index 9666ef9805..55e58e20e5 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1124,7 +1124,9 @@ substitute-command-keys
                 (delete-char 2)
                 (let* ((fun (intern (buffer-substring (point) (1- end-point))))
                        (key (with-current-buffer orig-buf
-                              (where-is-internal fun keymap t))))
+                              (or
+                               (where-is-internal fun (list keymap) t)
+                               (where-is-internal fun keymap t)))))
                   ;; If this a command remap, we need to follow it.
                   (when (and (vectorp key)
                              (> (length key) 1)
@@ -1132,7 +1134,9 @@ substitute-command-keys
                              (symbolp (aref key 1)))
                     (setq fun (aref key 1))
                     (setq key (with-current-buffer orig-buf
-                                (where-is-internal fun keymap t))))
+                                (or
+                                 (where-is-internal fun (list keymap) t)
+                                 (where-is-internal fun keymap t)))))
                   (if (not key)
                       ;; Function is not on any key.
                       (let ((op (point)))

Robert
-- 



^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-26 10:06                                       ` Robert Pluim
@ 2021-10-26 10:25                                         ` Robert Pluim
  2021-10-26 12:39                                         ` Stefan Monnier
  1 sibling, 0 replies; 17+ messages in thread
From: Robert Pluim @ 2021-10-26 10:25 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Ergus, Stefan Kangas, Juri Linkov, Andreas Schwab, emacs-devel,
	Drew Adams

>>>>> On Tue, 26 Oct 2021 12:06:28 +0200, Robert Pluim <rpluim@gmail.com> said:

    Robert> I think the minimal change from the current behaviour would be this,
    Robert> since 'keymap' starts out as 'overriding-local-map', but then can get
    Robert> set to 'nil' or a specific keymap based on the contents of the string.

Actually, no. Thatʼs a good way to break your Emacs. Back to the
drawing board.

Robert
-- 



^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-26 10:06                                       ` Robert Pluim
  2021-10-26 10:25                                         ` Robert Pluim
@ 2021-10-26 12:39                                         ` Stefan Monnier
  2021-10-26 12:46                                           ` Robert Pluim
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Monnier @ 2021-10-26 12:39 UTC (permalink / raw)
  To: Robert Pluim
  Cc: Juri Linkov, Andreas Schwab, Stefan Kangas, Ergus, Drew Adams,
	emacs-devel

> -                              (where-is-internal fun keymap t))))
> +                              (or
> +                               (where-is-internal fun (list keymap) t)
> +                               (where-is-internal fun keymap t)))))

I think this is right, except you need to check `(if keymap` before
using `(list keymap)`.


        Stefan "based on your subsequent statement that it breaks Emacs ;-)"




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request
  2021-10-26 12:39                                         ` Stefan Monnier
@ 2021-10-26 12:46                                           ` Robert Pluim
  0 siblings, 0 replies; 17+ messages in thread
From: Robert Pluim @ 2021-10-26 12:46 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Ergus, Stefan Kangas, emacs-devel, Andreas Schwab, Juri Linkov,
	Drew Adams

>>>>> On Tue, 26 Oct 2021 08:39:48 -0400, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> -                              (where-is-internal fun keymap t))))
    >> +                              (or
    >> +                               (where-is-internal fun (list keymap) t)
    >> +                               (where-is-internal fun keymap t)))))

    Stefan> I think this is right, except you need to check `(if keymap` before
    Stefan> using `(list keymap)`.

Yes

    Stefan>         Stefan "based on your subsequent statement that it breaks Emacs ;-)"

Well, only if you byte-compile stuff. Interactively it works fine :-)

Robert
-- 



^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-10-26 12:46 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <87czs53aei.fsf.ref@aol.com>
     [not found] ` <87czs53aei.fsf@aol.com>
     [not found]   ` <87h7hh6o8t.fsf@mail.linkov.net>
     [not found]     ` <87wnqcv25h.fsf@mail.linkov.net>
     [not found]       ` <SA2PR10MB447476D46BE375158C054A77F3029@SA2PR10MB4474.namprd10.prod.outlook.com>
     [not found]         ` <874kdfekr8.fsf@gmail.com>
     [not found]           ` <87r1gj6say.fsf@mail.linkov.net>
     [not found]             ` <87r1cdz72i.fsf@gmail.com>
     [not found]               ` <875ytn8ufp.fsf@mail.linkov.net>
2021-10-24 14:20                 ` bug#49265: [External] : bug#49265: 28.0.50; repeat mode feature request Robert Pluim
2021-10-24 19:12                   ` Juri Linkov
2021-10-24 20:02                     ` Robert Pluim
2021-10-25  7:41                       ` Juri Linkov
2021-10-25  8:55                         ` Robert Pluim
2021-10-25  9:21                           ` Stefan Kangas
2021-10-25  9:59                             ` Robert Pluim
2021-10-25 10:14                             ` Andreas Schwab
2021-10-25 17:54                               ` Juri Linkov
2021-10-25 18:08                                 ` Stefan Monnier
2021-10-25 19:59                                   ` Robert Pluim
2021-10-25 20:57                                     ` Stefan Monnier
2021-10-26 10:06                                       ` Robert Pluim
2021-10-26 10:25                                         ` Robert Pluim
2021-10-26 12:39                                         ` Stefan Monnier
2021-10-26 12:46                                           ` Robert Pluim
2021-10-25 17:58                           ` Juri Linkov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).