* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
[not found] ` <20210217180507.C061220DFB@vcs0.savannah.gnu.org>
@ 2021-02-18 4:39 ` Stefan Monnier
2021-02-18 9:37 ` Juri Linkov
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2021-02-18 4:39 UTC (permalink / raw)
To: Juri Linkov; +Cc: emacs-devel
Hi Jury,
> +(defvar next-error-repeat-map
> + (let ((map (make-sparse-keymap)))
> + (define-key map "n" 'next-error)
> + (define-key map "\M-n" 'next-error)
> + (define-key map "p" 'previous-error)
> + (define-key map "\M-p" 'previous-error)
> + map)
> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
> +(put 'next-error 'repeat-map 'next-error-repeat-map)
> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
Could we avoid this duplication between the map and the
`repeat-map` property?
> + (set-transient-map map)))))))
For example passing a second argument t to `set-transient-map` makes
`set-transient-map` look to see if the key just typed was found in the
map and if so keep the transient alive, which seems to [after an
admittedly cursory check of the code] play the same role as what your
`repeat-map` property does, but without the duplication.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-18 4:39 ` master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515) Stefan Monnier
@ 2021-02-18 9:37 ` Juri Linkov
2021-02-18 15:32 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2021-02-18 9:37 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
>> +(defvar next-error-repeat-map
>> + (let ((map (make-sparse-keymap)))
>> + (define-key map "n" 'next-error)
>> + (define-key map "\M-n" 'next-error)
>> + (define-key map "p" 'previous-error)
>> + (define-key map "\M-p" 'previous-error)
>> + map)
>> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
>> +(put 'next-error 'repeat-map 'next-error-repeat-map)
>> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
>
> Could we avoid this duplication between the map and the
> `repeat-map` property?
>
>> + (set-transient-map map)))))))
>
> For example passing a second argument t to `set-transient-map` makes
> `set-transient-map` look to see if the key just typed was found in the
> map and if so keep the transient alive, which seems to [after an
> admittedly cursory check of the code] play the same role as what your
> `repeat-map` property does, but without the duplication.
The current design relies on post-command-hook that runs after every command.
Using the second argument KEEP-PRED of set-transient-map will add duplication:
the same map will be set by both post-command-hook and KEEP-PRED.
So currently there is no duplication. But is it possible to avoid duplication
when using KEEP-PRED too? Could post-command-hook detect that KEEP-PRED
is in use for the next command, and not to call set-transient-map again?
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-18 9:37 ` Juri Linkov
@ 2021-02-18 15:32 ` Stefan Monnier
2021-02-18 17:29 ` Juri Linkov
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2021-02-18 15:32 UTC (permalink / raw)
To: Juri Linkov; +Cc: emacs-devel
>>> +(defvar next-error-repeat-map
>>> + (let ((map (make-sparse-keymap)))
>>> + (define-key map "n" 'next-error)
>>> + (define-key map "\M-n" 'next-error)
>>> + (define-key map "p" 'previous-error)
>>> + (define-key map "\M-p" 'previous-error)
>>> + map)
>>> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
>>> +(put 'next-error 'repeat-map 'next-error-repeat-map)
>>> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
>>
>> Could we avoid this duplication between the map and the
>> `repeat-map` property?
>>
>>> + (set-transient-map map)))))))
>>
>> For example passing a second argument t to `set-transient-map` makes
>> `set-transient-map` look to see if the key just typed was found in the
>> map and if so keep the transient alive, which seems to [after an
>> admittedly cursory check of the code] play the same role as what your
>> `repeat-map` property does, but without the duplication.
>
> The current design relies on post-command-hook that runs after every command.
> Using the second argument KEEP-PRED of set-transient-map will add duplication:
> the same map will be set by both post-command-hook and KEEP-PRED.
My point was not to say "use `keep-pred`", but to point out that
`keep-pred` found a way to solve the same problem without the duplication.
[ I'd be nice to be able to use `keep-pred` directly since it would
likely make the code simpler, but it may well be that this is
incompatible with the current code's design or with its intended
behavior, or that it would require further changes that would make the
overall code more complex. ]
> So currently there is no duplication.
There is in the above code snippet: the `put` statements are redundant
since the same info is fundamentally already available from
`next-error-repeat-map`.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-18 15:32 ` Stefan Monnier
@ 2021-02-18 17:29 ` Juri Linkov
2021-02-23 22:08 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2021-02-18 17:29 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
>>>> +(defvar next-error-repeat-map
>>>> + (let ((map (make-sparse-keymap)))
>>>> + (define-key map "n" 'next-error)
>>>> + (define-key map "\M-n" 'next-error)
>>>> + (define-key map "p" 'previous-error)
>>>> + (define-key map "\M-p" 'previous-error)
>>>> + map)
>>>> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
>>>> +(put 'next-error 'repeat-map 'next-error-repeat-map)
>>>> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
>>>
>>> Could we avoid this duplication between the map and the
>>> `repeat-map` property?
>
> There is in the above code snippet: the `put` statements are redundant
> since the same info is fundamentally already available from
> `next-error-repeat-map`.
Now I see where is duplication. But currently I have no idea
in what keymaps post-command-hook should look for keybindings.
When a command is associated with a keymap `next-error-repeat-map`
via `put` property, then post-command-hook knows where to look
for keybindings for the current command (using `this-command`).
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-18 17:29 ` Juri Linkov
@ 2021-02-23 22:08 ` Stefan Monnier
2021-02-24 18:45 ` Juri Linkov
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2021-02-23 22:08 UTC (permalink / raw)
To: Juri Linkov; +Cc: emacs-devel
>>>>> +(defvar next-error-repeat-map
>>>>> + (let ((map (make-sparse-keymap)))
>>>>> + (define-key map "n" 'next-error)
>>>>> + (define-key map "\M-n" 'next-error)
>>>>> + (define-key map "p" 'previous-error)
>>>>> + (define-key map "\M-p" 'previous-error)
>>>>> + map)
>>>>> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
>>>>> +(put 'next-error 'repeat-map 'next-error-repeat-map)
>>>>> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
>>>>
>>>> Could we avoid this duplication between the map and the
>>>> `repeat-map` property?
>>
>> There is in the above code snippet: the `put` statements are redundant
>> since the same info is fundamentally already available from
>> `next-error-repeat-map`.
>
> Now I see where is duplication. But currently I have no idea
> in what keymaps post-command-hook should look for keybindings.
I don't see the problem: since your code installs the keymap, it can
also arrange to stash the info of which keymap to look up.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-23 22:08 ` Stefan Monnier
@ 2021-02-24 18:45 ` Juri Linkov
2021-02-24 18:58 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2021-02-24 18:45 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
>>>>>> +(defvar next-error-repeat-map
>>>>>> + (let ((map (make-sparse-keymap)))
>>>>>> + (define-key map "n" 'next-error)
>>>>>> + (define-key map "\M-n" 'next-error)
>>>>>> + (define-key map "p" 'previous-error)
>>>>>> + (define-key map "\M-p" 'previous-error)
>>>>>> + map)
>>>>>> + "Keymap to repeat next-error key sequences. Used in `repeat-mode'.")
>>>>>> +(put 'next-error 'repeat-map 'next-error-repeat-map)
>>>>>> +(put 'previous-error 'repeat-map 'next-error-repeat-map)
>>>>>
>>>>> Could we avoid this duplication between the map and the
>>>>> `repeat-map` property?
>>>
>>> There is in the above code snippet: the `put` statements are redundant
>>> since the same info is fundamentally already available from
>>> `next-error-repeat-map`.
>>
>> Now I see where is duplication. But currently I have no idea
>> in what keymaps post-command-hook should look for keybindings.
>
> I don't see the problem: since your code installs the keymap, it can
> also arrange to stash the info of which keymap to look up.
In
(put 'next-error 'repeat-map 'next-error-repeat-map)
(put 'previous-error 'repeat-map 'next-error-repeat-map)
both next-error and previous-error are entry points to
a repeatable key sequence, i.e. both can start a key sequence:
M-g n n p ...
M-g p p n ...
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-24 18:45 ` Juri Linkov
@ 2021-02-24 18:58 ` Stefan Monnier
2021-02-24 19:44 ` Juri Linkov
0 siblings, 1 reply; 9+ messages in thread
From: Stefan Monnier @ 2021-02-24 18:58 UTC (permalink / raw)
To: Juri Linkov; +Cc: emacs-devel
>> I don't see the problem: since your code installs the keymap, it can
>> also arrange to stash the info of which keymap to look up.
>
> In
>
> (put 'next-error 'repeat-map 'next-error-repeat-map)
> (put 'previous-error 'repeat-map 'next-error-repeat-map)
>
> both next-error and previous-error are entry points to
> a repeatable key sequence, i.e. both can start a key sequence:
>
> M-g n n p ...
> M-g p p n ...
I don't understand why you think what I said is related to what you say.
Some code somewhere installs a transient keymap. *That* code can do
some extra work to stash that keymap somewhere else as well so you can
look it up from there instead of having to check the `repeat-map` property.
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-24 18:58 ` Stefan Monnier
@ 2021-02-24 19:44 ` Juri Linkov
2021-02-24 20:53 ` Stefan Monnier
0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2021-02-24 19:44 UTC (permalink / raw)
To: Stefan Monnier; +Cc: emacs-devel
>>> I don't see the problem: since your code installs the keymap, it can
>>> also arrange to stash the info of which keymap to look up.
>>
>> In
>>
>> (put 'next-error 'repeat-map 'next-error-repeat-map)
>> (put 'previous-error 'repeat-map 'next-error-repeat-map)
>>
>> both next-error and previous-error are entry points to
>> a repeatable key sequence, i.e. both can start a key sequence:
>>
>> M-g n n p ...
>> M-g p p n ...
>
> I don't understand why you think what I said is related to what you say.
> Some code somewhere installs a transient keymap. *That* code can do
> some extra work to stash that keymap somewhere else as well so you can
> look it up from there instead of having to check the `repeat-map` property.
The key word is "somewhere". The first somewhere that installs
a transient keymap in post-command-hook needs to get that keymap.
Currently it gets it from the symbol property of this-command.
The second somewhere is the place where it's already stashed by
set-transient-map in overriding-terminal-local-map. That keymap
could be checked, but there is no need since it would be easier
just to use the KEEP-PRED arg.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515)
2021-02-24 19:44 ` Juri Linkov
@ 2021-02-24 20:53 ` Stefan Monnier
0 siblings, 0 replies; 9+ messages in thread
From: Stefan Monnier @ 2021-02-24 20:53 UTC (permalink / raw)
To: Juri Linkov; +Cc: emacs-devel
>> I don't understand why you think what I said is related to what you say.
>> Some code somewhere installs a transient keymap. *That* code can do
>> some extra work to stash that keymap somewhere else as well so you can
>> look it up from there instead of having to check the `repeat-map` property.
>
> The key word is "somewhere". The first somewhere that installs
> a transient keymap in post-command-hook needs to get that keymap.
> Currently it gets it from the symbol property of this-command.
Aahhh.... now I understand.
So the `repeat-map` property is actually not redundant: it records which
transient keymap to use after that command and that keymap could
conceivably not include that command. Also, not all commands in that
keymap should trigger the transient use of that keymap.
I understand quickly but only after a long explanation.
Sorry 'bout that,
Stefan
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-02-24 20:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20210217180506.28704.14280@vcs0.savannah.gnu.org>
[not found] ` <20210217180507.C061220DFB@vcs0.savannah.gnu.org>
2021-02-18 4:39 ` master 12409c9: New transient mode 'repeat-mode' to allow shorter key sequences (bug#46515) Stefan Monnier
2021-02-18 9:37 ` Juri Linkov
2021-02-18 15:32 ` Stefan Monnier
2021-02-18 17:29 ` Juri Linkov
2021-02-23 22:08 ` Stefan Monnier
2021-02-24 18:45 ` Juri Linkov
2021-02-24 18:58 ` Stefan Monnier
2021-02-24 19:44 ` Juri Linkov
2021-02-24 20:53 ` Stefan Monnier
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).