bug#66394: 29.1; Make register-read-with-preview more useful

[Thierry Volpiatto <thievol@posteo.net>]

[-- Attachment #1.1: Type: text/plain, Size: 2322 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>>   stefankangas@gmail.com,  66394@debbugs.gnu.org
>> Date: Wed, 29 Nov 2023 18:18:40 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > The response "a" to the "Point to register:" question is something I
>> > needed to type;
>> 
>> Of course we are using now read-from-minibuffer and the test is related
>> to read-key not quitting with C-g
>> 
>> > previously the test never asked any questions (and it shouldn't,
>> > AFAIU).
>> 
>> Yes, it shouldn't, but now we are using a real minibuffer, is this test
>> really needed?
>> It is like if you had to make tests in all functions using
>> read-from-minibuffer to check if it quit properly with C-g (it does of course).
>
> Can we have a similar issue with read-from-minibuffer?  Not with C-g,
> but with some other invalid input, like some function key or non-ASCII
> character or invalid character codepoint?

No, because unlike read-key, read-from-minibuffer doesn't read these
keys and exit immediately.  For example we can now set a register to C-g
by using "C-q C-g" but hitting directly C-g will quit minibuffer (as
expected), BTW I have updated the manual, see the attached patchs.  I
have also fixed register-preview-default accordingly so that it print
the register as a string but not a key representation e.g. "^X" instead
of "C-x" (if one find this ugly we could use "C-x" as a display property
of "^X").

> If this cannot happen, then perhaps the whole test file should be
> removed?

I think it can be removed, yes (done in patch below).

> Anyway, I trust you to do what is needed here, I just want the test
> suite to keep running successfully, and I don't have time to work on
> this myself.  TIA.

Ok, can you review (and merge if ok) following patchs.

>> ALSO: I intentionally leave register-preview-delay defcustom with a FIXME
>> comment, do you want to remove it? It is not used anymore now AFAIK.
>
> This should be declared obsolete, with an explanation that it is no
> longer used, and a note about that should be added to NEWS, in the
> incompatible changes section.  Then we can remove the FIXME.

done.

Thanks.

-- 
Thierry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Make-register-preview-delay-obsolete.patch --]
[-- Type: text/x-diff, Size: 1914 bytes --]

From 3df81fb5dc5809cab7843e5358c17d0039b55eb1 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Thu, 30 Nov 2023 08:03:45 +0100
Subject: [PATCH 1/4] Make register-preview-delay obsolete

* etc/NEWS: Update.
* lisp/register.el (register-preview-delay): Make it obsolete.
---
 etc/NEWS         | 5 +++++
 lisp/register.el | 2 +-
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index 6661ac70e1b..bab3529339f 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1138,6 +1138,11 @@ showcases all their customization options.
 \f
 * Incompatible Lisp Changes in Emacs 30.1
 
+---
+** 'register-preview-delay' is no longer used.
+Register preview is no more delayed.  If you want to disable it use
+'register-use-preview' instead with a boolean value.
+
 +++
 ** 'M-TAB' now invokes 'completion-at-point' also in Text mode.
 Text mode no longer binds 'M-TAB' to 'ispell-complete-word', and
diff --git a/lisp/register.el b/lisp/register.el
index 61bef503f91..88d0e8e1d10 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -93,7 +93,6 @@ of the marked text."
   :type '(choice (const :tag "None" nil)
 		 (character :tag "Use register" :value ?+)))
 
-;; FIXME: This is no more needed, remove it.
 (defcustom register-preview-delay 1
   "If non-nil, time to wait in seconds before popping up register preview window.
 If nil, do not show register previews, unless `help-char' (or a member of
@@ -101,6 +100,7 @@ If nil, do not show register previews, unless `help-char' (or a member of
   :version "24.4"
   :type '(choice number (const :tag "No preview unless requested" nil))
   :group 'register)
+(make-obsolete-variable 'register-preview-delay "No longer used." "30.1")
 
 (defcustom register-preview-default-keys (mapcar #'string (number-sequence ?a ?z))
   "Default keys for setting a new register."
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-Fix-register-preview-default.patch --]
[-- Type: text/x-diff, Size: 1054 bytes --]

From bcf7bd8feb05914e6000b572c3e75664a9090c1a Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Thu, 30 Nov 2023 10:38:52 +0100
Subject: [PATCH 2/4] Fix register-preview-default

We need to print the string representation (one char) of an eventual
key description e.g. "^X" instead of "C-x".

* lisp/register.el (register-preview-default): Use `string' to print register.
---
 lisp/register.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/register.el b/lisp/register.el
index 88d0e8e1d10..9b457e716f2 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -131,7 +131,7 @@ See the documentation of the variable `register-alist' for possible VALUEs."
 (defun register-preview-default (r)
   "Function that is the default value of the variable `register-preview-function'."
   (format "%s: %s\n"
-	  (single-key-description (car r))
+	  (string (car r))
 	  (register-describe-oneline (car r))))
 
 (defvar register-preview-function #'register-preview-default
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: 0003-Update-register-manual.patch --]
[-- Type: text/x-diff, Size: 1169 bytes --]

From 93c07593bac8d7f0428a92502ee5de8def128c5c Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Thu, 30 Nov 2023 10:53:46 +0100
Subject: [PATCH 3/4] Update register manual

doc/emacs/regs.texi: Do it.
---
 doc/emacs/regs.texi | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/doc/emacs/regs.texi b/doc/emacs/regs.texi
index e52f68dd18e..5e5b7ae2b16 100644
--- a/doc/emacs/regs.texi
+++ b/doc/emacs/regs.texi
@@ -16,9 +16,8 @@ jump back to that position once or many times.
 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}.  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 for quitting (@pxref{Quitting}).
+non-alphanumeric characters, for instance @samp{C-d} by using for
+example @key{C-q} @samp{C-d}.
 
 @findex view-register
   A register can store a position, a piece of text, a rectangle, a
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.5: 0004-Delete-register-tests.el-now-no-more-needed.patch --]
[-- Type: text/x-diff, Size: 2187 bytes --]

From d60e3b4969a420fe9ba66b5732f57243385e5e88 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Thu, 30 Nov 2023 11:04:47 +0100
Subject: [PATCH 4/4] Delete register-tests.el now no more needed

* register-tests.el: Deleted file.
---
 test/lisp/register-tests.el | 43 -------------------------------------
 1 file changed, 43 deletions(-)
 delete mode 100644 test/lisp/register-tests.el

diff --git a/test/lisp/register-tests.el b/test/lisp/register-tests.el
deleted file mode 100644
index 6283d1c31e0..00000000000
--- a/test/lisp/register-tests.el
+++ /dev/null
@@ -1,43 +0,0 @@
-;;; register-tests.el --- tests for register.el  -*- lexical-binding: t-*-
-
-;; Copyright (C) 2017-2023 Free Software Foundation, Inc.
-
-;; Author: Tino Calancha <tino.calancha@gmail.com>
-;; Keywords:
-
-;; This file is part of GNU Emacs.
-
-;; GNU Emacs 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.
-
-;; GNU Emacs 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 GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
-
-;;; Commentary:
-
-\f
-;;; Code:
-(require 'ert)
-(require 'cl-lib)
-
-(ert-deftest register-test-bug27634 ()
-  "Test for https://debbugs.gnu.org/27634 ."
-  (dolist (event (list ?\C-g 'escape ?\C-\[))
-    (cl-letf (((symbol-function 'read-key) #'ignore)
-              (last-input-event event)
-              (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
-- 
2.34.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]