bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

In patch f67a9a12b7b0cdd6030cb080a6d6838255789a08, the commands
keymap-local-set and keymap-global-set became less strict for
non-interactive use, which is not the intended the design. The goal is
that the API only accepts strings in a single format. The vector to
string conversion should happen within the interactive form.

There was an old commit by Stefan Monnier, where he relaxed the API, but
Lars made it clear back then that it is better to only accept a single
format for the keymap API, such that guidance for the user is better due
to clear error messages.

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> In patch f67a9a12b7b0cdd6030cb080a6d6838255789a08, the commands
> keymap-local-set and keymap-global-set became less strict for
> non-interactive use, which is not the intended the design. The goal is
> that the API only accepts strings in a single format. The vector to
> string conversion should happen within the interactive form.

I don't see any use for the vector->string conversion to happen in the
interactive spec: I think the most important use cases for accepting
vectors is when these come directly from Lisp, in which case having to
convert them back to the KBD syntax (only to hope `key-parse` will
correctly undo the damage) is just a waste and a hurdle.

> There was an old commit by Stefan Monnier, where he relaxed the API, but
> Lars made it clear back then that it is better to only accept a single
> format for the keymap API, such that guidance for the user is better due
> to clear error messages.

Obviously, I disagree: the vector format is not going anywhere, so it
makes a lot sense to accept it, even though I fully agree that guidance
should never suggest the use of the vector format.


        Stefan

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 1/30/23 22:00, Stefan Monnier wrote:
> I don't see any use for the vector->string conversion to happen in the
> interactive spec: I think the most important use cases for accepting
> vectors is when these come directly from Lisp, in which case having to
> convert them back to the KBD syntax (only to hope `key-parse` will
> correctly undo the damage) is just a waste and a hurdle.

> Obviously, I disagree: the vector format is not going anywhere, so it
> makes a lot sense to accept it, even though I fully agree that guidance
> should never suggest the use of the vector format.

Stefan, I know that you disagree, see the other thread. I am not the one
to debate this with. Please discuss this with Lars, who introduced the
API and intended it to be as strict as it was. Now the API gets relaxed
through the backdoor because of the broken interactive use. This is why
I am objecting.

All I want is that the keymap.el API overall is consistent - currently
it is not. The keymap-local/global-set made parts of the API accept
vectors, key-parse accepts invalid strings and the rest seems to only
accept strictly conforming key strings.

I think what Lars had in mind with a strict string-only API for
keymap.el, since this creates a consistent picture for keymap
definitions. I agree that internally the vectors are not going away and
that is okay, but this does not prohibit creating the more strict
superficial string only API.

Daniel

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors]

> I think what Lars had in mind with a strict string-only API for
> keymap.el, since this creates a consistent picture for keymap
> definitions. I agree that internally the vectors are not going away and
> that is okay, but this does not prohibit creating the more strict
> superficial string only API.

It's still a PITA when you have a vector and you need to pass it to
a function which happens to be usually receiving its data from end-users
and hence passes its inputs through `key-parse`.

Accepting vectors in `key-parse` doesn't prevent the construction of
a consistent picture where end users only need to know about
KBD-style strings.


        Stefan

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 1/30/23 22:45, Stefan Monnier wrote:
>> I think what Lars had in mind with a strict string-only API for
>> keymap.el, since this creates a consistent picture for keymap
>> definitions. I agree that internally the vectors are not going away and
>> that is okay, but this does not prohibit creating the more strict
>> superficial string only API.
> 
> It's still a PITA when you have a vector and you need to pass it to
> a function which happens to be usually receiving its data from end-users
> and hence passes its inputs through `key-parse`.

I tend to agree. Also I recently had to write a few times (keymap-set
map "<remap> <too>" #'long) where I would have preferred to write
(keymap-set map [remap not] #'long). The same with `defvar-keymap' which
also needs this long remap syntax.

> Accepting vectors in `key-parse` doesn't prevent the construction of
> a consistent picture where end users only need to know about
> KBD-style strings.

Yes, but I am no fan of "hiding the truth" in the documentation. It
would not be bad to accept both the strict string syntax and vectors,
and also document that. It would still be consistent, maybe even more so
when considering the whole Emacs where many internal keymap and event
functions operate on vectors.

I hope it is not too late to fix the API for Emacs 29 such that it
becomes consistent. When the API is officially released it is harder to
change it.

All functions should only accept the strict string syntax as defined by
key-valid-p, including the key-parse function. As Stefan argues, the
functions could also accept vectors, but then please - all of them. I
would not mind that and may prefer it in some cases like the remapping
or when you already have another vector anyway.

On the other ahnd I also see the merits of the more restricted string
only API, which would be simpler to explain and one would not have to
hide the truth in the doc string. For internal uses with vectors there
are still define-key and the other APIs.

Daniel

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Lars Ingebrigtsen]

Daniel Mendler <mail@daniel-mendler.de> writes:

> There was an old commit by Stefan Monnier, where he relaxed the API, but
> Lars made it clear back then that it is better to only accept a single
> format for the keymap API, such that guidance for the user is better due
> to clear error messages.

Yes, that was kinda the point of the whole keymap-* thing.  If there's
one clear syntax, it makes many "but will this work, then?" problems
users have go away -- there's a lot of trial and error involved
currently.

In addition, having one single syntax allows people to grep for bindings
etc -- when exploring the source code, they can isearch for `C-c C-k'
(or whatever) exactly and reliably find the command they're looking for
that way (eventually).

So keymap-local-set and keymap-global-set should be fixed to be strict
again, otherwise there's not much point to the entire `keymap-*'
exercise.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

Dropping bug-gnu-emacs, adding emacs-devel

>>>>> On Tue, 31 Jan 2023 04:42:25 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:

    Lars> In addition, having one single syntax allows people to grep for bindings
    Lars> etc -- when exploring the source code, they can isearch for `C-c C-k'
    Lars> (or whatever) exactly and reliably find the command they're looking for
    Lars> that way (eventually).

We have bindings in emacs currently that are Not Compliant™, thatʼs
for sure.

    Lars> So keymap-local-set and keymap-global-set should be fixed to be strict
    Lars> again, otherwise there's not much point to the entire `keymap-*'
    Lars> exercise.

OK. How about this then (why are the `cursor-in-echo-area' shenanigans
necessary? I wonder if thatʼs a bug, since without them we either get
the cursor not showing in the minibuffer for
`read-key-sequence-vector', or we get an extra space displayed by
`read-command')

diff --git i/lisp/keymap.el w/lisp/keymap.el
index caabedd5aec..0c1aa412e35 100644
--- i/lisp/keymap.el
+++ w/lisp/keymap.el
@@ -76,9 +76,14 @@ keymap-global-set
 that local binding will continue to shadow any global binding
 that you make with this function."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key globally:\nCSet key %s globally to command: ")
-  (unless (stringp key)
-    (setq key (key-description key)))
+  (interactive
+   (let* ((menu-prompting nil)
+          (cursor-in-echo-area t)
+          (key (key-description (read-key-sequence-vector "Set key globally:" nil t))))
+     (list key
+           (let ((cursor-in-echo-area nil))
+             (read-command (format "Set key %s to command: "
+                                   key))))))
   (keymap-set (current-global-map) key command))
 
 (defun keymap-local-set (key command)
@@ -91,12 +96,17 @@ keymap-local-set
 The binding goes in the current buffer's local map, which in most
 cases is shared with all other buffers in the same major mode."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key locally:\nCSet key %s locally to command: ")
+  (interactive
+   (let* ((menu-prompting nil)
+          (cursor-in-echo-area t)
+          (key (key-description (read-key-sequence-vector "Set key locally:" nil t))))
+     (list key
+           (let ((cursor-in-echo-area nil))
+             (read-command (format "Set key %s to command: "
+                                   key))))))
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
-    (unless (stringp key)
-      (setq key (key-description key)))
     (keymap-set map key command)))

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stephen Berman]

On Tue, 31 Jan 2023 10:05:15 +0100 Robert Pluim <rpluim@gmail.com> wrote:

> Dropping bug-gnu-emacs, adding emacs-devel
>
>>>>>> On Tue, 31 Jan 2023 04:42:25 +0100, Lars Ingebrigtsen <larsi@gnus.org> said:
>
>     Lars> In addition, having one single syntax allows people to grep for bindings
>     Lars> etc -- when exploring the source code, they can isearch for `C-c C-k'
>     Lars> (or whatever) exactly and reliably find the command they're looking for
>     Lars> that way (eventually).
>
> We have bindings in emacs currently that are Not Compliant™, thatʼs
> for sure.
>
>     Lars> So keymap-local-set and keymap-global-set should be fixed to be strict
>     Lars> again, otherwise there's not much point to the entire `keymap-*'
>     Lars> exercise.
>
> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
> necessary? I wonder if thatʼs a bug, since without them we either get
> the cursor not showing in the minibuffer for
> `read-key-sequence-vector', or we get an extra space displayed by
> `read-command')

TIL cursor-in-echo-area :-).  This looks to me like a good solution
(though aesthetically I would add a space after the colon in the
read-key-sequence-vector prompt, even though the user input doesn't
appear there).

Steve Berman

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

(and redirecting back to bug#61184. EMORECOFFEE this morning)

>>>>> On Tue, 31 Jan 2023 11:08:41 +0100, Stephen Berman <stephen.berman@gmx.net> said:

    >> 
    >> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
    >> necessary? I wonder if thatʼs a bug, since without them we either get
    >> the cursor not showing in the minibuffer for
    >> `read-key-sequence-vector', or we get an extra space displayed by
    >> `read-command')

    Stephen> TIL cursor-in-echo-area :-).  This looks to me like a
    Stephen> good solution

I looked at callint.c to figure out why `read-key-sequence' and
(interactive "K") were different (why *are* they different?)

    Stephen> (though aesthetically I would add a space after the colon in the
    Stephen> read-key-sequence-vector prompt, even though the user input doesn't
    Stephen> appear there).

If you do that you get two spaces between the colon and the cursor.

Robert
-- 

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stephen Berman]

On Tue, 31 Jan 2023 11:23:36 +0100 Robert Pluim <rpluim@gmail.com> wrote:

> (and redirecting back to bug#61184. EMORECOFFEE this morning)
>
>>>>>> On Tue, 31 Jan 2023 11:08:41 +0100, Stephen Berman
> <stephen.berman@gmx.net> said:
>
>     >> 
>     >> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
>     >> necessary? I wonder if thatʼs a bug, since without them we either get
>     >> the cursor not showing in the minibuffer for
>     >> `read-key-sequence-vector', or we get an extra space displayed by
>     >> `read-command')
>
>     Stephen> TIL cursor-in-echo-area :-).  This looks to me like a
>     Stephen> good solution
>
> I looked at callint.c to figure out why `read-key-sequence' and
> (interactive "K") were different

Ah, ok.

>                                  (why *are* they different?)

Good question.

>     Stephen> (though aesthetically I would add a space after the colon in the
>     Stephen> read-key-sequence-vector prompt, even though the user input doesn't
>     Stephen> appear there).
>
> If you do that you get two spaces between the colon and the cursor.

Hm, I see just one space here with Emacs 29 and master (GNU/Linux)...

Steve Berman

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Tue, 31 Jan 2023 11:31:29 +0100, Stephen Berman <stephen.berman@gmx.net> said:

    Stephen> On Tue, 31 Jan 2023 11:23:36 +0100 Robert Pluim <rpluim@gmail.com> wrote:
    >> (and redirecting back to bug#61184. EMORECOFFEE this morning)
    >> 
    >>>>>>> On Tue, 31 Jan 2023 11:08:41 +0100, Stephen Berman
    >> <stephen.berman@gmx.net> said:
    >> 
    >> >> 
    >> >> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
    >> >> necessary? I wonder if thatʼs a bug, since without them we either get
    >> >> the cursor not showing in the minibuffer for
    >> >> `read-key-sequence-vector', or we get an extra space displayed by
    >> >> `read-command')
    >> 
    Stephen> TIL cursor-in-echo-area :-).  This looks to me like a
    Stephen> good solution
    >> 
    >> I looked at callint.c to figure out why `read-key-sequence' and
    >> (interactive "K") were different

    Stephen> Ah, ok.

    >> (why *are* they different?)

    Stephen> Good question.

    Stephen> (though aesthetically I would add a space after the colon in the
    Stephen> read-key-sequence-vector prompt, even though the user input doesn't
    Stephen> appear there).
    >> 
    >> If you do that you get two spaces between the colon and the cursor.

    Stephen> Hm, I see just one space here with Emacs 29 and master (GNU/Linux)...

including in a tty frame? I see one space in gui, two in
tty. Redisplay expert to the white courtesy phone please.

Robert
-- 

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stephen Berman]

On Tue, 31 Jan 2023 11:37:19 +0100 Robert Pluim <rpluim@gmail.com> wrote:

>>>>>> On Tue, 31 Jan 2023 11:31:29 +0100, Stephen Berman
> <stephen.berman@gmx.net> said:
>
>     Stephen> On Tue, 31 Jan 2023 11:23:36 +0100 Robert Pluim
>     Stephen> <rpluim@gmail.com> wrote:
>     >> (and redirecting back to bug#61184. EMORECOFFEE this morning)
>     >> 
>     >>>>>>> On Tue, 31 Jan 2023 11:08:41 +0100, Stephen Berman
>     >> <stephen.berman@gmx.net> said:
>     >> 
>     >> >> 
>     >> >> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
>     >> >> necessary? I wonder if thatʼs a bug, since without them we either get
>     >> >> the cursor not showing in the minibuffer for
>     >> >> `read-key-sequence-vector', or we get an extra space displayed by
>     >> >> `read-command')
>     >> 
>     Stephen> TIL cursor-in-echo-area :-).  This looks to me like a
>     Stephen> good solution
>     >> 
>     >> I looked at callint.c to figure out why `read-key-sequence' and
>     >> (interactive "K") were different
>
>     Stephen> Ah, ok.
>
>     >> (why *are* they different?)
>
>     Stephen> Good question.
>
>     Stephen> (though aesthetically I would add a space after the colon in the
>     Stephen> read-key-sequence-vector prompt, even though the user input doesn't
>     Stephen> appear there).
>     >> 
>     >> If you do that you get two spaces between the colon and the cursor.
>
>     Stephen> Hm, I see just one space here with Emacs 29 and master (GNU/Linux)...
>
> including in a tty frame? I see one space in gui, two in
> tty. Redisplay expert to the white courtesy phone please.

Ah, yes.  I'd only tested with -Q, not with -Q -nw.  I also see two in
the latter.  Yet more questions...

Steve Berman

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Lars Ingebrigtsen <larsi@gnus.org>,  Daniel Mendler
>  <mail@daniel-mendler.de>,  61184@debbugs.gnu.org,  Stefan Monnier
>  <monnier@iro.umontreal.ca>,  Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 31 Jan 2023 11:37:19 +0100
> 
> >>>>> On Tue, 31 Jan 2023 11:31:29 +0100, Stephen Berman <stephen.berman@gmx.net> said:
> 
>     >> (why *are* they different?)
> 
>     Stephen> Good question.
> 
>     Stephen> (though aesthetically I would add a space after the colon in the
>     Stephen> read-key-sequence-vector prompt, even though the user input doesn't
>     Stephen> appear there).
>     >> 
>     >> If you do that you get two spaces between the colon and the cursor.
> 
>     Stephen> Hm, I see just one space here with Emacs 29 and master (GNU/Linux)...
> 
> including in a tty frame? I see one space in gui, two in
> tty. Redisplay expert to the white courtesy phone please.

You called?

That's a bug: two separate code paths that did equivalent jobs --
almost.  Walking glyphs is a tricksy business...

Should be fixed now.

RE: [External] : Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Drew Adams]

> TIL cursor-in-echo-area :-).

TIL "TIL"

Not to be confused with "till".



	

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stefan Monnier]

> --- i/lisp/keymap.el
> +++ w/lisp/keymap.el
> @@ -76,9 +76,14 @@ keymap-global-set
>  that local binding will continue to shadow any global binding
>  that you make with this function."
>    (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
> -  (interactive "KSet key globally:\nCSet key %s globally to command: ")
> -  (unless (stringp key)
> -    (setq key (key-description key)))
> +  (interactive
> +   (let* ((menu-prompting nil)
> +          (cursor-in-echo-area t)
> +          (key (key-description (read-key-sequence-vector "Set key globally:" nil t))))
> +     (list key
> +           (let ((cursor-in-echo-area nil))
> +             (read-command (format "Set key %s to command: "
> +                                   key))))))
>    (keymap-set (current-global-map) key command))
>  
>  (defun keymap-local-set (key command)
> @@ -91,12 +96,17 @@ keymap-local-set
>  The binding goes in the current buffer's local map, which in most
>  cases is shared with all other buffers in the same major mode."
>    (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
> -  (interactive "KSet key locally:\nCSet key %s locally to command: ")
> +  (interactive
> +   (let* ((menu-prompting nil)
> +          (cursor-in-echo-area t)
> +          (key (key-description (read-key-sequence-vector "Set key locally:" nil t))))
> +     (list key
> +           (let ((cursor-in-echo-area nil))
> +             (read-command (format "Set key %s to command: "
> +                                   key))))))

Please put that code in a separate function so as to avoid
this duplication.


        Stefan

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Tue, 31 Jan 2023 09:30:04 -0500, Stefan Monnier <monnier@iro.umontreal.ca> said:

    >> --- i/lisp/keymap.el
    >> +++ w/lisp/keymap.el
    >> @@ -76,9 +76,14 @@ keymap-global-set
    >> that local binding will continue to shadow any global binding
    >> that you make with this function."
    >> (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
    >> -  (interactive "KSet key globally:\nCSet key %s globally to command: ")
    >> -  (unless (stringp key)
    >> -    (setq key (key-description key)))
    >> +  (interactive
    >> +   (let* ((menu-prompting nil)
    >> +          (cursor-in-echo-area t)
    >> +          (key (key-description (read-key-sequence-vector "Set key globally:" nil t))))
    >> +     (list key
    >> +           (let ((cursor-in-echo-area nil))
    >> +             (read-command (format "Set key %s to command: "
    >> +                                   key))))))
    >> (keymap-set (current-global-map) key command))
    >> 
    >> (defun keymap-local-set (key command)
    >> @@ -91,12 +96,17 @@ keymap-local-set
    >> The binding goes in the current buffer's local map, which in most
    >> cases is shared with all other buffers in the same major mode."
    >> (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
    >> -  (interactive "KSet key locally:\nCSet key %s locally to command: ")
    >> +  (interactive
    >> +   (let* ((menu-prompting nil)
    >> +          (cursor-in-echo-area t)
    >> +          (key (key-description (read-key-sequence-vector "Set key locally:" nil t))))
    >> +     (list key
    >> +           (let ((cursor-in-echo-area nil))
    >> +             (read-command (format "Set key %s to command: "
    >> +                                   key))))))

    Stefan> Please put that code in a separate function so as to avoid
    Stefan> this duplication.

MoooooM, Stefan is making me clean up *before* Iʼve finished! 😀

Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Daniel Mendler <mail@daniel-mendler.de>,  emacs-devel@gnu.org,  Stefan
>  Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>
> Date: Tue, 31 Jan 2023 10:05:15 +0100
> 
>     Lars> So keymap-local-set and keymap-global-set should be fixed to be strict
>     Lars> again, otherwise there's not much point to the entire `keymap-*'
>     Lars> exercise.
> 
> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
> necessary? I wonder if thatʼs a bug, since without them we either get
> the cursor not showing in the minibuffer for
> `read-key-sequence-vector', or we get an extra space displayed by
> `read-command')

Why does it have to be so complicated, though?  If the problem is not
to call key-description in non-interactive invocations, can't we call
key-description inside the interactive form?  Or use some other trick
to invoke key-description only in interactive calls?

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Tue, 31 Jan 2023 17:06:21 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Daniel Mendler <mail@daniel-mendler.de>,  emacs-devel@gnu.org,  Stefan
    >> Monnier <monnier@iro.umontreal.ca>, Eli Zaretskii <eliz@gnu.org>
    >> Date: Tue, 31 Jan 2023 10:05:15 +0100
    >> 
    Lars> So keymap-local-set and keymap-global-set should be fixed to be strict
    Lars> again, otherwise there's not much point to the entire `keymap-*'
    Lars> exercise.
    >> 
    >> OK. How about this then (why are the `cursor-in-echo-area' shenanigans
    >> necessary? I wonder if thatʼs a bug, since without them we either get
    >> the cursor not showing in the minibuffer for
    >> `read-key-sequence-vector', or we get an extra space displayed by
    >> `read-command')

    Eli> Why does it have to be so complicated, though?  If the problem is not
    Eli> to call key-description in non-interactive invocations, can't we call
    Eli> key-description inside the interactive form?  Or use some other trick
    Eli> to invoke key-description only in interactive calls?

? Weʼre only calling key-description inside `interactive' in the
patch.

Ideally Iʼd like to use a format string to `interactive', and
massage the results, but I donʼt think thatʼs possible (Iʼd love to be
wrong).

I guess we could use `called-interactively-p', but thatʼs frowned upon,
or add an optional `interactive' arg thatʼs set to `t' by the
`interactive' call, but that all feels messy.

Or we add a new interactive spec: 'Κ' that does the same as 'K' but
calls `key-description' (Iʼm joking)

Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
>   monnier@iro.umontreal.ca
> Date: Tue, 31 Jan 2023 16:48:02 +0100
> 
> >>>>> On Tue, 31 Jan 2023 17:06:21 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> Why does it have to be so complicated, though?  If the problem is not
>     Eli> to call key-description in non-interactive invocations, can't we call
>     Eli> key-description inside the interactive form?  Or use some other trick
>     Eli> to invoke key-description only in interactive calls?
> 
> ? Weʼre only calling key-description inside `interactive' in the
> patch.

I meant _before_ the patch.  The only problem with that code was that
it called key-description fro non-interactive invocations.  Can't we
handle just that minor issue, and leave the rest intact?  If not, why
not?

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Tue, 31 Jan 2023 18:37:35 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
    >> monnier@iro.umontreal.ca
    >> Date: Tue, 31 Jan 2023 16:48:02 +0100
    >> 
    >> >>>>> On Tue, 31 Jan 2023 17:06:21 +0200, Eli Zaretskii <eliz@gnu.org> said:
    >> 
    Eli> Why does it have to be so complicated, though?  If the problem is not
    Eli> to call key-description in non-interactive invocations, can't we call
    Eli> key-description inside the interactive form?  Or use some other trick
    Eli> to invoke key-description only in interactive calls?
    >> 
    >> ? Weʼre only calling key-description inside `interactive' in the
    >> patch.

    Eli> I meant _before_ the patch.  The only problem with that code was that
    Eli> it called key-description fro non-interactive invocations.  Can't we
    Eli> handle just that minor issue, and leave the rest intact?  If not, why
    Eli> not?

Sure, if you tell me how to reliably determine
that. `called-interactively-p' comes with all sorts of dire warnings.

Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
>   monnier@iro.umontreal.ca
> Date: Tue, 31 Jan 2023 17:48:27 +0100
> 
> >>>>> On Tue, 31 Jan 2023 18:37:35 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
>     >> monnier@iro.umontreal.ca
>     >> Date: Tue, 31 Jan 2023 16:48:02 +0100
>     >> 
>     >> >>>>> On Tue, 31 Jan 2023 17:06:21 +0200, Eli Zaretskii <eliz@gnu.org> said:
>     >> 
>     Eli> Why does it have to be so complicated, though?  If the problem is not
>     Eli> to call key-description in non-interactive invocations, can't we call
>     Eli> key-description inside the interactive form?  Or use some other trick
>     Eli> to invoke key-description only in interactive calls?
>     >> 
>     >> ? Weʼre only calling key-description inside `interactive' in the
>     >> patch.
> 
>     Eli> I meant _before_ the patch.  The only problem with that code was that
>     Eli> it called key-description fro non-interactive invocations.  Can't we
>     Eli> handle just that minor issue, and leave the rest intact?  If not, why
>     Eli> not?
> 
> Sure, if you tell me how to reliably determine
> that. `called-interactively-p' comes with all sorts of dire warnings.

What's wrong with the first method described in the node "Distinguish
Interactive" in the ELisp reference?

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Tue, 31 Jan 2023 20:43:09 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> What's wrong with the first method described in the node "Distinguish
    Eli> Interactive" in the ELisp reference?

You mean like this? Itʼs definitely a smaller change.

diff --git i/lisp/keymap.el w/lisp/keymap.el
index de90b03ba64..5897418c7ab 100644
--- i/lisp/keymap.el
+++ w/lisp/keymap.el
@@ -65,7 +65,7 @@ keymap-set
     (setq definition (key-parse definition)))
   (define-key keymap (key-parse key) definition))
 
-(defun keymap-global-set (key command)
+(defun keymap-global-set (key command &optional interactive)
   "Give KEY a global binding as COMMAND.
 COMMAND is the command definition to use; usually it is
 a symbol naming an interactively-callable function.
@@ -76,12 +76,12 @@ keymap-global-set
 that local binding will continue to shadow any global binding
 that you make with this function."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key globally:\nCSet key %s globally to command: ")
-  (unless (stringp key)
+  (interactive "KSet key globally:\nCSet key %s globally to command: \np")
+  (when interactive
     (setq key (key-description key)))
   (keymap-set (current-global-map) key command))
 
-(defun keymap-local-set (key command)
+(defun keymap-local-set (key command &optional interactive)
   "Give KEY a local binding as COMMAND.
 COMMAND is the command definition to use; usually it is
 a symbol naming an interactively-callable function.
@@ -91,11 +91,11 @@ keymap-local-set
 The binding goes in the current buffer's local map, which in most
 cases is shared with all other buffers in the same major mode."
   (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key locally:\nCSet key %s locally to command: ")
+  (interactive "KSet key locally:\nCSet key %s locally to command: \np")
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
-    (unless (stringp key)
+    (when interactive
       (setq key (key-description key)))
     (keymap-set map key command)))


Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
>   monnier@iro.umontreal.ca
> Date: Wed, 01 Feb 2023 13:52:19 +0100
> 
> >>>>> On Tue, 31 Jan 2023 20:43:09 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> What's wrong with the first method described in the node "Distinguish
>     Eli> Interactive" in the ELisp reference?
> 
> You mean like this? Itʼs definitely a smaller change.

Yes, exactly.  Thanks.

Unless anyone else objects, please install this in a day or two.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 14:06, Eli Zaretskii wrote:
>> From: Robert Pluim <rpluim@gmail.com>
>> Cc: larsi@gnus.org,  mail@daniel-mendler.de,  emacs-devel@gnu.org,
>>   monnier@iro.umontreal.ca
>> Date: Wed, 01 Feb 2023 13:52:19 +0100
>>
>>>>>>> On Tue, 31 Jan 2023 20:43:09 +0200, Eli Zaretskii <eliz@gnu.org> said:
>>
>>     Eli> What's wrong with the first method described in the node "Distinguish
>>     Eli> Interactive" in the ELisp reference?
>>
>> You mean like this? Itʼs definitely a smaller change.
> 
> Yes, exactly.  Thanks.
> 
> Unless anyone else objects, please install this in a day or two.

I object. With this change the non-interactive implementation is
polluted with an unnecessary INTERACTIVE argument, which would then
allow the non-interactive caller to still pass vector arguments. You
could as well call the argument ALLOW-VECTOR. If the non-interactive
function gets extended at some point with additional arguments how
should we proceed then? I also argue that the primary use case of these
functions is non-interactive and that should be prioritized.

Why can you not just move the whole conversion business into the
`interactive' form? This means we cannot use a string as interactive
form but we have to implement our own `keymap--read` function which is
then used like this: `(interactive (list (keymap--read ...) ...))`. It
is not as concise as the string form but would avoid any problems.

As better alternative we could also go with Stefan's proposal to allow
vectors as arguments in the first place. This would resolve this issue
cleanly without any extra code.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Wed, 1 Feb 2023 14:13:25 +0100, Daniel Mendler <mail@daniel-mendler.de> said:

    >> Yes, exactly.  Thanks.
    >> 
    >> Unless anyone else objects, please install this in a day or two.

    Daniel> I object. With this change the non-interactive implementation is
    Daniel> polluted with an unnecessary INTERACTIVE argument, which would then
    Daniel> allow the non-interactive caller to still pass vector arguments. You
    Daniel> could as well call the argument ALLOW-VECTOR. If the non-interactive
    Daniel> function gets extended at some point with additional arguments how
    Daniel> should we proceed then? I also argue that the primary use case of these
    Daniel> functions is non-interactive and that should be prioritized.

I use `local-set-key' interactively all the time, so Iʼm not convinced
thatʼs generally true.

    Daniel> Why can you not just move the whole conversion business into the
    Daniel> `interactive' form? This means we cannot use a string as interactive
    Daniel> form but we have to implement our own `keymap--read` function which is
    Daniel> then used like this: `(interactive (list (keymap--read ...) ...))`. It
    Daniel> is not as concise as the string form but would avoid any problems.

Thatʼs basically my previous patch with the repetitive code moved into
a separate function as Stefan suggested. Or we could avoid the extra
arg by using `called-interactively-p'

    Daniel> As better alternative we could also go with Stefan's proposal to allow
    Daniel> vectors as arguments in the first place. This would resolve this issue
    Daniel> cleanly without any extra code.

And this goes against Larsʼ intentions for the new keymap code, so I
donʼt think thatʼs a good idea.

Maybe Iʼll just implement my 'Κ' idea ;-)

Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 14:44, Robert Pluim wrote:
>>>>>> On Wed, 1 Feb 2023 14:13:25 +0100, Daniel Mendler <mail@daniel-mendler.de> said:
> 
>     >> Yes, exactly.  Thanks.
>     >> 
>     >> Unless anyone else objects, please install this in a day or two.
> 
>     Daniel> I object. With this change the non-interactive implementation is
>     Daniel> polluted with an unnecessary INTERACTIVE argument, which would then
>     Daniel> allow the non-interactive caller to still pass vector arguments. You
>     Daniel> could as well call the argument ALLOW-VECTOR. If the non-interactive
>     Daniel> function gets extended at some point with additional arguments how
>     Daniel> should we proceed then? I also argue that the primary use case of these
>     Daniel> functions is non-interactive and that should be prioritized.
> 
> I use `local-set-key' interactively all the time, so Iʼm not convinced
> thatʼs generally true.

I base my argument on the fact that many users modify keybindings in
their user configuration. However this applies more to
`keymap-globl-set' or `global-set-key'. But there is not much point in
arguing about such statistics since we want to have both non-interactive
and interactive use supported.

My point is that in a newly designed keymap API there should not be a
place for such superfluous arguments which are only a leak of the
underlying implementation to support the interactive use case.

>     Daniel> Why can you not just move the whole conversion business into the
>     Daniel> `interactive' form? This means we cannot use a string as interactive
>     Daniel> form but we have to implement our own `keymap--read` function which is
>     Daniel> then used like this: `(interactive (list (keymap--read ...) ...))`. It
>     Daniel> is not as concise as the string form but would avoid any problems.
> 
> Thatʼs basically my previous patch with the repetitive code moved into
> a separate function as Stefan suggested. Or we could avoid the extra
> arg by using `called-interactively-p'

Yes, your patch plus Stefan's improvement proposal seems like a
reasonable solution to me.

>     Daniel> As better alternative we could also go with Stefan's proposal to allow
>     Daniel> vectors as arguments in the first place. This would resolve this issue
>     Daniel> cleanly without any extra code.
> 
> And this goes against Larsʼ intentions for the new keymap code, so I
> donʼt think thatʼs a good idea.

Yes, it does. But I am neutral with respect to that decision. I would be
okay if all keymap functions accepted vectors as I would be if they
don't. But given that we have to jump through hoops to achieve our goal,
maybe relaxing the strictness would indeed be better as Stefan proposed.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> Date: Wed, 1 Feb 2023 14:13:25 +0100
> Cc: larsi@gnus.org, emacs-devel@gnu.org, monnier@iro.umontreal.ca
> From: Daniel Mendler <mail@daniel-mendler.de>
> 
> > Unless anyone else objects, please install this in a day or two.
> 
> I object. With this change the non-interactive implementation is
> polluted with an unnecessary INTERACTIVE argument

It's an optional argument, so I fail to see how is that "pollution".

> which would then
> allow the non-interactive caller to still pass vector arguments.

So?  A malevolent enough programmer could replace
keymap-global/local-set with an implementation that performs the
conversion unconditionally, so the danger of someone hanging
themselves with the rope Emacs gives them always exists.  This
function is supposed to help those who _want_ such problems to be
caught, it isn't supposed to make Emacs a high-security prison,
because that's simply impossible in Emacs.

> Why can you not just move the whole conversion business into the
> `interactive' form? This means we cannot use a string as interactive
> form but we have to implement our own `keymap--read` function which is
> then used like this: `(interactive (list (keymap--read ...) ...))`. It
> is not as concise as the string form but would avoid any problems.

Excuse me, but that's the tail wagging the dog.  Please be reasonable,
we want a change that is simple and safe enough to go into Emacs 29,
because currently those functions are completely useless as
interactive commands, and we want them to become the mainstay of
binding keys interactively.

> As better alternative we could also go with Stefan's proposal to allow
> vectors as arguments in the first place.

Over Lars's disagreement?  I don't want to do that, with all due
respect to Stefan, certainly not on emacs-29.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 14:50, Eli Zaretskii wrote:
> So?  A malevolent enough programmer could replace
> keymap-global/local-set with an implementation that performs the
> conversion unconditionally, so the danger of someone hanging
> themselves with the rope Emacs gives them always exists.  This
> function is supposed to help those who _want_ such problems to be
> caught, it isn't supposed to make Emacs a high-security prison,
> because that's simply impossible in Emacs.

Of course everything is possible. But that's not my point here. The
keymap.el API is a newly designed API, so please let's design it in a
clean way, where we don't have meaningless arguments.

My suggestion is to go with one of these three solutions:

- Use `call-interactively-p'
- Use Stefan's proposal of a separate function `keymap--read' which is
called in the interactive form.
- Use Stefan's other proposal to allow vectors for all keymap functions
consistently. This way we don't have to jump through hoops here.

>> Why can you not just move the whole conversion business into the
>> `interactive' form? This means we cannot use a string as interactive
>> form but we have to implement our own `keymap--read` function which is
>> then used like this: `(interactive (list (keymap--read ...) ...))`. It
>> is not as concise as the string form but would avoid any problems.
> 
> Excuse me, but that's the tail wagging the dog.  Please be reasonable,
> we want a change that is simple and safe enough to go into Emacs 29,
> because currently those functions are completely useless as
> interactive commands, and we want them to become the mainstay of
> binding keys interactively.

I think the patch proposed before was fairly reasonable, and could be
even improved with a separate `keymap--read' function as Stefan
proposed. It is less intrusive than the patch which has been proposed
now with the additional INTERACTIVE argument, which modifies the
interface. Adding an argument is a more intrusive change.

>> As better alternative we could also go with Stefan's proposal to allow
>> vectors as arguments in the first place.
> 
> Over Lars's disagreement?  I don't want to do that, with all due
> respect to Stefan, certainly not on emacs-29.

Okay, that's fine with me. I am fine with both allowing vectors and not
allowing vectors. But I am not fine with making a mess out of an API
which have been designed newly from the ground.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> Date: Wed, 1 Feb 2023 14:57:16 +0100
> Cc: rpluim@gmail.com, larsi@gnus.org, emacs-devel@gnu.org,
>  monnier@iro.umontreal.ca
> From: Daniel Mendler <mail@daniel-mendler.de>
> 
> On 2/1/23 14:50, Eli Zaretskii wrote:
> > So?  A malevolent enough programmer could replace
> > keymap-global/local-set with an implementation that performs the
> > conversion unconditionally, so the danger of someone hanging
> > themselves with the rope Emacs gives them always exists.  This
> > function is supposed to help those who _want_ such problems to be
> > caught, it isn't supposed to make Emacs a high-security prison,
> > because that's simply impossible in Emacs.
> 
> Of course everything is possible. But that's not my point here. The
> keymap.el API is a newly designed API, so please let's design it in a
> clean way, where we don't have meaningless arguments.

The advertised API wouldn't change.  We don't expect anyone to use the
additional argument in non-interactive invocation.  We can use
advertised-calling-convention declaration to hide that argument from
documented interfaces.

I'm also okay with using called-interactively-p, but I thought for
once we should do what we preach.  (And whoever wants to circumvent
called-interactively-p can always use call-interactively anyway.)  But
if people dislike the method that we ourselves document as the
preferred one, I can live with the second best.

> >> Why can you not just move the whole conversion business into the
> >> `interactive' form? This means we cannot use a string as interactive
> >> form but we have to implement our own `keymap--read` function which is
> >> then used like this: `(interactive (list (keymap--read ...) ...))`. It
> >> is not as concise as the string form but would avoid any problems.
> > 
> > Excuse me, but that's the tail wagging the dog.  Please be reasonable,
> > we want a change that is simple and safe enough to go into Emacs 29,
> > because currently those functions are completely useless as
> > interactive commands, and we want them to become the mainstay of
> > binding keys interactively.
> 
> I think the patch proposed before was fairly reasonable, and could be
> even improved with a separate `keymap--read' function as Stefan
> proposed. It is less intrusive than the patch which has been proposed
> now with the additional INTERACTIVE argument, which modifies the
> interface. Adding an argument is a more intrusive change.

Not from my POV.  The patch was complex, was using a different
interfaces (which surely will bring some unintended surprises, like
any read-WHATEVER API used to read input), and relying on obscure
options like cursor-in-echo-area that evidently is not used much in
these cases (or else the bug with cursor positioning on TTY frames
would have been reported long ago).  From where I stand, it's
antithesis of safe changes close to pretest.

> I am not fine with making a mess out of an API which have been
> designed newly from the ground.

We are not messing anything, see above.  These are all accepted,
documented, and recommended techniques.  I get it that you don't like
them, but the documentation clearly indicates that your opinions on
this are not shared by the project.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 18:30, Eli Zaretskii wrote:
>> Of course everything is possible. But that's not my point here. The
>> keymap.el API is a newly designed API, so please let's design it in a
>> clean way, where we don't have meaningless arguments.
> 
> The advertised API wouldn't change.  We don't expect anyone to use the
> additional argument in non-interactive invocation.  We can use
> advertised-calling-convention declaration to hide that argument from
> documented interfaces.

That's good. If the argument is not advertised then the implementation
detail is at least hidden superficially.

> I'm also okay with using called-interactively-p, but I thought for
> once we should do what we preach.  (And whoever wants to circumvent
> called-interactively-p can always use call-interactively anyway.)  But
> if people dislike the method that we ourselves document as the
> preferred one, I can live with the second best.
I think call-interactively-p would be better for this use case, but
that's my opinion versus yours.

>> I am not fine with making a mess out of an API which have been
>> designed newly from the ground.
> 
> We are not messing anything, see above.  These are all accepted,
> documented, and recommended techniques.  I get it that you don't like
> them, but the documentation clearly indicates that your opinions on
> this are not shared by the project.

Actually, I agree that using an interactive argument is an acceptable
approach in some cases. I use this technique myself in some of my
projects published on GNU ELPA, so what you write is factually wrong.
But if there is a rule or recommended technique, there can also be an
exception.

I still believe it is better to not leak implementation details as an
optional argument in this case, even if it is hidden via the advertised
calling convention. Robert mentioned that an additional interactive type
K could be introduced. That might be a good long term solution, but
requires more intrusive changes which are out of question for emacs-29.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stefan Monnier]

> I think call-interactively-p would be better for this use case, but
> that's my opinion versus yours.

Have you ever looked at the implementation of `called-interactively-p`?


        Stefan

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 19:54, Stefan Monnier wrote:
>> I think call-interactively-p would be better for this use case, but
>> that's my opinion versus yours.
> 
> Have you ever looked at the implementation of `called-interactively-p`?

I guess there is no doubt that we all agree that the implementation is
not great. But I would not consider it prohibitively bad, such that it
should never ever be used in new code. If that is the case why is the
function not officially deprecated?

To be clear, I was aware of the recommendation to avoid
`called-interactively-p' and iirc I have not used it in any of my
packages. It is a kludge to use it, but using "hidden arguments" and
"advertised calling convention" seems as much a kludge in my eyes. Is it
commonly agreed upon that advertised calling conventions are the lesser
evil? I've usually understood advertised calling convention as a means
for deprecation - did I misunderstand that? I wonder if
`called-interactively-p' would lead to actual problems in this
particular use case?

The problem with the hidden INTERACTIVE (or ALLOW-VECTOR) argument is
that it is still somewhat exposed to non-interactive callers. For
example, in the context of the Compat library, should this argument be
made available, since non-interactive callers may want to use it? What
if callers start to use it if they want to pass vector arguments?

An example where INTERACTIVE arguments seems more justified to me are
the `completion-at-point-functions' provided by my Cape package. The
Capf `cape-line', which also happens to be a command, completes full
lines in a buffer. If called interactively or with a non-nil interactive
argument, it will use `completion-in-region'. The nice property is that
the equivalence `(call-interactively #'cape-line) == (cape-line t)` holds.

To come back to `keymap-local-set' - the good thing is that we can avoid
both kludges, advertised calling conventions and `call-interactively-p'.
All we have to do is move the key reading entirely to the interactive
form, which also seems to be semantically most correct. If I would rate
the possible solutions:

1. Just allow vector arguments for all the functions. Simplest solution,
least amount of code.

2. Use `(interactive (list (keymap--read-key) ...))`. Unfortunately this
solution lead to technical difficulties in the `keymap--read-key' function.

3. Use the `call-interactively-p' kludge.

4. Use the hidden INTERACTIVE argument kludge.

Unfortunately none of the solutions 1 to 3 seems to be good enough.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Stefan Monnier]

>>> I think call-interactively-p would be better for this use case, but
>>> that's my opinion versus yours.
>> Have you ever looked at the implementation of `called-interactively-p`?
> I guess there is no doubt that we all agree that the implementation is
> not great.

The problem is that it's not the fault of the implementation.
The idea of having it as a function is fundamentally flawed, which is
why the function can be nothing but a mess.

> But I would not consider it prohibitively bad, such that it
> should never ever be used in new code.  If that is the case why is the
> function not officially deprecated?

`called-interactive-p` is in the same category as `advice-add`.
It's great that we have it, but you should only use it when there's no
other way.

> To be clear, I was aware of the recommendation to avoid
> `called-interactively-p' and iirc I have not used it in any of my
> packages.  It is a kludge to use it, but using "hidden arguments" and
> "advertised calling convention" seems as much a kludge in my eyes.

Maybe they're kludges, but at least they have a well defined meaning.
`called-interactively-p` is in a completely different category of kludges.

> The problem with the hidden INTERACTIVE (or ALLOW-VECTOR) argument is
> that it is still somewhat exposed to non-interactive callers. For

Emacs is not in the business of imposing The Right Way, instead we
prefer to encourage it while still allowing people to shoot themselves
in the foot in all kinds of fun ways.


        Stefan

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/1/23 23:42, Stefan Monnier wrote:
>>>> I think call-interactively-p would be better for this use case, but
>>>> that's my opinion versus yours.
>>> Have you ever looked at the implementation of `called-interactively-p`?
>> I guess there is no doubt that we all agree that the implementation is
>> not great.
> 
> The problem is that it's not the fault of the implementation.
> The idea of having it as a function is fundamentally flawed, which is
> why the function can be nothing but a mess.

Yes, I agree.

>> But I would not consider it prohibitively bad, such that it
>> should never ever be used in new code.  If that is the case why is the
>> function not officially deprecated?
> 
> `called-interactive-p` is in the same category as `advice-add`.
> It's great that we have it, but you should only use it when there's no
> other way.

I would go so far to say that `advice-add' is technically much saner
than `called-interactively-p`. I wouldn't want to miss `advice-add'!

>> To be clear, I was aware of the recommendation to avoid
>> `called-interactively-p' and iirc I have not used it in any of my
>> packages.  It is a kludge to use it, but using "hidden arguments" and
>> "advertised calling convention" seems as much a kludge in my eyes.
> 
> Maybe they're kludges, but at least they have a well defined meaning.
> `called-interactively-p` is in a completely different category of kludges.

Well, this is where I disagree. It is a *technically* kludge, but it
will only occur within the innards of the function, and it won't be
exposed on the level of the function calling convention. The advised
function calling convention is a kludge which is exposed to the caller
of the function - hidden or not.

>> The problem with the hidden INTERACTIVE (or ALLOW-VECTOR) argument is
>> that it is still somewhat exposed to non-interactive callers. For
> 
> Emacs is not in the business of imposing The Right Way, instead we
> prefer to encourage it while still allowing people to shoot themselves
> in the foot in all kinds of fun ways.

Tell that to Eli, who seems to be in the business of preaching The Right
Way and also following it. At least that's better than preaching water
and drinking wine.

But that's beside the point - keymap.el is a newly introduced library
with a newly designed API. I would expect that such a library can be
implemented according to the best practices without any kludges -
without `call-interactively-p' and without advertised calling
conventions. That this does not seem to be possible, leaves me
unsatisfied. Well, it would actually be possible, we just have to do it
differently - either allowing vectors as arguments or by using a
different interactive form calling `keymap--read-key`. But these better
solutions were rejected for other reasons.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> Date: Wed, 1 Feb 2023 23:56:12 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, rpluim@gmail.com, larsi@gnus.org,
>  emacs-devel@gnu.org
> From: Daniel Mendler <mail@daniel-mendler.de>
> 
> > Emacs is not in the business of imposing The Right Way, instead we
> > prefer to encourage it while still allowing people to shoot themselves
> > in the foot in all kinds of fun ways.
> 
> Tell that to Eli, who seems to be in the business of preaching The Right
> Way and also following it.

But preaching is very different from imposing.  And I think "do as I
say, not as I do" is not a great stance for us (although we sometimes
do it).

> But that's beside the point - keymap.el is a newly introduced library
> with a newly designed API. I would expect that such a library can be
> implemented according to the best practices without any kludges -
> without `call-interactively-p' and without advertised calling
> conventions. That this does not seem to be possible, leaves me
> unsatisfied. Well, it would actually be possible, we just have to do it
> differently - either allowing vectors as arguments or by using a
> different interactive form calling `keymap--read-key`. But these better
> solutions were rejected for other reasons.

We could go for a different design if we had more time.  These
functions were introduced just 2.5 months ago, and the problems with
them were only brought to our attention very recently.  We don't have
enough time to switch to using other APIs in the interactive form, as
we had bitter experience with doing that in similar cases (subtle bugs
and behavior changes that took many moons to uncover and even longer
to fix).  It's too bad no one tried these new commands earlier and
reported the problems back then, but that's water under the bridge.
So we don't have the luxury of going for the ideal design.  We must
choose one of the safe solutions that will not delay the pretest nor
endanger its quick success and the following release.  I prefer the
additional argument method, and it sounds like so does Stefan.

So, Robert, please install your last patch, and thanks.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/2/23 07:58, Eli Zaretskii wrote:
>>> Emacs is not in the business of imposing The Right Way, instead we
>>> prefer to encourage it while still allowing people to shoot themselves
>>> in the foot in all kinds of fun ways.
>>
>> Tell that to Eli, who seems to be in the business of preaching The Right
>> Way and also following it.
> 
> But preaching is very different from imposing.  And I think "do as I
> say, not as I do" is not a great stance for us (although we sometimes
> do it).

You preach and impose it on the new API. But it is okay.

>> But that's beside the point - keymap.el is a newly introduced library
>> with a newly designed API. I would expect that such a library can be
>> implemented according to the best practices without any kludges -
>> without `call-interactively-p' and without advertised calling
>> conventions. That this does not seem to be possible, leaves me
>> unsatisfied. Well, it would actually be possible, we just have to do it
>> differently - either allowing vectors as arguments or by using a
>> different interactive form calling `keymap--read-key`. But these better
>> solutions were rejected for other reasons.
> 
> We could go for a different design if we had more time.  These
> functions were introduced just 2.5 months ago, and the problems with
> them were only brought to our attention very recently.  We don't have
> enough time to switch to using other APIs in the interactive form, as
> we had bitter experience with doing that in similar cases (subtle bugs
> and behavior changes that took many moons to uncover and even longer
> to fix).  It's too bad no one tried these new commands earlier and
> reported the problems back then, but that's water under the bridge.
> So we don't have the luxury of going for the ideal design.  We must
> choose one of the safe solutions that will not delay the pretest nor
> endanger its quick success and the following release.  I prefer the
> additional argument method, and it sounds like so does Stefan.

I agree that we should be careful for emacs-29. I think you convinced me
that the additional (non-advertised) argument isn't worse than
`call-interactively-p'. I would still prefer a solution without either
of those kludges (or maybe even just allowing vector arguments to avoid
the complications), but I've repeated that often enough. Maybe when the
API is overhauled the next time.

Thanks for the consideration and thanks for addressing this issue in the
first place.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Thu, 02 Feb 2023 08:58:19 +0200, Eli Zaretskii <eliz@gnu.org> said:

    Eli> So, Robert, please install your last patch, and thanks.

With `advertised-calling-convention', perhaps?

diff --git a/lisp/keymap.el b/lisp/keymap.el
index de90b03ba64..7008bac8b24 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -65,7 +65,7 @@ keymap-set
     (setq definition (key-parse definition)))
   (define-key keymap (key-parse key) definition))
 
-(defun keymap-global-set (key command)
+(defun keymap-global-set (key command &optional interactive)
   "Give KEY a global binding as COMMAND.
 COMMAND is the command definition to use; usually it is
 a symbol naming an interactively-callable function.
@@ -75,13 +75,14 @@ keymap-global-set
 Note that if KEY has a local binding in the current buffer,
 that local binding will continue to shadow any global binding
 that you make with this function."
-  (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key globally:\nCSet key %s globally to command: ")
-  (unless (stringp key)
+  (declare (compiler-macro (lambda (form) (keymap--compile-check key) form))
+           (advertised-calling-convention (key command) "29.1"))
+  (interactive "KSet key globally:\nCSet key %s globally to command: \np")
+  (when interactive
     (setq key (key-description key)))
   (keymap-set (current-global-map) key command))
 
-(defun keymap-local-set (key command)
+(defun keymap-local-set (key command &optional interactive)
   "Give KEY a local binding as COMMAND.
 COMMAND is the command definition to use; usually it is
 a symbol naming an interactively-callable function.
@@ -90,12 +91,13 @@ keymap-local-set
 
 The binding goes in the current buffer's local map, which in most
 cases is shared with all other buffers in the same major mode."
-  (declare (compiler-macro (lambda (form) (keymap--compile-check key) form)))
-  (interactive "KSet key locally:\nCSet key %s locally to command: ")
+  (declare (compiler-macro (lambda (form) (keymap--compile-check key) form))
+           (advertised-calling-convention (key command) "29.1"))
+  (interactive "KSet key locally:\nCSet key %s locally to command: \np")
   (let ((map (current-local-map)))
     (unless map
       (use-local-map (setq map (make-sparse-keymap))))
-    (unless (stringp key)
+    (when interactive
       (setq key (key-description key)))
     (keymap-set map key command)))
 
Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Daniel Mendler <mail@daniel-mendler.de>,  monnier@iro.umontreal.ca,
>   larsi@gnus.org,  emacs-devel@gnu.org
> Date: Thu, 02 Feb 2023 10:40:54 +0100
> 
> >>>>> On Thu, 02 Feb 2023 08:58:19 +0200, Eli Zaretskii <eliz@gnu.org> said:
> 
>     Eli> So, Robert, please install your last patch, and thanks.
> 
> With `advertised-calling-convention', perhaps?

Yes, thanks.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

>>>>> On Thu, 02 Feb 2023 12:17:16 +0200, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: Daniel Mendler <mail@daniel-mendler.de>,  monnier@iro.umontreal.ca,
    >> larsi@gnus.org,  emacs-devel@gnu.org
    >> Date: Thu, 02 Feb 2023 10:40:54 +0100
    >> 
    >> >>>>> On Thu, 02 Feb 2023 08:58:19 +0200, Eli Zaretskii
    >> <eliz@gnu.org> said:
    >> 
    Eli> So, Robert, please install your last patch, and thanks.
    >> 
    >> With `advertised-calling-convention', perhaps?

    Eli> Yes, thanks.

Now done as e444115d026

Robert
-- 

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[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. ]]]

  > `called-interactive[ly]-p` is in the same category as `advice-add`.
  > It's great that we have it, but you should only use it when there's no
  > other way.

I wrote the recommendation to use an extra argument to distinguish
interactive calls because it is simple, reliable, and understandable.
It uses mechanisms that we use all the time in other ways.
When you look at code which does that, you will understand what it does.

`called-interactively-p' can't help being complex because it needs to
figure out retroactively how the call took place.  The extra argument
method instead arranges to remember how the call took place,

Ideally, we should obsolete `called-interactively-p' and then eliminate it.
Hwever, first we'd have to replace all the uses of it and thus verify
that absolutely all of them can be replaced.  I'm not sure it is worth
the effort to do this just to make things cleaner.

The recommended method replaces (called-interactively-p 'any).
Is there an easy way to use this approach to replace
(called-interactively-p 'interactive)?

If not, we could create one: define an interactive spec which
supplies the proper value for the distinguishing argument.

Is there already a way to do that?

  > The problem with the hidden INTERACTIVE (or ALLOW-VECTOR) argument is
  > that it is still somewhat exposed to non-interactive callers. For
  > example, in the context of the Compat library, should this argument be
  > made available, since non-interactive callers may want to use it? What
  > if callers start to use it if they want to pass vector arguments?

I don't know enough about that code to follow the argument here.
Could someone tell me what the Compat library does?
Is it in Emacs -- if so, what is the file name?

  > To come back to `keymap-local-set' - the good thing is that we can avoid
  > both kludges, advertised calling conventions and `call-interactively-p'.
  > All we have to do is move the key reading entirely to the interactive
  > form, which also seems to be semantically most correct. If I would rate
  > the possible solutions:

  > 1. Just allow vector arguments for all the functions. Simplest solution,
  > least amount of code.

If the change this advocates is what I think it is,
it would inconvenience users by making them change their old code.

If someone thinks that the extra-argument solution has a flaw in this
case, would you please present reasons for that conclusion>

-- 
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)

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 2/3/23 10:56, Richard Stallman wrote:
> I don't know enough about that code to follow the argument here.
> Could someone tell me what the Compat library does?
> Is it in Emacs -- if so, what is the file name?

Compat is not part of the Emacs code base. It is instead distributed via
GNU ELPA. It is an Elisp compatibility library, which allows packages to
use functions and macros introduced in newer versions of Emacs, while
staying backward compatible. For example, the function
`keymap-local-set' will be introduced as part of Emacs 29.1.
Correspondingly, Compat 29.1 provides this function too. A package
targeting Emacs 24.4 and newer can then depend on Compat 29.1 and use
`keymap-local-set'.

See the GNU ELPA Compat page https://elpa.gnu.org/packages/compat.html
and the manual linked there for more details.

Daniel

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[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. ]]]

  > The advertised API wouldn't change.  We don't expect anyone to use the
  > additional argument in non-interactive invocation.  We can use
  > advertised-calling-convention declaration to hide that argument from
  > documented interfaces.

Why hide it?   It's better to document it.
Occasionally, passing a nontrivial value for that argument is useful.

  > I still believe it is better to not leak implementation details as an
  > optional argument in this 

I would not call this 'leaking', because that assumes there something
we need to "contain".  Having an argument for interactive call,
described in the function's doc string is not a problem, just a slight
complexity.

By coontrsst, `called-interactively-p' has real problems.
It was called "fragile" for good reasons.


-- 
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)

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Fri, 03 Feb 2023 04:56:49 -0500
> 
>   > The advertised API wouldn't change.  We don't expect anyone to use the
>   > additional argument in non-interactive invocation.  We can use
>   > advertised-calling-convention declaration to hide that argument from
>   > documented interfaces.
> 
> Why hide it?   It's better to document it.
> Occasionally, passing a nontrivial value for that argument is useful.

We don't want Lisp programs to call this function pretending to be the
user, because this function's raison d'être is to catch invalid key
sequences.  So we don't want to advertise that argument in the
documentation.  Of course, anyone who looks at the source will quickly
discover the argument and will be able to take advantage of it.  But
we don't want that to be too easy.

As for documenting it: I don't see the need, as its role is crystal
clear once one looks at the code which uses it.

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[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. ]]]

  > >   > The advertised API wouldn't change.  We don't expect anyone to use the
  > >   > additional argument in non-interactive invocation.  We can use
  > >   > advertised-calling-convention declaration to hide that argument from
  > >   > documented interfaces.
  > > 
  > > Why hide it?   It's better to document it.
  > > Occasionally, passing a nontrivial value for that argument is useful.

  > We don't want Lisp programs to call this function pretending to be the
  > user, because this function's raison d'être is to catch invalid key
  > sequences.

I thought we were talking about the general question, comparing
various nethods for distinguishing an interactive call.  You seem to
be talking about why some specific function wants to know when it is
called interactively.  But I don't know which function it is.

In general, when a function does something different for an interactive call.
it may be useful for its caller to say, "Treat this call as interactive."

For instance, if `foo' does something special if called interactively,
and `bar' calls `foo', maybe `bar' wants to check for an interactive
call and pass that along to `foo'.  With an argument to distinguish,
that is eas for `foo' to do.  With `callsd-interactiely-p',
it seems impossible.



-- 
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)

Re: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> From: Richard Stallman <rms@gnu.org>
> Cc: mail@daniel-mendler.de, emacs-devel@gnu.org
> Date: Sat, 04 Feb 2023 23:27:10 -0500
> 
>   > >   > The advertised API wouldn't change.  We don't expect anyone to use the
>   > >   > additional argument in non-interactive invocation.  We can use
>   > >   > advertised-calling-convention declaration to hide that argument from
>   > >   > documented interfaces.
>   > > 
>   > > Why hide it?   It's better to document it.
>   > > Occasionally, passing a nontrivial value for that argument is useful.
> 
>   > We don't want Lisp programs to call this function pretending to be the
>   > user, because this function's raison d'être is to catch invalid key
>   > sequences.
> 
> I thought we were talking about the general question, comparing
> various nethods for distinguishing an interactive call.  You seem to
> be talking about why some specific function wants to know when it is
> called interactively.  But I don't know which function it is.

The two functions which are being discussed here are named in the
Subject.

> In general, when a function does something different for an interactive call.
> it may be useful for its caller to say, "Treat this call as interactive."

If needed, this is possible in this case, although we don't expect
that to happen in practice.

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Robert Pluim <rpluim@gmail.com>,
>  Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 30 Jan 2023 21:52:54 +0100
> From: Daniel Mendler <mail@daniel-mendler.de>
> 
> In patch f67a9a12b7b0cdd6030cb080a6d6838255789a08, the commands
> keymap-local-set and keymap-global-set became less strict for
> non-interactive use, which is not the intended the design.

Please tell more about that, because I don't yet see how that commit
made the non-interactive (and only non-interactive) invocations check
the argument less strictly.

Thanks.

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 1/31/23 13:10, Eli Zaretskii wrote:
>> Cc: Lars Ingebrigtsen <larsi@gnus.org>, Robert Pluim <rpluim@gmail.com>,
>>  Stefan Monnier <monnier@iro.umontreal.ca>
>> Date: Mon, 30 Jan 2023 21:52:54 +0100
>> From: Daniel Mendler <mail@daniel-mendler.de>
>>
>> In patch f67a9a12b7b0cdd6030cb080a6d6838255789a08, the commands
>> keymap-local-set and keymap-global-set became less strict for
>> non-interactive use, which is not the intended the design.
> 
> Please tell more about that, because I don't yet see how that commit
> made the non-interactive (and only non-interactive) invocations check
> the argument less strictly.

The function became less strict since it also accepts vector arguments
now. The intended design by Lars was to only accept strings, and only
strings which are valid according to `key-valid-p'.

Daniel

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Eli Zaretskii]

> Date: Tue, 31 Jan 2023 13:49:11 +0100
> Cc: 61184@debbugs.gnu.org, larsi@gnus.org, rpluim@gmail.com,
>  monnier@iro.umontreal.ca
> From: Daniel Mendler <mail@daniel-mendler.de>
> 
> The function became less strict since it also accepts vector arguments
> now.

Because after the change it calls key-description?  Or due to
something else?

If the call to key-description is the problem, we could make it only
in the interactive case, I think?  Would that fix the problem?

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Daniel Mendler]

On 1/31/23 15:16, Eli Zaretskii wrote:
>> Date: Tue, 31 Jan 2023 13:49:11 +0100
>> Cc: 61184@debbugs.gnu.org, larsi@gnus.org, rpluim@gmail.com,
>>  monnier@iro.umontreal.ca
>> From: Daniel Mendler <mail@daniel-mendler.de>
>>
>> The function became less strict since it also accepts vector arguments
>> now.
> 
> Because after the change it calls key-description?  Or due to
> something else?

Yes, because of the conversion by key-description.

> If the call to key-description is the problem, we could make it only
> in the interactive case, I think?  Would that fix the problem?

Yes, that would work as I already suggested in my original mail.

Daniel

bug#61184: 29.0.60; keymap-local-set and keymap-global-set became less strict

[Robert Pluim]

tags 61184 fixed
close 61184 29.1
quit

>>>>> On Tue, 31 Jan 2023 15:53:01 +0100, Daniel Mendler <mail@daniel-mendler.de> said:

    Daniel> On 1/31/23 15:16, Eli Zaretskii wrote:
    >>> Date: Tue, 31 Jan 2023 13:49:11 +0100
    >>> Cc: 61184@debbugs.gnu.org, larsi@gnus.org, rpluim@gmail.com,
    >>> monnier@iro.umontreal.ca
    >>> From: Daniel Mendler <mail@daniel-mendler.de>
    >>> 
    >>> The function became less strict since it also accepts vector arguments
    >>> now.
    >> 
    >> Because after the change it calls key-description?  Or due to
    >> something else?

    Daniel> Yes, because of the conversion by key-description.

    >> If the call to key-description is the problem, we could make it only
    >> in the interactive case, I think?  Would that fix the problem?

    Daniel> Yes, that would work as I already suggested in my original mail.

    Daniel> Daniel

The re-fix for 61149 fixes this

Robert
--