* bug#58799: Make winner key sequences repeatable in repeat-mode
@ 2022-10-26 15:01 Damien Cassou
2022-10-26 19:52 ` Stefan Kangas
0 siblings, 1 reply; 19+ messages in thread
From: Damien Cassou @ 2022-10-26 15:01 UTC (permalink / raw)
To: 58799
[-- Attachment #1: Type: text/plain, Size: 580 bytes --]
Tags: patch
In GNU Emacs 28.2 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.34, cairo version 1.16.0)
Windowing system distributor 'The X.Org Foundation', version 11.0.12014000
System Description: Fedora Linux 36 (Workstation Edition)
Configured using:
'configure
--prefix=/nix/store/1ihp8r45mw29fdipjmqrm2vk0fvwd86x-emacs-28.2
--disable-build-details --with-modules --with-x-toolkit=gtk3 --with-xft
--with-cairo --with-native-compilation'
--
Damien Cassou
"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-winner-key-sequences-repeatable-in-repeat-mode.patch --]
[-- Type: text/patch, Size: 1001 bytes --]
From 2f26bbad08f71cb4332cd60fa5698a8c18b4abd2 Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Wed, 26 Oct 2022 16:53:23 +0200
Subject: [PATCH] Make winner key sequences repeatable in repeat-mode
* lisp/winner.el (winner-repeat-map): New variable.
(winner-undo): Put 'repeat-map' property with 'winner-repeat-map'.
(winner-redo): Put 'repeat-map' property with 'winner-repeat-map'.
---
lisp/winner.el | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lisp/winner.el b/lisp/winner.el
index 174b698e7b..30b0a6ada8 100644
--- a/lisp/winner.el
+++ b/lisp/winner.el
@@ -328,6 +328,14 @@ winner-mode-map
map)
"Keymap for Winner mode.")
+(defvar-keymap winner-repeat-map
+ :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
+ [left] #'winner-undo
+ [right] #'winner-redo)
+
+(put #'winner-undo 'repeat-map 'winner-repeat-map)
+(put #'winner-redo 'repeat-map 'winner-repeat-map)
+
\f
;;;###autoload
(define-minor-mode winner-mode
--
2.36.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-26 15:01 bug#58799: Make winner key sequences repeatable in repeat-mode Damien Cassou
@ 2022-10-26 19:52 ` Stefan Kangas
2022-10-27 9:04 ` Robert Pluim
2022-10-30 6:39 ` Damien Cassou
0 siblings, 2 replies; 19+ messages in thread
From: Stefan Kangas @ 2022-10-26 19:52 UTC (permalink / raw)
To: Damien Cassou, 58799
Damien Cassou <damien@cassou.me> writes:
> From 2f26bbad08f71cb4332cd60fa5698a8c18b4abd2 Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien@cassou.me>
> Date: Wed, 26 Oct 2022 16:53:23 +0200
> Subject: [PATCH] Make winner key sequences repeatable in repeat-mode
>
> * lisp/winner.el (winner-repeat-map): New variable.
> (winner-undo): Put 'repeat-map' property with 'winner-repeat-map'.
> (winner-redo): Put 'repeat-map' property with 'winner-repeat-map'.
There is no need to repeat the same text twice, so you can delete the
first occurrence.
> ---
> lisp/winner.el | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/lisp/winner.el b/lisp/winner.el
> index 174b698e7b..30b0a6ada8 100644
> --- a/lisp/winner.el
> +++ b/lisp/winner.el
> @@ -328,6 +328,14 @@ winner-mode-map
> map)
> "Keymap for Winner mode.")
>
> +(defvar-keymap winner-repeat-map
> + :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
> + [left] #'winner-undo
> + [right] #'winner-redo)
It should be "<right>" and "<left>" here, I think.
> +
> +(put #'winner-undo 'repeat-map 'winner-repeat-map)
> +(put #'winner-redo 'repeat-map 'winner-repeat-map)
> +
> \f
> ;;;###autoload
> (define-minor-mode winner-mode
> --
> 2.36.2
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-26 19:52 ` Stefan Kangas
@ 2022-10-27 9:04 ` Robert Pluim
2022-10-27 10:58 ` Stefan Kangas
2022-10-30 6:39 ` Damien Cassou
1 sibling, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-27 9:04 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, 58799
>>>>> On Wed, 26 Oct 2022 12:52:29 -0700, Stefan Kangas <stefankangas@gmail.com> said:
>> +(defvar-keymap winner-repeat-map
>> + :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
>> + [left] #'winner-undo
>> + [right] #'winner-redo)
Stefan> It should be "<right>" and "<left>" here, I think.
Hmm, thatʼs an interesting one:
(key-valid-p "[right]") => nil
(key-valid-p "<right>") => t
but
(kbd "<right>") => [right]
(keymap-set global-map (kbd "<right>") #ʼignore) => error
:-)
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 9:04 ` Robert Pluim
@ 2022-10-27 10:58 ` Stefan Kangas
2022-10-27 11:39 ` Robert Pluim
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Kangas @ 2022-10-27 10:58 UTC (permalink / raw)
To: Robert Pluim; +Cc: Damien Cassou, 58799
Robert Pluim <rpluim@gmail.com> writes:
> Hmm, thatʼs an interesting one:
>
> (key-valid-p "[right]") => nil
> (key-valid-p "<right>") => t
>
> but
>
> (kbd "<right>") => [right]
> (keymap-set global-map (kbd "<right>") #ʼignore) => error
I don't think I see any contradiction, as `keymap-set' requires its
second argument to be `key-valid-p'. So that last one should just be:
(keymap-set global-map "<right>" #'ignore)
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 10:58 ` Stefan Kangas
@ 2022-10-27 11:39 ` Robert Pluim
2022-10-27 12:31 ` Stefan Kangas
0 siblings, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-27 11:39 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, 58799
>>>>> On Thu, 27 Oct 2022 03:58:38 -0700, Stefan Kangas <stefankangas@gmail.com> said:
Stefan> I don't think I see any contradiction, as `keymap-set' requires its
Stefan> second argument to be `key-valid-p'. So that last one should just be:
Stefan> (keymap-set global-map "<right>" #'ignore)
Now that Iʼve actually run some more tests, I see where the confusion
is coming from. You canʼt actually eval this:
(defvar-keymap winner-repeat-map
:doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
[left] #'winner-undo
[right] #'winner-redo)
or this:
(defvar-keymap winner-repeat-map
:doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
<left> #'winner-undo
<right> #'winner-redo)
(but you can stick them in a .el file)
But you can eval this:
(defvar-keymap winner-repeat-map
:doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
"<left>" #'winner-undo
"<right>" #'winner-redo)
Well, TIL. I donʼt think itʼs going to result in any patches from me
though, unless we want to disallow `[left]' and `[right]' in
`defvar-keymap' :-)
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 11:39 ` Robert Pluim
@ 2022-10-27 12:31 ` Stefan Kangas
2022-10-27 13:38 ` Robert Pluim
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Kangas @ 2022-10-27 12:31 UTC (permalink / raw)
To: Robert Pluim; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
Robert Pluim <rpluim@gmail.com> writes:
> (defvar-keymap winner-repeat-map
> :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
> <left> #'winner-undo
> <right> #'winner-redo)
>
> (but you can stick them in a .el file)
Hmm, right. I think that's a bug though, because when loading such a
.elc file I get:
keymap--check: [left] is not a valid key definition; see ‘key-valid-p’
So we should probably make sure that the `defvar-keymap' macro errors
out.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 12:31 ` Stefan Kangas
@ 2022-10-27 13:38 ` Robert Pluim
2022-10-27 16:10 ` Stefan Kangas
0 siblings, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-27 13:38 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
>>>>> On Thu, 27 Oct 2022 05:31:21 -0700, Stefan Kangas <stefankangas@gmail.com> said:
Stefan> Robert Pluim <rpluim@gmail.com> writes:
>> (defvar-keymap winner-repeat-map
>> :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
>> <left> #'winner-undo
>> <right> #'winner-redo)
>>
>> (but you can stick them in a .el file)
Stefan> Hmm, right. I think that's a bug though, because when loading such a
Stefan> .elc file I get:
Stefan> keymap--check: [left] is not a valid key definition; see ‘key-valid-p’
Stefan> So we should probably make sure that the `defvar-keymap' macro errors
Stefan> out.
Thatʼs easy enough (putting it in `define-keymap' would bring out the
backwards-compatibility police, I think). Probably needs an update to
the `defvar-keymap' docstring as well.
Robert
--
diff --git a/lisp/keymap.el b/lisp/keymap.el
index 107565590c..73a9e657fe 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -581,6 +581,10 @@ defvar-keymap
(setq key (pop defs))
(pop defs)
(when (not (eq key :menu))
+ (when (not (key-valid-p key))
+ (error
+ "Invalid key '%s' in keymap '%s' (see `key-valid-p')"
+ key variable-name))
(if (member key seen-keys)
(error "Duplicate definition for key '%s' in keymap '%s'"
key variable-name)
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 13:38 ` Robert Pluim
@ 2022-10-27 16:10 ` Stefan Kangas
2022-10-27 16:18 ` Robert Pluim
2022-10-28 9:36 ` Robert Pluim
0 siblings, 2 replies; 19+ messages in thread
From: Stefan Kangas @ 2022-10-27 16:10 UTC (permalink / raw)
To: Robert Pluim; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
Robert Pluim <rpluim@gmail.com> writes:
> Stefan> So we should probably make sure that the `defvar-keymap' macro errors
> Stefan> out.
>
> Thatʼs easy enough (putting it in `define-keymap' would bring out the
> backwards-compatibility police, I think). Probably needs an update to
> the `defvar-keymap' docstring as well.
`define-keymap' is new in Emacs 29.1, so I think it will be okay to
change it.
> diff --git a/lisp/keymap.el b/lisp/keymap.el
> index 107565590c..73a9e657fe 100644
> --- a/lisp/keymap.el
> +++ b/lisp/keymap.el
> @@ -581,6 +581,10 @@ defvar-keymap
> (setq key (pop defs))
> (pop defs)
> (when (not (eq key :menu))
> + (when (not (key-valid-p key))
> + (error
> + "Invalid key '%s' in keymap '%s' (see `key-valid-p')"
> + key variable-name))
> (if (member key seen-keys)
> (error "Duplicate definition for key '%s' in keymap '%s'"
> key variable-name)
LGTM.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 16:10 ` Stefan Kangas
@ 2022-10-27 16:18 ` Robert Pluim
2022-10-27 16:33 ` Robert Pluim
2022-10-28 9:36 ` Robert Pluim
1 sibling, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-27 16:18 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
>>>>> On Thu, 27 Oct 2022 09:10:12 -0700, Stefan Kangas <stefankangas@gmail.com> said:
Stefan> Robert Pluim <rpluim@gmail.com> writes:
Stefan> So we should probably make sure that the `defvar-keymap' macro errors
Stefan> out.
>>
>> Thatʼs easy enough (putting it in `define-keymap' would bring out the
>> backwards-compatibility police, I think). Probably needs an update to
>> the `defvar-keymap' docstring as well.
Stefan> `define-keymap' is new in Emacs 29.1, so I think it will be okay to
Stefan> change it.
OK. Itʼs the logical place to put it.
>> diff --git a/lisp/keymap.el b/lisp/keymap.el
>> index 107565590c..73a9e657fe 100644
>> --- a/lisp/keymap.el
>> +++ b/lisp/keymap.el
>> @@ -581,6 +581,10 @@ defvar-keymap
>> (setq key (pop defs))
>> (pop defs)
>> (when (not (eq key :menu))
>> + (when (not (key-valid-p key))
>> + (error
>> + "Invalid key '%s' in keymap '%s' (see `key-valid-p')"
>> + key variable-name))
>> (if (member key seen-keys)
>> (error "Duplicate definition for key '%s' in keymap '%s'"
>> key variable-name)
Stefan> LGTM.
Hmm, in light of the above, maybe the duplicate detection should be
moved to `define-keymap' as well.
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 16:18 ` Robert Pluim
@ 2022-10-27 16:33 ` Robert Pluim
0 siblings, 0 replies; 19+ messages in thread
From: Robert Pluim @ 2022-10-27 16:33 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
>>>>> On Thu, 27 Oct 2022 18:18:37 +0200, Robert Pluim <rpluim@gmail.com> said:
Robert> Hmm, in light of the above, maybe the duplicate detection should be
Robert> moved to `define-keymap' as well.
Except itʼs already there. I really must look at my own previous
commits before commenting :-)
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-27 16:10 ` Stefan Kangas
2022-10-27 16:18 ` Robert Pluim
@ 2022-10-28 9:36 ` Robert Pluim
2022-10-28 15:41 ` Stefan Kangas
1 sibling, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-28 9:36 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
>>>>> On Thu, 27 Oct 2022 09:10:12 -0700, Stefan Kangas <stefankangas@gmail.com> said:
>> diff --git a/lisp/keymap.el b/lisp/keymap.el
>> index 107565590c..73a9e657fe 100644
>> --- a/lisp/keymap.el
>> +++ b/lisp/keymap.el
>> @@ -581,6 +581,10 @@ defvar-keymap
>> (setq key (pop defs))
>> (pop defs)
>> (when (not (eq key :menu))
>> + (when (not (key-valid-p key))
>> + (error
>> + "Invalid key '%s' in keymap '%s' (see `key-valid-p')"
>> + key variable-name))
>> (if (member key seen-keys)
>> (error "Duplicate definition for key '%s' in keymap '%s'"
>> key variable-name)
Stefan> LGTM.
I now think this would be a bad idea, because of the following type of
code:
(defvar-keymap footnote-minor-mode-map
:doc "Keymap used for binding footnote minor mode."
(key-description footnote-prefix) footnote-mode-map)
Thatʼs perfectly valid, but fails with the above patch unless we
1. Check (key-valid-p (eval key))
2. Add an autoload cookie for footnote-minor-mode-map
That seems too high a price to pay for avoiding the original problem.
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-28 9:36 ` Robert Pluim
@ 2022-10-28 15:41 ` Stefan Kangas
2022-10-31 14:25 ` Robert Pluim
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Kangas @ 2022-10-28 15:41 UTC (permalink / raw)
To: Robert Pluim; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
Robert Pluim <rpluim@gmail.com> writes:
> (defvar-keymap footnote-minor-mode-map
> :doc "Keymap used for binding footnote minor mode."
> (key-description footnote-prefix) footnote-mode-map)
>
> Thatʼs perfectly valid, but fails with the above patch unless we
>
> 1. Check (key-valid-p (eval key))
> 2. Add an autoload cookie for footnote-minor-mode-map
>
> That seems too high a price to pay for avoiding the original problem.
Hmm, that's a too high price to pay.
Could we check for some common mistakes without having to say `(eval
key)' though? For example by requiring KEY to be
(or (stringp key) (listp key))
?
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-26 19:52 ` Stefan Kangas
2022-10-27 9:04 ` Robert Pluim
@ 2022-10-30 6:39 ` Damien Cassou
2022-10-30 12:38 ` Stefan Kangas
1 sibling, 1 reply; 19+ messages in thread
From: Damien Cassou @ 2022-10-30 6:39 UTC (permalink / raw)
To: Stefan Kangas, 58799
[-- Attachment #1: Type: text/plain, Size: 808 bytes --]
Hi Stefan,
Stefan Kangas <stefankangas@gmail.com> writes:
> Damien Cassou <damien@cassou.me> writes:
>> * lisp/winner.el (winner-repeat-map): New variable.
>> (winner-undo): Put 'repeat-map' property with 'winner-repeat-map'.
>> (winner-redo): Put 'repeat-map' property with 'winner-repeat-map'.
>
> There is no need to repeat the same text twice, so you can delete the
> first occurrence.
Indeed, fixed.
>> +(defvar-keymap winner-repeat-map
>> + :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
>> + [left] #'winner-undo
>> + [right] #'winner-redo)
>
> It should be "<right>" and "<left>" here, I think.
Fixed.
Thank you very much for your reviews.
--
Damien Cassou
"Success is the ability to go from one failure to another without
losing enthusiasm." --Winston Churchill
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-winner-key-sequences-repeatable-in-repeat-mode.patch --]
[-- Type: text/x-patch, Size: 953 bytes --]
From f23ee8ce89bf6e60d97d738f61e4663fd1e2b3dc Mon Sep 17 00:00:00 2001
From: Damien Cassou <damien@cassou.me>
Date: Wed, 26 Oct 2022 16:53:23 +0200
Subject: [PATCH] Make winner key sequences repeatable in repeat-mode
* lisp/winner.el (winner-repeat-map): New variable.
(winner-undo):
(winner-redo): Put 'repeat-map' property with 'winner-repeat-map'.
---
lisp/winner.el | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/lisp/winner.el b/lisp/winner.el
index 174b698e7b..d2040c7e5d 100644
--- a/lisp/winner.el
+++ b/lisp/winner.el
@@ -328,6 +328,14 @@ winner-mode-map
map)
"Keymap for Winner mode.")
+(defvar-keymap winner-repeat-map
+ :doc "Keymap to repeat winner key sequences. Used in `repeat-mode'."
+ "<left>" #'winner-undo
+ "<right>" #'winner-redo)
+
+(put #'winner-undo 'repeat-map 'winner-repeat-map)
+(put #'winner-redo 'repeat-map 'winner-repeat-map)
+
\f
;;;###autoload
(define-minor-mode winner-mode
--
2.36.2
^ permalink raw reply related [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-30 6:39 ` Damien Cassou
@ 2022-10-30 12:38 ` Stefan Kangas
0 siblings, 0 replies; 19+ messages in thread
From: Stefan Kangas @ 2022-10-30 12:38 UTC (permalink / raw)
To: Damien Cassou, 58799
Damien Cassou <damien@cassou.me> writes:
> From f23ee8ce89bf6e60d97d738f61e4663fd1e2b3dc Mon Sep 17 00:00:00 2001
> From: Damien Cassou <damien@cassou.me>
> Date: Wed, 26 Oct 2022 16:53:23 +0200
> Subject: [PATCH] Make winner key sequences repeatable in repeat-mode
>
> * lisp/winner.el (winner-repeat-map): New variable.
> (winner-undo):
> (winner-redo): Put 'repeat-map' property with 'winner-repeat-map'.
Thanks for the patch. I pushed it with this improved commit message
(also adding the bug number):
Make winner key sequences repeatable in repeat-mode
* lisp/winner.el (winner-repeat-map): New variable.
(winner-undo, winner-redo): Put 'repeat-map' property with
'winner-repeat-map'. (Bug#58799)
I'm leaving the bug open as we are still discussing some improvements to
`defvar-keymap'.
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-28 15:41 ` Stefan Kangas
@ 2022-10-31 14:25 ` Robert Pluim
2022-10-31 14:47 ` Stefan Kangas
0 siblings, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-31 14:25 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
>>>>> On Fri, 28 Oct 2022 08:41:37 -0700, Stefan Kangas <stefankangas@gmail.com> said:
Stefan> Robert Pluim <rpluim@gmail.com> writes:
>> (defvar-keymap footnote-minor-mode-map
>> :doc "Keymap used for binding footnote minor mode."
>> (key-description footnote-prefix) footnote-mode-map)
>>
>> Thatʼs perfectly valid, but fails with the above patch unless we
>>
>> 1. Check (key-valid-p (eval key))
>> 2. Add an autoload cookie for footnote-minor-mode-map
>>
>> That seems too high a price to pay for avoiding the original problem.
Stefan> Hmm, that's a too high price to pay.
Stefan> Could we check for some common mistakes without having to say `(eval
Stefan> key)' though? For example by requiring KEY to be
Stefan> (or (stringp key) (listp key))
People also do things like this:
(defcustom pong-left-key "4"
"Alternate key to press for bat 1 to go up (primary one is [left])."
:type '(restricted-sexp :match-alternatives (stringp vectorp)))
.
(defvar-keymap pong-mode-map
:doc "Modemap for pong-mode."
:name 'pong-mode-map
pong-left-key #'pong-move-left
so Iʼm inclined to either fix it in `define-keymap' only, or just let
the existing compiler macro for keymaps handle it.
In any case, I think we can close this particular bug.
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
* bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-31 14:25 ` Robert Pluim
@ 2022-10-31 14:47 ` Stefan Kangas
2022-10-31 15:09 ` Robert Pluim
0 siblings, 1 reply; 19+ messages in thread
From: Stefan Kangas @ 2022-10-31 14:47 UTC (permalink / raw)
To: Robert Pluim; +Cc: Damien Cassou, Lars Ingebrigtsen, 58799
close 58799 29.1
thanks
Robert Pluim <rpluim@gmail.com> writes:
> In any case, I think we can close this particular bug.
Agreed. Now done.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-31 14:47 ` Stefan Kangas
@ 2022-10-31 15:09 ` Robert Pluim
2022-10-31 15:28 ` Eli Zaretskii
0 siblings, 1 reply; 19+ messages in thread
From: Robert Pluim @ 2022-10-31 15:09 UTC (permalink / raw)
To: Stefan Kangas; +Cc: Lars Ingebrigtsen, emacs-devel
(redirecting to emacs-devel)
>>>>> On Mon, 31 Oct 2022 07:47:36 -0700, Stefan Kangas <stefankangas@gmail.com> said:
Stefan> close 58799 29.1
Stefan> thanks
Stefan> Robert Pluim <rpluim@gmail.com> writes:
>> In any case, I think we can close this particular bug.
Stefan> Agreed. Now done.
So the minimal change I think is this:
diff --git c/lisp/keymap.el i/lisp/keymap.el
index 107565590c..750095fab4 100644
--- c/lisp/keymap.el
+++ i/lisp/keymap.el
@@ -545,6 +545,10 @@ define-keymap
(let ((def (pop definitions)))
(if (eq key :menu)
(easy-menu-define nil keymap "" def)
+ (unless (key-valid-p key)
+ (error
+ "Key '%s' in keymap '%s' is not `key-valid-p'"
+ key keymap))
(if (member key seen-keys)
(error "Duplicate definition for key: %S %s" key keymap)
(push key seen-keys))
which will give a run-time error for things like "[left]" and
'[left]', but thatʼs redundant with the compile time warning:
make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
ELC winner.elc
In toplevel form:
winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"
Eli? Lars? Is this overkill for such a rare case?
Robert
--
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-31 15:09 ` Robert Pluim
@ 2022-10-31 15:28 ` Eli Zaretskii
2022-10-31 15:42 ` Robert Pluim
0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2022-10-31 15:28 UTC (permalink / raw)
To: Robert Pluim; +Cc: stefankangas, larsi, emacs-devel
> From: Robert Pluim <rpluim@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>, emacs-devel@gnu.org
> Date: Mon, 31 Oct 2022 16:09:54 +0100
>
> diff --git c/lisp/keymap.el i/lisp/keymap.el
> index 107565590c..750095fab4 100644
> --- c/lisp/keymap.el
> +++ i/lisp/keymap.el
> @@ -545,6 +545,10 @@ define-keymap
> (let ((def (pop definitions)))
> (if (eq key :menu)
> (easy-menu-define nil keymap "" def)
> + (unless (key-valid-p key)
> + (error
> + "Key '%s' in keymap '%s' is not `key-valid-p'"
> + key keymap))
> (if (member key seen-keys)
> (error "Duplicate definition for key: %S %s" key keymap)
> (push key seen-keys))
>
> which will give a run-time error for things like "[left]" and
> '[left]', but thatʼs redundant with the compile time warning:
>
> make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
> ELC winner.elc
>
> In toplevel form:
> winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"
>
> Eli? Lars? Is this overkill for such a rare case?
I don't know. I guess it depends on how many people will be tripped
by this. The change itself is small enough, I think.
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: bug#58799: Make winner key sequences repeatable in repeat-mode
2022-10-31 15:28 ` Eli Zaretskii
@ 2022-10-31 15:42 ` Robert Pluim
0 siblings, 0 replies; 19+ messages in thread
From: Robert Pluim @ 2022-10-31 15:42 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: stefankangas, larsi, emacs-devel
>>>>> On Mon, 31 Oct 2022 17:28:18 +0200, Eli Zaretskii <eliz@gnu.org> said:
>> make[3]: Entering directory '/home/rpluim/repos/emacs/lispʼ
>> ELC winner.elc
>>
>> In toplevel form:
>> winner.el:314:15: Warning: Invalid ‘kbd’ syntax: "[left]"
>>
>> Eli? Lars? Is this overkill for such a rare case?
Eli> I don't know. I guess it depends on how many people will be tripped
Eli> by this. The change itself is small enough, I think.
0, it turns out, as thereʼs already a runtime check for this case 😀
Motto: test the existing code before proposing changes
Robert
--
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2022-10-31 15:42 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-26 15:01 bug#58799: Make winner key sequences repeatable in repeat-mode Damien Cassou
2022-10-26 19:52 ` Stefan Kangas
2022-10-27 9:04 ` Robert Pluim
2022-10-27 10:58 ` Stefan Kangas
2022-10-27 11:39 ` Robert Pluim
2022-10-27 12:31 ` Stefan Kangas
2022-10-27 13:38 ` Robert Pluim
2022-10-27 16:10 ` Stefan Kangas
2022-10-27 16:18 ` Robert Pluim
2022-10-27 16:33 ` Robert Pluim
2022-10-28 9:36 ` Robert Pluim
2022-10-28 15:41 ` Stefan Kangas
2022-10-31 14:25 ` Robert Pluim
2022-10-31 14:47 ` Stefan Kangas
2022-10-31 15:09 ` Robert Pluim
2022-10-31 15:28 ` Eli Zaretskii
2022-10-31 15:42 ` Robert Pluim
2022-10-30 6:39 ` Damien Cassou
2022-10-30 12:38 ` Stefan Kangas
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.