unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
       [not found] ` <875ycngyji.fsf@gnus.org>
@ 2023-01-31  9:05   ` Robert Pluim
  2023-01-31 10:08     ` Stephen Berman
                       ` (2 more replies)
  0 siblings, 3 replies; 34+ messages in thread
From: Robert Pluim @ 2023-01-31  9:05 UTC (permalink / raw)
  To: Lars Ingebrigtsen
  Cc: Daniel Mendler, emacs-devel, Stefan Monnier, Eli Zaretskii

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31  9:05   ` 29.0.60; keymap-local-set and keymap-global-set became less strict Robert Pluim
@ 2023-01-31 10:08     ` Stephen Berman
  2023-01-31 14:27       ` [External] : " Drew Adams
  2023-01-31 14:30     ` Stefan Monnier
  2023-01-31 15:06     ` Eli Zaretskii
  2 siblings, 1 reply; 34+ messages in thread
From: Stephen Berman @ 2023-01-31 10:08 UTC (permalink / raw)
  To: Robert Pluim
  Cc: Lars Ingebrigtsen, Daniel Mendler, emacs-devel, Stefan Monnier,
	Eli Zaretskii

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



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

* RE: [External] : Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 10:08     ` Stephen Berman
@ 2023-01-31 14:27       ` Drew Adams
  0 siblings, 0 replies; 34+ messages in thread
From: Drew Adams @ 2023-01-31 14:27 UTC (permalink / raw)
  To: Stephen Berman, Robert Pluim
  Cc: Lars Ingebrigtsen, Daniel Mendler, emacs-devel@gnu.org,
	Stefan Monnier, Eli Zaretskii

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

TIL "TIL"

Not to be confused with "till".



	

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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31  9:05   ` 29.0.60; keymap-local-set and keymap-global-set became less strict Robert Pluim
  2023-01-31 10:08     ` Stephen Berman
@ 2023-01-31 14:30     ` Stefan Monnier
  2023-01-31 15:02       ` Robert Pluim
  2023-01-31 15:06     ` Eli Zaretskii
  2 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2023-01-31 14:30 UTC (permalink / raw)
  To: Robert Pluim
  Cc: Lars Ingebrigtsen, Daniel Mendler, emacs-devel, Eli Zaretskii

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




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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 14:30     ` Stefan Monnier
@ 2023-01-31 15:02       ` Robert Pluim
  0 siblings, 0 replies; 34+ messages in thread
From: Robert Pluim @ 2023-01-31 15:02 UTC (permalink / raw)
  To: Stefan Monnier
  Cc: Lars Ingebrigtsen, Daniel Mendler, emacs-devel, Eli Zaretskii

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31  9:05   ` 29.0.60; keymap-local-set and keymap-global-set became less strict Robert Pluim
  2023-01-31 10:08     ` Stephen Berman
  2023-01-31 14:30     ` Stefan Monnier
@ 2023-01-31 15:06     ` Eli Zaretskii
  2023-01-31 15:48       ` Robert Pluim
  2 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-01-31 15:06 UTC (permalink / raw)
  To: Robert Pluim; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 15:06     ` Eli Zaretskii
@ 2023-01-31 15:48       ` Robert Pluim
  2023-01-31 16:37         ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Robert Pluim @ 2023-01-31 15:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 15:48       ` Robert Pluim
@ 2023-01-31 16:37         ` Eli Zaretskii
  2023-01-31 16:48           ` Robert Pluim
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-01-31 16:37 UTC (permalink / raw)
  To: Robert Pluim; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 16:37         ` Eli Zaretskii
@ 2023-01-31 16:48           ` Robert Pluim
  2023-01-31 18:43             ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Robert Pluim @ 2023-01-31 16:48 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 16:48           ` Robert Pluim
@ 2023-01-31 18:43             ` Eli Zaretskii
  2023-02-01 12:52               ` Robert Pluim
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-01-31 18:43 UTC (permalink / raw)
  To: Robert Pluim; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-01-31 18:43             ` Eli Zaretskii
@ 2023-02-01 12:52               ` Robert Pluim
  2023-02-01 13:06                 ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Robert Pluim @ 2023-02-01 12:52 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 12:52               ` Robert Pluim
@ 2023-02-01 13:06                 ` Eli Zaretskii
  2023-02-01 13:13                   ` Daniel Mendler
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-01 13:06 UTC (permalink / raw)
  To: Robert Pluim; +Cc: larsi, mail, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:06                 ` Eli Zaretskii
@ 2023-02-01 13:13                   ` Daniel Mendler
  2023-02-01 13:44                     ` Robert Pluim
  2023-02-01 13:50                     ` Eli Zaretskii
  0 siblings, 2 replies; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 13:13 UTC (permalink / raw)
  To: Eli Zaretskii, Robert Pluim; +Cc: larsi, emacs-devel, monnier



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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:13                   ` Daniel Mendler
@ 2023-02-01 13:44                     ` Robert Pluim
  2023-02-01 14:11                       ` Daniel Mendler
  2023-02-01 13:50                     ` Eli Zaretskii
  1 sibling, 1 reply; 34+ messages in thread
From: Robert Pluim @ 2023-02-01 13:44 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: Eli Zaretskii, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:13                   ` Daniel Mendler
  2023-02-01 13:44                     ` Robert Pluim
@ 2023-02-01 13:50                     ` Eli Zaretskii
  2023-02-01 13:57                       ` Daniel Mendler
  1 sibling, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-01 13:50 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: rpluim, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:50                     ` Eli Zaretskii
@ 2023-02-01 13:57                       ` Daniel Mendler
  2023-02-01 17:30                         ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 13:57 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rpluim, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:44                     ` Robert Pluim
@ 2023-02-01 14:11                       ` Daniel Mendler
  0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 14:11 UTC (permalink / raw)
  To: Robert Pluim; +Cc: Eli Zaretskii, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 13:57                       ` Daniel Mendler
@ 2023-02-01 17:30                         ` Eli Zaretskii
  2023-02-01 18:20                           ` Daniel Mendler
  2023-02-03  9:56                           ` Richard Stallman
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-01 17:30 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: rpluim, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 17:30                         ` Eli Zaretskii
@ 2023-02-01 18:20                           ` Daniel Mendler
  2023-02-01 18:54                             ` Stefan Monnier
  2023-02-03  9:56                           ` Richard Stallman
  1 sibling, 1 reply; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 18:20 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rpluim, larsi, emacs-devel, monnier

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 18:20                           ` Daniel Mendler
@ 2023-02-01 18:54                             ` Stefan Monnier
  2023-02-01 20:22                               ` Daniel Mendler
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Monnier @ 2023-02-01 18:54 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: Eli Zaretskii, rpluim, larsi, emacs-devel

> 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




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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 18:54                             ` Stefan Monnier
@ 2023-02-01 20:22                               ` Daniel Mendler
  2023-02-01 22:42                                 ` Stefan Monnier
  0 siblings, 1 reply; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 20:22 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, rpluim, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 20:22                               ` Daniel Mendler
@ 2023-02-01 22:42                                 ` Stefan Monnier
  2023-02-01 22:56                                   ` Daniel Mendler
  2023-02-03  9:56                                   ` Richard Stallman
  0 siblings, 2 replies; 34+ messages in thread
From: Stefan Monnier @ 2023-02-01 22:42 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: Eli Zaretskii, rpluim, larsi, emacs-devel

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




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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 22:42                                 ` Stefan Monnier
@ 2023-02-01 22:56                                   ` Daniel Mendler
  2023-02-02  6:58                                     ` Eli Zaretskii
  2023-02-03  9:56                                   ` Richard Stallman
  1 sibling, 1 reply; 34+ messages in thread
From: Daniel Mendler @ 2023-02-01 22:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, rpluim, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 22:56                                   ` Daniel Mendler
@ 2023-02-02  6:58                                     ` Eli Zaretskii
  2023-02-02  9:29                                       ` Daniel Mendler
  2023-02-02  9:40                                       ` Robert Pluim
  0 siblings, 2 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-02  6:58 UTC (permalink / raw)
  To: Daniel Mendler; +Cc: monnier, rpluim, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-02  6:58                                     ` Eli Zaretskii
@ 2023-02-02  9:29                                       ` Daniel Mendler
  2023-02-02  9:40                                       ` Robert Pluim
  1 sibling, 0 replies; 34+ messages in thread
From: Daniel Mendler @ 2023-02-02  9:29 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, rpluim, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-02  6:58                                     ` Eli Zaretskii
  2023-02-02  9:29                                       ` Daniel Mendler
@ 2023-02-02  9:40                                       ` Robert Pluim
  2023-02-02 10:17                                         ` Eli Zaretskii
  1 sibling, 1 reply; 34+ messages in thread
From: Robert Pluim @ 2023-02-02  9:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Daniel Mendler, monnier, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-02  9:40                                       ` Robert Pluim
@ 2023-02-02 10:17                                         ` Eli Zaretskii
  2023-02-03  9:17                                           ` Robert Pluim
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-02 10:17 UTC (permalink / raw)
  To: Robert Pluim; +Cc: mail, monnier, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-02 10:17                                         ` Eli Zaretskii
@ 2023-02-03  9:17                                           ` Robert Pluim
  0 siblings, 0 replies; 34+ messages in thread
From: Robert Pluim @ 2023-02-03  9:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mail, monnier, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 22:42                                 ` Stefan Monnier
  2023-02-01 22:56                                   ` Daniel Mendler
@ 2023-02-03  9:56                                   ` Richard Stallman
  2023-02-04  9:55                                     ` Daniel Mendler
  1 sibling, 1 reply; 34+ messages in thread
From: Richard Stallman @ 2023-02-03  9:56 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: mail, eliz, rpluim, larsi, emacs-devel

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





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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-01 17:30                         ` Eli Zaretskii
  2023-02-01 18:20                           ` Daniel Mendler
@ 2023-02-03  9:56                           ` Richard Stallman
  2023-02-03 12:11                             ` Eli Zaretskii
  1 sibling, 1 reply; 34+ messages in thread
From: Richard Stallman @ 2023-02-03  9:56 UTC (permalink / raw)
  To: Eli Zaretskii, mail; +Cc: emacs-devel

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





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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-03  9:56                           ` Richard Stallman
@ 2023-02-03 12:11                             ` Eli Zaretskii
  2023-02-05  4:27                               ` Richard Stallman
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-03 12:11 UTC (permalink / raw)
  To: rms; +Cc: mail, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-03  9:56                                   ` Richard Stallman
@ 2023-02-04  9:55                                     ` Daniel Mendler
  0 siblings, 0 replies; 34+ messages in thread
From: Daniel Mendler @ 2023-02-04  9:55 UTC (permalink / raw)
  To: rms, Stefan Monnier; +Cc: eliz, rpluim, larsi, emacs-devel

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



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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-03 12:11                             ` Eli Zaretskii
@ 2023-02-05  4:27                               ` Richard Stallman
  2023-02-05  7:11                                 ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Richard Stallman @ 2023-02-05  4:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: mail, emacs-devel

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





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

* Re: 29.0.60; keymap-local-set and keymap-global-set became less strict
  2023-02-05  4:27                               ` Richard Stallman
@ 2023-02-05  7:11                                 ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-02-05  7:11 UTC (permalink / raw)
  To: rms; +Cc: mail, emacs-devel

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



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

end of thread, other threads:[~2023-02-05  7:11 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <5876987d-2479-f512-5767-218c8c16a909@daniel-mendler.de>
     [not found] ` <875ycngyji.fsf@gnus.org>
2023-01-31  9:05   ` 29.0.60; keymap-local-set and keymap-global-set became less strict Robert Pluim
2023-01-31 10:08     ` Stephen Berman
2023-01-31 14:27       ` [External] : " Drew Adams
2023-01-31 14:30     ` Stefan Monnier
2023-01-31 15:02       ` Robert Pluim
2023-01-31 15:06     ` Eli Zaretskii
2023-01-31 15:48       ` Robert Pluim
2023-01-31 16:37         ` Eli Zaretskii
2023-01-31 16:48           ` Robert Pluim
2023-01-31 18:43             ` Eli Zaretskii
2023-02-01 12:52               ` Robert Pluim
2023-02-01 13:06                 ` Eli Zaretskii
2023-02-01 13:13                   ` Daniel Mendler
2023-02-01 13:44                     ` Robert Pluim
2023-02-01 14:11                       ` Daniel Mendler
2023-02-01 13:50                     ` Eli Zaretskii
2023-02-01 13:57                       ` Daniel Mendler
2023-02-01 17:30                         ` Eli Zaretskii
2023-02-01 18:20                           ` Daniel Mendler
2023-02-01 18:54                             ` Stefan Monnier
2023-02-01 20:22                               ` Daniel Mendler
2023-02-01 22:42                                 ` Stefan Monnier
2023-02-01 22:56                                   ` Daniel Mendler
2023-02-02  6:58                                     ` Eli Zaretskii
2023-02-02  9:29                                       ` Daniel Mendler
2023-02-02  9:40                                       ` Robert Pluim
2023-02-02 10:17                                         ` Eli Zaretskii
2023-02-03  9:17                                           ` Robert Pluim
2023-02-03  9:56                                   ` Richard Stallman
2023-02-04  9:55                                     ` Daniel Mendler
2023-02-03  9:56                           ` Richard Stallman
2023-02-03 12:11                             ` Eli Zaretskii
2023-02-05  4:27                               ` Richard Stallman
2023-02-05  7:11                                 ` Eli Zaretskii

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

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

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