bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Shynur Xie]

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

To make 

    (key-translate "C-x" "<control-x>")
    ;; this is an example in elisp manual 

work.

[-- Attachment #2: 0001-key-translate-uses-the-1st-key-of-a-key-sequence.patch --]
[-- Type: application/octet-stream, Size: 1386 bytes --]

From 856bd7a0c696d0ff22e62d70688b287a8214dcda Mon Sep 17 00:00:00 2001
From: shynur <one.last.kiss@outlook.com>
Date: Mon, 4 Sep 2023 23:37:33 +0800
Subject: [PATCH] `key-translate' uses the 1st key of a key sequence

---
 lisp/keymap.el | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lisp/keymap.el b/lisp/keymap.el
index 017b2d6..5a4021a 100644
--- a/lisp/keymap.el
+++ b/lisp/keymap.el
@@ -378,7 +378,7 @@ which is
 This function creates a `keyboard-translate-table' if necessary
 and then modifies one entry in it.
 
-Both KEY and TO should be specified by strings that satisfy `key-valid-p'."
+Both FROM and TO should be specified by strings that satisfy `key-valid-p'."
   (declare (compiler-macro
             (lambda (form) (keymap--compile-check from to) form)))
   (keymap--check from)
@@ -386,7 +386,9 @@ Both KEY and TO should be specified by strings that satisfy `key-valid-p'."
   (or (char-table-p keyboard-translate-table)
       (setq keyboard-translate-table
 	    (make-char-table 'keyboard-translate-table nil)))
-  (aset keyboard-translate-table (key-parse from) (key-parse to)))
+  (aset keyboard-translate-table
+        (aref (key-parse from) 0)
+        (aref (key-parse   to) 0)))
 
 (defun keymap-lookup (keymap key &optional accept-default no-remap position)
   "Return the binding for command KEY in KEYMAP.
-- 
2.41.0.windows.3

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Robert Pluim]

>>>>> On Mon, 4 Sep 2023 15:46:32 +0000, Shynur Xie <one.last.kiss@outlook.com> said:

    Shynur> To make 
    Shynur>     (key-translate "C-x" "<control-x>")
    Shynur>     ;; this is an example in elisp manual 

    Shynur> work.

Yes, oops. Perhaps we should also check that `from' and `to' specify
single keys, although that might be overkill.

I see commits from you both with and without
Copyright-paperwork-exempt, do you have a copyright assignment on
file?

Robert
-- 

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Eli Zaretskii]

> Cc: 65735@debbugs.gnu.org
> From: Robert Pluim <rpluim@gmail.com>
> Date: Thu, 07 Sep 2023 09:54:13 +0200
> 
> I see commits from you both with and without
> Copyright-paperwork-exempt, do you have a copyright assignment on
> file?

He does, now.

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Robert Pluim]

>>>>> On Thu, 07 Sep 2023 11:15:44 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> Cc: 65735@debbugs.gnu.org
    >> From: Robert Pluim <rpluim@gmail.com>
    >> Date: Thu, 07 Sep 2023 09:54:13 +0200
    >> 
    >> I see commits from you both with and without
    >> Copyright-paperwork-exempt, do you have a copyright assignment on
    >> file?

    Eli> He does, now.

OK. Then Iʼll push it to emacs-29, since otherwise `key-translate' is
completely useless.

In the interests of, uhm, eating our own dogfood, how about the
following as well for master (or should I let sleeping dogs lie 😺)

diff --git i/lisp/simple.el w/lisp/simple.el
index 05a3c4b93d6..da9db1685c4 100644
--- i/lisp/simple.el
+++ w/lisp/simple.el
@@ -10658,10 +10658,10 @@ normal-erase-is-backspace-mode
 	  (t
 	   (if enabled
 	       (progn
-		 (keyboard-translate ?\C-h ?\C-?)
-		 (keyboard-translate ?\C-? ?\C-d))
-	     (keyboard-translate ?\C-h ?\C-h)
-	     (keyboard-translate ?\C-? ?\C-?))))
+		 (key-translate "C-h" "DEL")
+		 (key-translate "DEL" "C-d"))
+	     (key-translate "C-h" "C-h")
+	     (key-translate "DEL" "DEL"))))
 
     (if (called-interactively-p 'interactive)
 	(message "Delete key deletes %s"
diff --git i/lisp/term/bobcat.el w/lisp/term/bobcat.el
index 983c8cded2f..2f611953523 100644
--- i/lisp/term/bobcat.el
+++ w/lisp/term/bobcat.el
@@ -3,8 +3,8 @@
 (defun terminal-init-bobcat ()
   "Terminal initialization function for bobcat."
   ;; HP terminals usually encourage using ^H as the rubout character
-  (keyboard-translate ?\177 ?\^h)
-  (keyboard-translate ?\^h ?\177))
+  (key-translate "DEL "C-h")
+  (key-translate "C-h "DEL"))
 
 (provide 'term/bobcat)
 

Robert
-- 

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: one.last.kiss@outlook.com,  65735@debbugs.gnu.org
> Date: Thu, 07 Sep 2023 13:57:14 +0200
> 
> >>>>> On Thu, 07 Sep 2023 11:15:44 +0300, Eli Zaretskii <eliz@gnu.org> said:
> 
>     >> Cc: 65735@debbugs.gnu.org
>     >> From: Robert Pluim <rpluim@gmail.com>
>     >> Date: Thu, 07 Sep 2023 09:54:13 +0200
>     >> 
>     >> I see commits from you both with and without
>     >> Copyright-paperwork-exempt, do you have a copyright assignment on
>     >> file?
> 
>     Eli> He does, now.
> 
> OK. Then Iʼll push it to emacs-29, since otherwise `key-translate' is
> completely useless.

Yes, thanks.

> In the interests of, uhm, eating our own dogfood, how about the
> following as well for master (or should I let sleeping dogs lie 😺)

I'll let Stefan answer that, as, among the two of us, he is the
enthusiast of modernizing ;-)

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Shynur Xie]

> Robert:
> Perhaps we should also check that `from' and `to' specify single
> keys, although that might be overkill.

Not necessary IMO.
As a regular Emacs user, I would read its docstring before using this
function; the docstring already explains how to use it very clearly.
Moreover, there're several examples in the elisp manual for reference:

   +----------------------------------------------------+
   | -- Function: key-translate from to                 |
   |     ... ...                                        |
   |   Here’s an example of ...                         |
   |     (key-translate "C-x" "<control-x>")            |
   |     (keymap-global-set "<control-x>" 'kill-region) |
   +----------------------------------------------------+

> Robert:
> do you have a copyright assignment on file?

Yes.

> Robert:
> --- i/lisp/simple.el
> +++ w/lisp/simple.el

> - (keyboard-translate ?\C-h ?\C-?)
> -	(keyboard-translate ?\C-? ?\C-d))
> -	(keyboard-translate ?\C-h ?\C-h)
> -	(keyboard-translate ?\C-? ?\C-?))))
> +	(key-translate "C-h" "DEL")
> +	(key-translate "DEL" "C-d"))
> +	(key-translate "C-h" "C-h")
> +	(key-translate "DEL" "DEL"))))

The docstring of `keyboard-translate' does suggest using
`key-translate'; similarly, `define-key' suggests using `keymap-set'.

FWIW, I noticed that there're also some `define-key' invocations in
the file "lisp/simple.el".

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Robert Pluim]

>>>>> On Thu, 7 Sep 2023 13:42:18 +0000, Shynur Xie <one.last.kiss@outlook.com> said:

    >> Robert:
    >> Perhaps we should also check that `from' and `to' specify single
    >> keys, although that might be overkill.

    Shynur> Not necessary IMO.
    Shynur> As a regular Emacs user, I would read its docstring before using this
    Shynur> function; the docstring already explains how to use it very clearly.
    Shynur> Moreover, there're several examples in the elisp manual for reference:

OK, Iʼll leave that bit alone.

    Shynur> The docstring of `keyboard-translate' does suggest using
    Shynur> `key-translate'; similarly, `define-key' suggests using `keymap-set'.

Right. But the question is whether we should change Emacs to use the
new functions, or whether we call it 'unnecessary churn'.

    Shynur> FWIW, I noticed that there're also some `define-key' invocations in
    Shynur> the file "lisp/simple.el".

One function at a time :-)

Robert
-- 

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Stefan Kangas]

Eli Zaretskii <eliz@gnu.org> writes:

>> In the interests of, uhm, eating our own dogfood, how about the
>> following as well for master (or should I let sleeping dogs lie 😺)
>
> I'll let Stefan answer that, as, among the two of us, he is the
> enthusiast of modernizing ;-)

;-)

I have no objections to the patch, so please go ahead.

One benefit of using our "new" API's (a.k.a. dogfooding) is that it
gives us a chance to test them in practice.  For example, it helped me
find some issues in keymap.el that we were able to rectify before 29.1.

bug#65735: [PATCH] key-translate should use the 1st key of a key sequence

[Robert Pluim]

tags 65735 fixed
close 65735 29.2
quit

>>>>> On Thu, 7 Sep 2023 07:01:44 -0700, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Eli Zaretskii <eliz@gnu.org> writes:
    >>> In the interests of, uhm, eating our own dogfood, how about the
    >>> following as well for master (or should I let sleeping dogs lie 😺)
    >> 
    >> I'll let Stefan answer that, as, among the two of us, he is the
    >> enthusiast of modernizing ;-)

    Stefan> ;-)

    Stefan> I have no objections to the patch, so please go ahead.

    Stefan> One benefit of using our "new" API's (a.k.a. dogfooding) is that it
    Stefan> gives us a chance to test them in practice.  For example, it helped me
    Stefan> find some issues in keymap.el that we were able to rectify before 29.1.

Closing.
Committed as 93134bb9c2f

Iʼll push the change to master once emacs-29 has been merged there.

Robert
--