bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Now that we prefer the key-valid-p format, we should think about what to
do about defcustom as well.  We currently have that:

    (defcustom foo "C-a" "" :type 'key-sequence)

Followed by `M-x customize-option RET foo RET' leads to a customize
buffer where this is displayed as "C - a".  IOW, there is only support
for the legacy format "\C-a".

We can't just change this in a backwards incompatible way, but perhaps
we should document it as deprecated and introduce a new format for the
new type?

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Juri Linkov]

> Now that we prefer the key-valid-p format, we should think about what to
> do about defcustom as well.  We currently have that:
>
>     (defcustom foo "C-a" "" :type 'key-sequence)
>
> Followed by `M-x customize-option RET foo RET' leads to a customize
> buffer where this is displayed as "C - a".  IOW, there is only support
> for the legacy format "\C-a".
>
> We can't just change this in a backwards incompatible way, but perhaps
> we should document it as deprecated and introduce a new format for the
> new type?

Indeed, a new type would be preferable.  How about

  :type 'kbd

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Indeed, a new type would be preferable.  How about
>
>   :type 'kbd

A new type would be good, but we're not using the `kbd' name explicitly
in `key-valid-p' etc, so perhaps we could come up with a different name.
But I have no suggestions.  🤐

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Juri Linkov <juri@linkov.net> writes:
>
>> Indeed, a new type would be preferable.  How about
>>
>>   :type 'kbd
>
> A new type would be good, but we're not using the `kbd' name explicitly
> in `key-valid-p' etc, so perhaps we could come up with a different name.
> But I have no suggestions.  🤐

How about

    :type 'key-binding

?

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> How about
>
>     :type 'key-binding
>
> ?

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Juri Linkov]

> Lars Ingebrigtsen <larsi@gnus.org> writes:
>
>> Juri Linkov <juri@linkov.net> writes:
>>
>>> Indeed, a new type would be preferable.  How about
>>>
>>>   :type 'kbd
>>
>> A new type would be good, but we're not using the `kbd' name explicitly
>> in `key-valid-p' etc, so perhaps we could come up with a different name.
>> But I have no suggestions.  🤐
>
> How about
>
>     :type 'key-binding

Or simply :type 'keys?

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Juri Linkov <juri@linkov.net> writes:

>> How about
>>
>>     :type 'key-binding
>
> Or simply :type 'keys?

It sounds like you are supposed to provide a list to that type, I think?

If we are going short, how about :type 'key?

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> If we are going short, how about :type 'key?
>
> But it's not a key, it's a key sequence.

We'll, we're kinda sloppy in the terminology here, so I think any of
these are fine -- key-binding, keys, key.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> We'll, we're kinda sloppy in the terminology here, so I think any of
> these are fine -- key-binding, keys, key.

FWIW, I find the plural "keys" confusing, but other than that any of the
suggestions proposed so far are fine by me as well.

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Juri Linkov]

>> We'll, we're kinda sloppy in the terminology here, so I think any of
>> these are fine -- key-binding, keys, key.
>
> FWIW, I find the plural "keys" confusing, but other than that any of the
> suggestions proposed so far are fine by me as well.

I agree.  OTOH, I still see no problem with 'kbd', or even 'kbd-sequence'.

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> FWIW, I find the plural "keys" confusing, but other than that any of the
>> suggestions proposed so far are fine by me as well.
>
> I agree.  OTOH, I still see no problem with 'kbd', or even 'kbd-sequence'.

The problem is that we don't use the name "kbd" anywhere in the new
keymap functions, so it'd be confusing to use them in defcustom.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> I still see no problem with 'kbd', or even 'kbd-sequence'.
>
> The problem is that we don't use the name "kbd" anywhere in the new
> keymap functions, so it'd be confusing to use them in defcustom.

I started looking into this but noticed that the docstring for
`key-valid-p' says:

    Say whether KEYS is a valid ‘kbd’ sequence.
    A ‘kbd’ sequence is a string consisting of one and more key
    strokes.  The key strokes are separated by a space character.

I guess that docstring should be updated if we don't want to settle for
the terminology "`kbd' sequence".

However, if we do that the problem is that we then risk having two
definitions of "key sequence": both the old and the new style.

See also (info "(elisp) Key Sequences"), that uses the old style.

---

Relatedly, the `define-keymap' docstring currently says:

    Create a new keymap and define KEY/DEFINITION pairs as key sequences.

Should that say "key bindings" instead of "key sequences"?  Otherwise,
it doesn't seem clear what it does.

---

`keymap-set' says:

    Set key sequence KEY to DEFINITION in KEYMAP.

So here is again the "new" style used as the definition of a key
sequence, which seems to contradict the Elisp manual.

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> I started looking into this but noticed that the docstring for
> `key-valid-p' says:
>
>     Say whether KEYS is a valid ‘kbd’ sequence.
>     A ‘kbd’ sequence is a string consisting of one and more key
>     strokes.  The key strokes are separated by a space character.
>
> I guess that docstring should be updated if we don't want to settle for
> the terminology "`kbd' sequence".

Yes, I forgot to update the doc string when I redid this.

> However, if we do that the problem is that we then risk having two
> definitions of "key sequence": both the old and the new style.
>
> See also (info "(elisp) Key Sequences"), that uses the old style.

Yes, that's confusing.  I meant to have the `key-valid-p' say what it
accepts as a valid key binding, and then the other functions refer to it.

> Relatedly, the `define-keymap' docstring currently says:
>
>     Create a new keymap and define KEY/DEFINITION pairs as key sequences.
>
> Should that say "key bindings" instead of "key sequences"?  Otherwise,
> it doesn't seem clear what it does.

Yes, it should.

> `keymap-set' says:
>
>     Set key sequence KEY to DEFINITION in KEYMAP.
>
> So here is again the "new" style used as the definition of a key
> sequence, which seems to contradict the Elisp manual.

Yes.  Please go ahead and fix up the doc strings.  😀

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

>> However, if we do that the problem is that we then risk having two
>> definitions of "key sequence": both the old and the new style.
>>
>> See also (info "(elisp) Key Sequences"), that uses the old style.
>
> Yes, that's confusing.  I meant to have the `key-valid-p' say what it
> accepts as a valid key binding, and then the other functions refer to it.

Do you mean here "a valid key"?  A key binding AFAIU is a mapping from
"KEY => DEFINITION" as described in `keymap-set-after', and belongs in a
key map.

>> `keymap-set' says:
>>
>>     Set key sequence KEY to DEFINITION in KEYMAP.
>>
>> So here is again the "new" style used as the definition of a key
>> sequence, which seems to contradict the Elisp manual.
>
> Yes.  Please go ahead and fix up the doc strings.  😀

I tried to do that on master (commit 8df3a71c52), please take a look.
I also filed a number of documentation bugs separately while looking
over the  info manual.

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

>> Yes, that's confusing.  I meant to have the `key-valid-p' say what it
>> accepts as a valid key binding, and then the other functions refer to it.
>
> Do you mean here "a valid key"?

Yes.

> I tried to do that on master (commit 8df3a71c52), please take a look.
> I also filed a number of documentation bugs separately while looking
> over the  info manual.

Looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

tags 52523 + patch
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> The problem is that we don't use the name "kbd" anywhere in the new
> keymap functions, so it'd be confusing to use them in defcustom.

Having looked over the documentation to these functions, I actually
think "key" is the least confusing name here.

How does the attached patch look?

[-- Attachment #2: 0001-New-type-key-for-defcustom.patch --]
[-- Type: text/x-diff, Size: 3436 bytes --]

From c433b9c0212c1b6d25dd06ba1b6359d7dedcdedd Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Tue, 28 Dec 2021 02:59:51 +0100
Subject: [PATCH] New :type key for defcustom

As compared to the old type key-sequence that deals with raw key
sequences, this :type conforms to the format used by the new keymap-*
functions.
* lisp/wid-edit.el (key): New widget type.  (Bug#52523)
(widget-key-prompt-value-history): New variable.
(widget-key-validate): New function.
(key-sequence): Doc fix.
* doc/lispref/customize.texi (Simple Types): Document above new type.
---
 doc/lispref/customize.texi |  6 +++++-
 etc/NEWS                   |  5 +++++
 lisp/wid-edit.el           | 23 ++++++++++++++++++++++-
 3 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/doc/lispref/customize.texi b/doc/lispref/customize.texi
index 00287a7212..da5e761198 100644
--- a/doc/lispref/customize.texi
+++ b/doc/lispref/customize.texi
@@ -654,10 +654,14 @@ Simple Types
 specify the text to describe each value in a way that fits the specific
 meaning of the alternative.
 
+@item key
+The value is a valid key according to @kbd{key-valid-p}, and suitable
+for use with, for example @code{keymap-set}.
+
 @item key-sequence
 The value is a key sequence.  The customization buffer shows the key
 sequence using the same syntax as the @kbd{kbd} function.  @xref{Key
-Sequences}.
+Sequences}.  This is a legacy type; use @code{key} instead.
 
 @item coding-system
 The value must be a coding-system name, and you can do completion with
diff --git a/etc/NEWS b/etc/NEWS
index 01e1a8e940..ad9e61480b 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -974,6 +974,11 @@ syntax.
 This is like 'kbd', but only returns vectors instead of a mix of
 vectors and strings.
 
++++
+*** New ':type' for 'defcustom' for keys.
+The new 'key' type can be used for options that should be a valid key
+according to 'key-valid-p'.  The type 'key-sequence' is now obsolete.
+
 +++
 ** New substitution in docstrings and 'substitute-command-keys'.
 Use \\`KEYSEQ' to insert a literal key sequence "KEYSEQ" (for example
diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index a53add7d08..754aa83d0a 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -3459,7 +3459,7 @@ widget-key-sequence-map
     map))
 
 (define-widget 'key-sequence 'restricted-sexp
-  "A key sequence."
+  "A key sequence.  Now obsolete; use the `key' type instead."
   :prompt-value 'widget-field-prompt-value
   :prompt-internal 'widget-symbol-prompt-internal
 ; :prompt-match 'fboundp   ;; What was this good for?  KFS
@@ -3524,6 +3524,27 @@ widget-key-sequence-value-to-external
 	(read-kbd-macro value))
     value))
 
+\f
+(defvar widget-key-prompt-value-history nil
+  "History of input to `widget-key-prompt-value'.")
+
+(define-widget 'key 'editable-field
+  "A key sequence."
+  :prompt-value 'widget-field-prompt-value
+  :match 'key-valid-p
+  :format "%{%t%}: %v"
+  :validate 'widget-key-validate
+  :keymap widget-key-sequence-map
+  :help-echo "C-q: insert KEY, EVENT, or CODE; RET: enter value"
+  :tag "Key")
+
+(defun widget-key-validate (widget)
+  (unless (and (stringp (widget-value widget))
+               (key-valid-p (widget-value widget)))
+    (widget-put widget :error (format "Invalid key: %S"
+                                      (widget-value widget)))
+    widget))
+
 \f
 (define-widget 'sexp 'editable-field
   "An arbitrary Lisp expression."
-- 
2.30.2

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Eli Zaretskii]

> From: Stefan Kangas <stefan@marxist.se>
> Date: Mon, 27 Dec 2021 19:08:42 -0800
> Cc: 52523@debbugs.gnu.org, Juri Linkov <juri@linkov.net>
> 
> Having looked over the documentation to these functions, I actually
> think "key" is the least confusing name here.
> 
> How does the attached patch look?
> 
>  
> +@item key
> +The value is a valid key according to @kbd{key-valid-p}, and suitable
> +for use with, for example @code{keymap-set}.

But the doc string of key-valid-p says:

  Say whether KEYS is a valid ‘kbd’ sequence.
  A ‘kbd’ sequence is a string consisting of one and more key
  strokes.  The key strokes are separated by a space character.

So just referring to key-valid-p is not enough, because it accepts a
more general kind of objects.

>  (define-widget 'key-sequence 'restricted-sexp
> -  "A key sequence."
> +  "A key sequence.  Now obsolete; use the `key' type instead."

Please drop the "Now" part: it doesn't fare well with time.  Instead,
say something like "This is obsolete; ...".

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> But the doc string of key-valid-p says:
>
>   Say whether KEYS is a valid ‘kbd’ sequence.
>   A ‘kbd’ sequence is a string consisting of one and more key
>   strokes.  The key strokes are separated by a space character.

This has been changed now.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Lars Ingebrigtsen]

Stefan Kangas <stefan@marxist.se> writes:

> Having looked over the documentation to these functions, I actually
> think "key" is the least confusing name here.
>
> How does the attached patch look?

Other than the "Now obsolete" also noted by Eli, it looks good to me.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Stefan Kangas]

close 52523 29.1
thanks

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Other than the "Now obsolete" also noted by Eli, it looks good to me.

Fixed that and pushed to master as commit 984391a9dc.

bug#52523: 29.0.50; defcustom :type 'key-sequence not following key-valid-p format

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > Now that we prefer the key-valid-p format, we should think about what to
  > do about defcustom as well.  We currently have that:

  >     (defcustom foo "C-a" "" :type 'key-sequence)

That defcustom seems to use incorrect syntax for its initial value.
Here are two real ones:

    (defcustom outline-minor-mode-prefix "\C-c@"
      "Prefix key to use for Outline commands in Outline minor mode.
    The value of this variable is checked as part of loading Outline mode.
    After that, changing the prefix key requires manipulating keymaps."
      :type 'key-sequence
      :initialize 'custom-initialize-default
      ...)

    (defcustom gud-key-prefix "\C-x\C-a"
      "Prefix of all GUD commands valid in C buffers."
      :type 'key-sequence)

They both use the literal string syntax, rather than our new
user-friendly `kbd' syntax, but the value is a valid key sequence.

  > Followed by `M-x customize-option RET foo RET' leads to a customize
  > buffer where this is displayed as "C - a".  IOW, there is only support
  > for the legacy format "\C-a".

M-x customize-option RET gud-key-prefix RET shows this:

    Hide Gud Key Prefix: Key sequence: C-x C-a
       [ State ]: STANDARD.
       Prefix of all GUD commands valid in C buffers.
    Groups: [Gud]

It seems that all is working correctly, and nothing
needs to be fixed.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)