GNU bug report logs - #27634
25.2.1; C-g does not quit register-read-with-preview

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 27634 in the body.
You can then email your comments to 27634 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Paul Rankin <hello <at> paulwrankin.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.2.1; C-g does not quit register-read-with-preview
Date: Mon, 10 Jul 2017 13:58:54 +1000

When saving to a register C-g does not quit but instead saves to the register "C-g".

To reproduce:

1. $ emacs -Q
2. M-x point-to-register [C-x r SPC]
3. C-g

Expected results:

C-g should call keyboard-quit and escape point-to-register

Actual results:

Point marker is saved to register "C-g".

This runs counter to expected behaviour with both quitting from saving a register, and in quitting from recalling a register, e.g. by attempting to quit from saving a point to register in buffer-A with C-g, when the user then attempts to keyboard-quit from jump-to-register [C-x r j C-g] he/she will be returned to point in buffer-A.

The root of the problem appears to be that register-read-with-preview only tests input with characterp:

(characterp ?^G) -> t

This appears to conflict with the Emacs manual entry on registers, which states that registers can be a letters or numbers:

> Each register has a name that consists of a single character, which
> we will denote by R; R can be a letter (such as ‘a’) or a number (such
> as ‘1’); case matters, so register ‘a’ is not the same as register ‘A’.

(info "(emacs) Registers")

I have worked around this with the following patch to register.el.gz:

@@ -164,7 +164,7 @@
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
-	  (if (characterp last-input-event) last-input-event
+	  (if (< 31 last-input-event 127) last-input-event
 	    (error "Non-character input-event")))
       (and (timerp timer) (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))

However, I think this will not work on international keyboards and so is probably a poor fix.

Configuration:

GNU Emacs 25.2.1 (x86_64-apple-darwin16.6.0, NS appkit-1504.83 Version 10.12.5 (Build 16F73)) of 2017-06-06
macOS 10.12.5 (16F73)

MacBook Pro (Retina, 15-inch, Mid 2014)
2.2 GHz Intel Core i7
16 GB 1600 MHz DDR3
Intel Iris Pro 1536 MB

--
www.paulwrankin.com

Message #8 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Mon, 10 Jul 2017 15:33:42 +0900

Paul Rankin <hello <at> paulwrankin.com> writes:

> When saving to a register C-g does not quit but instead saves to the register "C-g".
>
> To reproduce:
>
> 1. $ emacs -Q
> 2. M-x point-to-register [C-x r SPC]
> 3. C-g
>
> Expected results:
>
> C-g should call keyboard-quit and escape point-to-register
>
> Actual results:
>
> Point marker is saved to register "C-g".

> I have worked around this with the following patch to register.el.gz:
>
> @@ -164,7 +164,7 @@
>  		       help-chars)
>  	    (unless (get-buffer-window buffer)
>  	      (register-preview buffer 'show-empty)))
> -	  (if (characterp last-input-event) last-input-event
> +	  (if (< 31 last-input-event 127) last-input-event
>  	    (error "Non-character input-event")))
>        (and (timerp timer) (cancel-timer timer))
>        (let ((w (get-buffer-window buffer)))
>
> However, I think this will not work on international keyboards and so is probably a poor fix.
How about the following?

@@ -164,6 +164,8 @@ register-read-with-preview
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
+          (when (eq (string-to-char "\C-g") last-input-event)
+            (keyboard-quit))
 	  (if (characterp last-input-event) last-input-event
 	    (error "Non-character input-event")))
       (and (timerp timer) (cancel-timer timer))


--8<-----------------------------cut here---------------start------------->8---
commit 51de600be1351704d4e435a4977d63b3d4e04eaf
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Mon Jul 10 15:22:32 2017 +0900

    Don't set registers in 'C-g'
    
    That key is reserved to abort commands (Bug#27634).
    * lisp/register-tests.el (register-read-with-preview):
    Abort when user inputs 'C-g'.
    * test/lisp/register-tests.el (register-test-bug27634): Add test.

diff --git a/lisp/register.el b/lisp/register.el
index 7cc3ccd870..19b7dee4b9 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -164,6 +164,8 @@ register-read-with-preview
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
+          (when (eq (string-to-char "\C-g") last-input-event)
+            (keyboard-quit))
 	  (if (characterp last-input-event) last-input-event
 	    (error "Non-character input-event")))
       (and (timerp timer) (cancel-timer timer))
diff --git a/test/lisp/register-tests.el b/test/lisp/register-tests.el
new file mode 100644
index 0000000000..30c1468e25
--- /dev/null
+++ b/test/lisp/register-tests.el
@@ -0,0 +1,40 @@
+;;; register-tests.el --- tests for electric.el
+
+;; Copyright (C) 2017 Free Software Foundation, Inc.
+
+;; Author: Tino Calacha <tino.calancha <at> gmail.com>
+;; Keywords:
+
+;; This program is free software; you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This program is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+
+;;; Code:
+(require 'ert)
+(require 'cl-lib)
+
+(ert-deftest register-test-bug27634 ()
+  "Test for http://debbugs.gnu.org/27634 ."
+  (cl-letf (((symbol-function 'read-key) #'ignore)
+            (last-input-event 7)
+            (register-alist nil))
+    (should (equal 'quit
+                   (condition-case err
+                       (call-interactively 'point-to-register)
+                     (quit (car err)))))
+    (should-not register-alist)))
+
+(provide 'register-tests)
+;;; register-tests.el ends here

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-07-10
Repository revision: 273f4bde39af5d87f10fd58f35b666dfa8a996a3

Message #11 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Paul Rankin <hello <at> paulwrankin.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Mon, 10 Jul 2017 17:20:55 +1000

On Mon, 10 Jul 2017, at 04:33 PM, Tino Calancha wrote:
> How about the following?
> 
> @@ -164,6 +164,8 @@ register-read-with-preview
>  		       help-chars)
>  	    (unless (get-buffer-window buffer)
>  	      (register-preview buffer 'show-empty)))
> +          (when (eq (string-to-char "\C-g") last-input-event)
> +            (keyboard-quit))
>  	  (if (characterp last-input-event) last-input-event
>  	    (error "Non-character input-event")))
>        (and (timerp timer) (cancel-timer timer))

I think that's more a bandaid than fixing the root of the problem, which is that the manual tells the user that register names are alphanumeric characters (and I assume 99% of users only use alphanumeric characters) but the function doesn't test for this. e.g. testing for C-g doesn't catch for ^L or ^M, etc.

If we want to be strict about it, this might work:

@@ -164,8 +164,8 @@
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
-	  (if (characterp last-input-event) last-input-event
-	    (error "Non-character input-event")))
+	  (if (= (char-syntax last-input-event) 119) last-input-event
+	    (error "Register name must be alphanumeric")))
       (and (timerp timer) (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))

That prohibits anything except "a-zA-Z0-9", although users may want to save registers to "$" or "*".

Message #14 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#27634: 25.2.1; C-g does not quit
 register-read-with-preview
Date: Mon, 10 Jul 2017 16:59:26 +0900 (JST)

On Mon, 10 Jul 2017, Paul Rankin wrote:

> On Mon, 10 Jul 2017, at 04:33 PM, Tino Calancha wrote:
>> How about the following?
>>
>> @@ -164,6 +164,8 @@ register-read-with-preview
>>  		       help-chars)
>>  	    (unless (get-buffer-window buffer)
>>  	      (register-preview buffer 'show-empty)))
>> +          (when (eq (string-to-char "\C-g") last-input-event)
>> +            (keyboard-quit))
>>  	  (if (characterp last-input-event) last-input-event
>>  	    (error "Non-character input-event")))
>>        (and (timerp timer) (cancel-timer timer))
>
> I think that's more a bandaid than fixing the root of the problem, which is that the manual tells the user that register names are alphanumeric characters (and I assume 99% of users only use alphanumeric characters) but the function doesn't test for this. e.g. testing for C-g doesn't catch for ^L or ^M, etc.
Opps, you are right.
>
> If we want to be strict about it, this might work:
>
> @@ -164,8 +164,8 @@
> 		       help-chars)
> 	    (unless (get-buffer-window buffer)
> 	      (register-preview buffer 'show-empty)))
> -	  (if (characterp last-input-event) last-input-event
> -	    (error "Non-character input-event")))
> +	  (if (= (char-syntax last-input-event) 119) last-input-event
> +	    (error "Register name must be alphanumeric")))
>       (and (timerp timer) (cancel-timer timer))
>       (let ((w (get-buffer-window buffer)))
>         (and (window-live-p w) (delete-window w)))
>
> That prohibits anything except "a-zA-Z0-9", although users may want to save registers to "$" or "*".
I cannot assure that i've never used "@" or "," as a register.  Probably 
i did it several times.

Message #17 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: 27634 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Mon, 10 Jul 2017 20:06:21 +0300

> From: Paul Rankin <hello <at> paulwrankin.com>
> Cc: 27634 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
> Date: Mon, 10 Jul 2017 17:20:55 +1000
> 
> On Mon, 10 Jul 2017, at 04:33 PM, Tino Calancha wrote:
> > How about the following?
> > 
> > @@ -164,6 +164,8 @@ register-read-with-preview
> >  		       help-chars)
> >  	    (unless (get-buffer-window buffer)
> >  	      (register-preview buffer 'show-empty)))
> > +          (when (eq (string-to-char "\C-g") last-input-event)
> > +            (keyboard-quit))
> >  	  (if (characterp last-input-event) last-input-event
> >  	    (error "Non-character input-event")))
> >        (and (timerp timer) (cancel-timer timer))
> 
> I think that's more a bandaid than fixing the root of the problem, which is that the manual tells the user that register names are alphanumeric characters (and I assume 99% of users only use alphanumeric characters) but the function doesn't test for this. e.g. testing for C-g doesn't catch for ^L or ^M, etc.

FWIW, I actually agree with Tino's solution, and was about to propose
something similar.  It's true that control characters are not
alphanumeric, but we could fix the documentation to be more accurate
if we care about that.  OTOH, we've supported control characters as
register names for many years, and by now it should be quite clear it
didn't bother anyone yet.

C-g needs indeed to generate keyboard quit, and perhaps ESC ESC as
well.  I'd look at read-char-choice for inspiration.

> If we want to be strict about it, this might work:
> 
> @@ -164,8 +164,8 @@
>  		       help-chars)
>  	    (unless (get-buffer-window buffer)
>  	      (register-preview buffer 'show-empty)))
> -	  (if (characterp last-input-event) last-input-event
> -	    (error "Non-character input-event")))
> +	  (if (= (char-syntax last-input-event) 119) last-input-event
> +	    (error "Register name must be alphanumeric")))
>        (and (timerp timer) (cancel-timer timer))
>        (let ((w (get-buffer-window buffer)))
>          (and (window-live-p w) (delete-window w)))
> 
> That prohibits anything except "a-zA-Z0-9", although users may want to save registers to "$" or "*".

Why would we want to be so strict when the only real problem is that
C-g doesn't quit?

Message #20 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Paul Rankin <hello <at> paulwrankin.com>, 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Mon, 10 Jul 2017 21:01:24 +0200

On Jul 10 2017, Tino Calancha <tino.calancha <at> gmail.com> wrote:

> +          (when (eq (string-to-char "\C-g") last-input-event)
                       ?\C-g

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #23 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Paul Rankin <hello <at> paulwrankin.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 27634 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Tue, 11 Jul 2017 14:14:22 +1000

On Tue, 11 Jul 2017, at 03:06 AM, Eli Zaretskii wrote:
> FWIW, I actually agree with Tino's solution, and was about to propose
> something similar. It's true that control characters are not
> alphanumeric, but we could fix the documentation to be more accurate
> if we care about that.  OTOH, we've supported control characters as
> register names for many years, and by now it should be quite clear it
> didn't bother anyone yet.

Hmm, it bothers me?

If no one had reported an issue before now, it doesn't then follow that the issue didn't bother anyone, or, wouldn't have bothered them if they knew about it.

But the question is moot I think, since this is an opportunity to improve the code.. why waste time arguing for poorer code when we can make it better?
 
> > That prohibits anything except "a-zA-Z0-9", although users may want to save registers to "$" or "*".
> 
> Why would we want to be so strict when the only real problem is that
> C-g doesn't quit?

I think there are two good options for good UX: make the code reflect the documentation (this is the strict option), or update both the documentation and the code to reflect what we believe is user expectation, i.e. that the user may save registers to any character key on their keyboard (this is my preference).

One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?

Message #26 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org,
 tino.calancha <at> gmail.com
Subject: Re: bug#27634: 25.2.1; C-g does not quit
 register-read-with-preview
Date: Tue, 11 Jul 2017 13:48:03 +0900 (JST)

On Tue, 11 Jul 2017, Paul Rankin wrote:

> One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?
I thought about that, but i discarded because i think binding something
other that `keyboard-quit' to `C-g' is a misuse.  The Emacs manual is full
of mentions to `C-g' as `keyboard-quit'.
There is even the following remark in the tips section:

"don't bind a key sequence ending in @key{C-g}, since that
is commonly used to cancel a key sequence."

If a user want to ignore such kind of advice he/she should
not expect everything will work the same.

Maybe we can fix this so that `register-read-with-preview'
will work with `C-g' bound to `my-cool-foo-command'; but we
cannot assure that no other Emacs part is affected because such
misguided `C-g' binding.  We must encourage users to follow
good practices.

Sure, it would be great if the entire Emacs code is robust against
any kind of user abuse/misuse, but that's not realistic.

Message #29 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Paul Rankin <hello <at> paulwrankin.com>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Tue, 11 Jul 2017 15:07:43 +1000

> On 11 Jul 2017, at 2:48 pm, Tino Calancha <tino.calancha <at> gmail.com> wrote:
> 
> 
> 
>> On Tue, 11 Jul 2017, Paul Rankin wrote:
>> 
>> One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?
> I thought about that, but i discarded because i think binding something
> other that `keyboard-quit' to `C-g' is a misuse.  The Emacs manual is full
> of mentions to `C-g' as `keyboard-quit'.
> There is even the following remark in the tips section:
> 
> "don't bind a key sequence ending in @key{C-g}, since that
> is commonly used to cancel a key sequence."
> 
> If a user want to ignore such kind of advice he/she should
> not expect everything will work the same.

I'm gonna do this just to mess with you 😉

> Maybe we can fix this so that `register-read-with-preview'
> will work with `C-g' bound to `my-cool-foo-command'; but we
> cannot assure that no other Emacs part is affected because such
> misguided `C-g' binding.  We must encourage users to follow
> good practices.

While I think encouragement and enforcement are different things, the point about C-g is more what if the user *also* binds keyboard-quit to "7". In this case the user expects 7 to call keyboard-quit, not just C-g.

Also as Eli said ESC ESC is supposed to keyboard quit I think.

But main thing, as in life, better to look for what what you want than control for what you don't want.

Message #32 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 27634 <at> debbugs.gnu.org,
 Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#27634: 25.2.1; C-g does not quit
 register-read-with-preview
Date: Tue, 11 Jul 2017 14:50:21 +0900 (JST)

[Message part 1 (text/plain, inline)]

On Tue, 11 Jul 2017, Paul Rankin wrote:

>>>
>>> One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?
>> I thought about that, but i discarded because i think binding something
>> other that `keyboard-quit' to `C-g' is a misuse.  The Emacs manual is full
>> of mentions to `C-g' as `keyboard-quit'.
>> There is even the following remark in the tips section:
>>
>> "don't bind a key sequence ending in @key{C-g}, since that
>> is commonly used to cancel a key sequence."
>>
>> If a user want to ignore such kind of advice he/she should
>> not expect everything will work the same.
>
> I'm gonna do this just to mess with you 😉
Thank you.  Actually i feel quite boring now, so it's OK :-)

>> Maybe we can fix this so that `register-read-with-preview'
>> will work with `C-g' bound to `my-cool-foo-command'; but we
>> cannot assure that no other Emacs part is affected because such
>> misguided `C-g' binding.  We must encourage users to follow
>> good practices.
>
>the point about C-g is more what if the user *also* binds keyboard-quit 
>to "7". In this case the user expects 7 to call keyboard-quit, not just 
>C-g.
I see.  Good point!
Paul, what do you think about this?
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -164,6 +164,9 @@ register-read-with-preview
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
+          (when (and (characterp last-input-event)
+                     (eq 'keyboard-quit (key-binding (string last-input-event))))
+            (keyboard-quit))
 	  (if (characterp last-input-event) last-input-event
 	    (error "Non-character input-event")))

I) Note, that my patch won't work in case our fearless user
   bind "7" to ...:
(lambda () (message "What the hell are you doing?")
		      (keyboard-quit))

... But i don't think we must protect about things like I).

Message #35 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> suse.de>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Paul Rankin <hello <at> paulwrankin.com>, 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Tue, 11 Jul 2017 09:20:12 +0200

On Jul 11 2017, Tino Calancha <tino.calancha <at> gmail.com> wrote:

>> the point about C-g is more what if the user *also* binds keyboard-quit
>> to "7". In this case the user expects 7 to call keyboard-quit, not just
>> C-g.

For quit it is irrelevant which key is bound to keyboard-quit, only the
quit char it used.

Andreas.

-- 
Andreas Schwab, SUSE Labs, schwab <at> suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE  1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."

Message #38 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Paul Rankin <hello <at> paulwrankin.com>
Cc: 27634 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Tue, 11 Jul 2017 17:36:09 +0300

> From: Paul Rankin <hello <at> paulwrankin.com>
> Cc: tino.calancha <at> gmail.com, 27634 <at> debbugs.gnu.org
> Date: Tue, 11 Jul 2017 14:14:22 +1000
> 
> On Tue, 11 Jul 2017, at 03:06 AM, Eli Zaretskii wrote:
> > FWIW, I actually agree with Tino's solution, and was about to propose
> > something similar. It's true that control characters are not
> > alphanumeric, but we could fix the documentation to be more accurate
> > if we care about that.  OTOH, we've supported control characters as
> > register names for many years, and by now it should be quite clear it
> > didn't bother anyone yet.
> 
> Hmm, it bothers me?

OK, not "anyone", just "one". ;-)

> But the question is moot I think, since this is an opportunity to improve the code.. why waste time arguing for poorer code when we can make it better?

We are not arguing _whether_ to make it better, we are arguing _how_
to make it better.

> > > That prohibits anything except "a-zA-Z0-9", although users may want to save registers to "$" or "*".
> > 
> > Why would we want to be so strict when the only real problem is that
> > C-g doesn't quit?
> 
> I think there are two good options for good UX: make the code reflect the documentation (this is the strict option), or update both the documentation and the code to reflect what we believe is user expectation, i.e. that the user may save registers to any character key on their keyboard (this is my preference).

I believe the user expectations by now are that any character should
do.  We want to exempt C-g and ESC ESC in order to allow the user to
bail out, but other than that, I see no reason to add restrictions
where they aren't needed.  "Alphanumeric" these days means much more
than just ASCII, and we have no reasons I can see to restrict users to
ASCII, certainly not after so many years of the current behavior.

> One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?

As Andreas points out, only quit char is important.  If we want to be
holier than the Pope, we could look at its value by calling
current-input-method.  I don't object to such an extension.

Message #41 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Paul Rankin <hello <at> paulwrankin.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 27634 <at> debbugs.gnu.org, tino.calancha <at> gmail.com
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Wed, 12 Jul 2017 12:12:27 +1000

On Wed, 12 Jul 2017, at 12:36 AM, Eli Zaretskii wrote:
> I believe the user expectations by now are that any character should
> do.  We want to exempt C-g and ESC ESC in order to allow the user to
> bail out, but other than that, I see no reason to add restrictions
> where they aren't needed.  "Alphanumeric" these days means much more
> than just ASCII, and we have no reasons I can see to restrict users to
> ASCII, certainly not after so many years of the current behavior.
> 
> > One overlooked thing about Tino's solution is that C-g is a keystroke and keyboard-quit is a function, which obviously aren't necessarily equivalent. What if the user remaps keyboard quit to "7"?
> 
> As Andreas points out, only quit char is important.  If we want to be
> holier than the Pope, we could look at its value by calling
> current-input-method.  I don't object to such an extension.

Looks like you guys have this under control 👍

Message #44 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Allen Li <vianchielfaura <at> gmail.com>
To: tino.calancha <at> gmail.com
Cc: bug-gnu-emacs <at> gnu.org
Subject: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Thu, 20 Jul 2017 20:47:32 -0700

[Message part 1 (text/plain, inline)]

It sounds like there's a general consensus on how to fix this.  Tino, would
you care to post a finalized patch?

Note: bug#25370 is a duplicate of this, it should probably be marked
closed, wontfix, etc.

[Message part 2 (text/html, inline)]

Message #49 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Allen Li <vianchielfaura <at> gmail.com>
Cc: Andreas Schwab <schwab <at> suse.de>, Paul Rankin <hello <at> paulwrankin.com>,
 Tino Calancha <tino.calancha <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Fri, 21 Jul 2017 15:19:03 +0900

Allen Li <vianchielfaura <at> gmail.com> writes:

> It sounds like there's a general consensus on how to fix this.  Tino, would you care to post a finalized patch?
OK.
> Note: bug#25370 is a duplicate of this, it should probably be marked closed, wontfix, etc.
Thanks, i merged this thread with your bug report.

Let's discuss following patch:

*) Call `keyboard-quit' whenever last-input-event is
    ?\C-g or 'escape or ?\C-\[.

**) Updated the manual to point out that non-alphanumeric
    keys are valid to store registers as well.

Let me know if that is OK or if we need a fine-grained control.

--8<-----------------------------cut here---------------start------------->8---
commit 484faa217fe5a76d12ce266bc3f84737da73f0ae
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Fri Jul 21 15:08:06 2017 +0900

    register-read-with-preview: Quit if user input C-g or ESC
    
    * lisp/register.el (register-read-with-preview):
    Quit if user input C-g or ESC (bug#27634).
    * doc/emacs/regs.texi (Registers): Update manual.

diff --git a/doc/emacs/regs.texi b/doc/emacs/regs.texi
index 7369f6b05b..40e3e2c1c3 100644
--- a/doc/emacs/regs.texi
+++ b/doc/emacs/regs.texi
@@ -15,7 +15,10 @@ Registers
   Each register has a name that consists of a single character, which
 we will denote by @var{r}; @var{r} can be a letter (such as @samp{a})
 or a number (such as @samp{1}); case matters, so register @samp{a} is
-not the same as register @samp{A}.
+not the same as register @samp{A}.  You can also set a register in
+non-alphanumeric characters, for instance @samp{*} or @samp{C-d}.
+Note, it's not possible to set a register in @samp{C-g} or @samp{ESC},
+because these keys are reserved to terminate interactive commands.
 
 @findex view-register
   A register can store a position, a piece of text, a rectangle, a
diff --git a/lisp/register.el b/lisp/register.el
index 7cc3ccd870..e395963f56 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -164,6 +164,10 @@ register-read-with-preview
 		       help-chars)
 	    (unless (get-buffer-window buffer)
 	      (register-preview buffer 'show-empty)))
+          (when (or (eq ?\C-g last-input-event)
+                    (eq 'escape last-input-event)
+                    (eq ?\C-\[ last-input-event))
+            (keyboard-quit))
 	  (if (characterp last-input-event) last-input-event
 	    (error "Non-character input-event")))
       (and (timerp timer) (cancel-timer timer))
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-07-21
Repository revision: 1d559e384b467b3f74e8b78695f124b561c884d9

Message #52 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: vianchielfaura <at> gmail.com, hello <at> paulwrankin.com, schwab <at> suse.de,
 27634 <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Fri, 21 Jul 2017 11:40:23 +0300

> From: Tino Calancha <tino.calancha <at> gmail.com>
> Cc: 27634 <at> debbugs.gnu.org, Paul Rankin <hello <at> paulwrankin.com>, Andreas Schwab <schwab <at> suse.de>, Eli Zaretskii <eliz <at> gnu.org>, Tino Calancha <tino.calancha <at> gmail.com>
> Date: Fri, 21 Jul 2017 15:19:03 +0900
> 
> Allen Li <vianchielfaura <at> gmail.com> writes:
> 
> > It sounds like there's a general consensus on how to fix this.  Tino, would you care to post a finalized patch?
> OK.
> > Note: bug#25370 is a duplicate of this, it should probably be marked closed, wontfix, etc.
> Thanks, i merged this thread with your bug report.
> 
> Let's discuss following patch:
> 
> *) Call `keyboard-quit' whenever last-input-event is
>     ?\C-g or 'escape or ?\C-\[.
> 
> **) Updated the manual to point out that non-alphanumeric
>     keys are valid to store registers as well.

Looks good, thanks.  (I didn't test the code, but I assume you did.)

Message #55 received at 27634 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: vianchielfaura <at> gmail.com, hello <at> paulwrankin.com, 27634 <at> debbugs.gnu.org,
 schwab <at> suse.de, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#27634: 25.2.1; C-g does not quit
 register-read-with-preview
Date: Fri, 21 Jul 2017 17:55:17 +0900 (JST)

On Fri, 21 Jul 2017, Eli Zaretskii wrote:

>> Let's discuss following patch:
>>
>> *) Call `keyboard-quit' whenever last-input-event is
>>     ?\C-g or 'escape or ?\C-\[.
>>
>> **) Updated the manual to point out that non-alphanumeric
>>     keys are valid to store registers as well.
>
> Looks good, thanks.  (I didn't test the code, but I assume you did.)
Yes i tested it.
Thank you.  I will push it next week if i don't get more communications.

Message #60 received at 27634-done <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 27634-done <at> debbugs.gnu.org
Subject: Re: bug#27634: 25.2.1; C-g does not quit register-read-with-preview
Date: Tue, 25 Jul 2017 11:45:11 +0900

Tino Calancha <tino.calancha <at> gmail.com> writes:

> On Fri, 21 Jul 2017, Eli Zaretskii wrote:
>
>> Looks good, thanks.  (I didn't test the code, but I assume you did.)
> Yes i tested it.
> Thank you.  I will push it next week if i don't get more communications.
Fixed in master branch as commit 35954cb92b8cd4ad093756d171688343bab02c2e

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.