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

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

[Thierry Volpiatto]

When using `copy-to-register`, it is hard to see which register is
already taken in the preview buffer.
This patch highlight the register entered at prompt if it is already
taken otherwise a minibuffer message is sent to notify user the register
is available.
If any interest here is the patch, feel free to modify if needed.
Thanks.

diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..4c83264d4eb 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -154,27 +154,37 @@ listing existing registers after `register-preview-delay' seconds.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-	 (timer (when (numberp register-preview-delay)
-		  (run-with-timer register-preview-delay nil
-				  (lambda ()
-				    (unless (get-buffer-window buffer)
-				      (register-preview buffer))))))
-	 (help-chars (cl-loop for c in (cons help-char help-event-list)
-			      when (not (get-register c))
-			      collect c)))
+         (pat "")
+         result timer)
+    (register-preview buffer)
     (unwind-protect
-	(progn
-	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
-		       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))
+         (progn
+           (minibuffer-with-setup-hook
+               (lambda ()
+                 (setq timer
+                       (run-with-idle-timer
+                        0.3 'repeat
+                        (lambda ()
+                          (with-selected-window (minibuffer-window)
+                            (let ((input (minibuffer-contents)))
+                              (when (not (string= input pat))
+                                (setq pat input))))
+                          (with-current-buffer buffer
+                            (let ((ov (make-overlay (point-min) (point-min))))
+                              (goto-char (point-min))
+                              (if (string= pat "")
+                                  (remove-overlays)
+                                (if (re-search-forward (concat "^" pat) nil t)
+                                    (progn (move-overlay
+                                            ov
+                                            (match-beginning 0) (match-end 0))
+                                           (overlay-put ov 'face 'helm-match))
+                                  (with-selected-window (minibuffer-window)
+                                    (minibuffer-message
+                                     "Register `%s' is available" pat))))))))))
+             (setq result (read-from-minibuffer prompt)))
+           (string-to-char result))
+      (when timer (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))



In GNU Emacs 29.1 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo
version 1.16.0, Xaw3d scroll bars) of 2023-10-01 built on IPad-S340
Windowing system distributor 'The X.Org Foundation', version 11.0.12101004
System Description: Linux Mint 21.2

Configured using:
 'configure CFLAGS=-O8 --bindir=/usr/local/sbin/emacs-29.1 --with-cairo
--with-x-toolkit=lucid --with-modules --without-tree-sitter
--without-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XAW3D XDBE XIM XINPUT2 XPM LUCID ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  locale-coding-system: utf-8-unix

Major mode: 

Minor modes in effect:
  emms-mode-line-mode: t
  emms-playing-time-display-mode: t
  emms-playing-time-mode: t
  bug-reference-prog-mode: t
  server-mode: t
  psession-mode: t
  psession-savehist-mode: t
  global-undo-tree-mode: t
  undo-tree-mode: t
  global-git-gutter-mode: t
  git-gutter-mode: t
  display-time-mode: t
  winner-mode: t
  tv-save-place-mode: t
  helm-epa-mode: t
  helm-descbinds-mode: t
  helm-top-poll-mode: t
  helm-adaptive-mode: t
  helm-mode: t
  helm-minibuffer-history-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-popup-tip-mode: t
  async-bytecomp-package-mode: t
  dired-async-mode: t
  minibuffer-depth-indicate-mode: t
  gcmh-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(epa-mail face-remap addressbook-bookmark tv-mu4e-config config-w3m
mu4e-contrib eshell esh-cmd generator esh-ext esh-opt esh-proc esh-io
esh-arg esh-module esh-groups esh-util mu4e-patch mu4e mu4e-org
org-config ob-gnuplot org-crypt org-protocol org ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
org-list org-footnote org-faces org-entities noutline outline
ob-emacs-lisp ob-core ob-eval org-cycle org-table ol org-fold
org-fold-core org-keys oc org-loaddefs org-version org-compat org-macs
mu4e-notification notifications mu4e-main mu4e-view mu4e-mime-parts
gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls dig gnus-sum
gnus-group gnus-undo gnus-start gnus-dbus gnus-cloud nnimap nnmail
mail-source utf7 nnoo gnus-spec gnus-int gnus-range gnus-win gnus
nnheader range appt diary-lib diary-loaddefs cal-menu calendar
cal-loaddefs mu4e-headers mu4e-thread mu4e-compose mu4e-draft
mu4e-actions smtpmail mu4e-search mu4e-lists mu4e-bookmarks mu4e-mark
mu4e-message shr pixel-fill kinsoku url-file svg dom flow-fill hl-line
mu4e-contacts mu4e-update mu4e-folders mu4e-context mu4e-query-items
mu4e-server mu4e-modeline mu4e-vars mu4e-helpers mu4e-config mu4e-window
ido mu4e-obsolete mailalias mailclient textsec uni-scripts idna-mapping
ucs-normalize uni-confusable textsec-check qp helm-dabbrev shadow
mail-extr emacsbug message yank-media puny rfc822 mml mml-sec gnus-util
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader sendmail rfc2047 rfc2045 ietf-drums mm-util mail-prsvr
mail-utils smerge-mode whitespace cl-extra helm-command helm-x-files
helm-for-files helm-bookmark bookmark emms-config emms-mpris
emms-librefm-stream emms-librefm-scrobbler emms-playlist-limit emms-i18n
emms-history emms-score emms-stream-info emms-metaplaylist-mode
emms-bookmarks emms-cue emms-mode-line-icon emms-browser sort
emms-volume emms-volume-sndioctl emms-volume-mixerctl emms-volume-pulse
emms-volume-amixer emms-playlist-sort emms-last-played emms-player-xine
emms-player-mpd tq emms-lyrics emms-url emms-streams emms-show-all
emms-tag-editor emms-tag-tracktag emms-mark emms-mode-line emms-cache
emms-info-native emms-info-spc bindat emms-info-exiftool
emms-info-tinytag emms-info-metaflac emms-info-opusinfo
emms-info-ogginfo emms-info-mp3info emms-playlist-mode emms-player-vlc
emms-player-mpv emms-playing-time emms-info emms-later-do
emms-player-mplayer emms-player-simple emms-source-playlist
emms-source-file locate emms-setup emms emms-compat emms-auto
helm-external helm-net tramp-archive tramp-gvfs tramp-cache time-stamp
zeroconf dbus xml helm-ring helm-elisp helm-eval edebug debug backtrace
find-func helm-info cl-indent helm-ls-git vc-git diff-mode vc
vc-dispatcher jka-compr make-mode flymake-shellcheck cus-start
flymake-proc flymake project warnings thingatpt sh-script smie treesit
executable bug-reference naquadah-theme server imenu psession frameset
undo-tree diff queue pcase git-gutter mule-util dired-extension time
winner describe-variable help-fns radix-tree help-mode tv-utils
tv-save-place.el advice init-helm epa derived epg rfc6068 epg-config
helm-epa isl helm-descbinds cus-edit pp icons wid-edit helm-sys popup
helm-adaptive helm-mode helm-misc helm-files image-dired
image-dired-tags image-dired-external image-dired-util xdg image-mode
exif filenotify tramp tramp-loaddefs trampver tramp-integration files-x
tramp-compat rx shell pcomplete parse-time iso8601 time-date
helm-buffers all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons helm-occur helm-tags helm-locate helm-grep wgrep-helm
wgrep grep compile text-property-search comint ansi-osc ring helm-regexp
format-spec ansi-color helm-utils helm-help helm-types
helm-extensions-autoloads helm-autoloads helm helm-global-bindings
helm-easymenu edmacro kmacro helm-core easy-mmode async-bytecomp
helm-source helm-multi-match helm-lib dired-async async dired-aux dired
dired-loaddefs mb-depth avoid cus-load gcmh all-the-icons-autoloads
gcmh-autoloads info ledger-mode-autoloads markdown-mode-autoloads
nerd-icons-autoloads w3m-load w3m-autoloads yaml-mode-autoloads package
browse-url url url-proxy url-privacy url-expand url-methods url-history
url-cookie generate-lisp-file url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs password-cache
json subr-x map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs
cl-lib rmc iso-transl tooltip cconv eldoc paren electric uniquify
ediff-hook vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify lcms2 dynamic-setting system-font-setting
font-render-setting cairo x-toolkit xinput2 x multi-tty
make-network-process emacs)

Memory information:
((conses 16 854462 334476)
 (symbols 48 35404 26)
 (strings 32 219034 39971)
 (string-bytes 1 6357489)
 (vectors 16 99232)
 (vector-slots 8 2128904 396764)
 (floats 8 1791 2270)
 (intervals 56 37515 32129)
 (buffers 976 138))
<#secure method=pgpmime mode=sign>

-- 
Thierry

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


Here a modified version of the patch that fix the face, send an error
when exiting with empty prompt and prevent adding more than one char in
prompt.
From this code, it is now easy to modify the behavior as needed (ideas welcome).

diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..47645098e6d 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -154,27 +154,45 @@ listing existing registers after `register-preview-delay' seconds.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-	 (timer (when (numberp register-preview-delay)
-		  (run-with-timer register-preview-delay nil
-				  (lambda ()
-				    (unless (get-buffer-window buffer)
-				      (register-preview buffer))))))
-	 (help-chars (cl-loop for c in (cons help-char help-event-list)
-			      when (not (get-register c))
-			      collect c)))
+         (pat "")
+         result timer)
+    (register-preview buffer)
     (unwind-protect
-	(progn
-	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
-		       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))
+         (progn
+           (minibuffer-with-setup-hook
+               (lambda ()
+                 (setq timer
+                       (run-with-idle-timer
+                        0.3 'repeat
+                        (lambda ()
+                          (with-selected-window (minibuffer-window)
+                            (setq minibuffer-scroll-window
+                                  (get-buffer-window buffer))
+                            (let ((input (minibuffer-contents)))
+                              (when (> (length input) 1)
+                                (setq input (substring input 0 1))
+                                (delete-minibuffer-contents)
+                                (insert input))
+                              (when (not (string= input pat))
+                                (setq pat input))))
+                          (with-current-buffer buffer
+                            (let ((ov (make-overlay (point-min) (point-min))))
+                              (goto-char (point-min))
+                              (if (string= pat "")
+                                  (remove-overlays)
+                                (if (re-search-forward (concat "^" pat) nil t)
+                                    (progn (move-overlay
+                                            ov
+                                            (match-beginning 0) (match-end 0))
+                                           (overlay-put ov 'face 'match))
+                                  (with-selected-window (minibuffer-window)
+                                    (minibuffer-message
+                                     "Register `%s' is available" pat))))))))))
+             (setq result (read-from-minibuffer prompt)))
+           (cl-assert (and result (not (string= result "")))
+                      nil "No register specified")
+           (string-to-char result))
+      (when timer (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))

-- 
Thierry

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

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


I moved the code here if any interest.
https://gist.github.com/thierryvolpiatto/2219f99ac96ed1b468fac204bca23b4a

I added filters to register-preview (jump-to-register should show
only positions and insert-register should show only text).
Also added navigation (up and down) in preview buffer (see gif).

Still need to modify (or add) docstrings and also decide what to do with
`register-preview-delay` (just a flag in this patch).
AFAIU the only reason to delay preview is when executing kbd macro, in
this case we could use executing-kbd-macro, but maybe I miss something
and there is other reasons to delay this.

-- 
Thierry

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

bug#66394: [RE] Make register-read-with-preview more useful

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

Thanks for inviting people to work with you on improving this code.
But please don't use github as the place to host your free software --
especially not if it relates to GNU.

Github requires a person to run nonfree software simply to make an
account; so if you invite people to collaborate with you on that
platform, indirectly that asks people to run nonfree software.

See https://www.gnu.org/software/repo-criteria-evaluation.html.


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

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


Richard Stallman <rms@gnu.org> writes:

> Thanks for inviting people to work with you on improving this code.
> But please don't use github as the place to host your free software --
> especially not if it relates to GNU.
>
> Github requires a person to run nonfree software simply to make an
> account; so if you invite people to collaborate with you on that
> platform, indirectly that asks people to run nonfree software.

I don't invite anybody to collaborate on github.
The code is stored there and AFAIK anybody can look at it without creating an
account, if any interest in this code I will provide a patch on this
list, what I did in previous mails without any answer BTW...

-- 
Thierry

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

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


Closed for no interest in this feature.

-- 
Thierry

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

bug#66394: [RE] Make register-read-with-preview more useful

[Stefan Kangas]

Thierry Volpiatto <thievol@posteo.net> writes:

> Closed for no interest in this feature.

The lack of a reply doesn't mean that there is a lack of interest.  As
for myself, I've been too busy to find the time to review it in the last
week.  I also have many other things on my plate.

In the future, please allow for, at minimum, 2-4 weeks to let people
comment, and then feel free to ping us if you still didn't get a reply.

If it's fine by you, I'd like to reopen this bug report.

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


Stefan Kangas <stefankangas@gmail.com> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Closed for no interest in this feature.
>
> The lack of a reply doesn't mean that there is a lack of interest.  As
> for myself, I've been too busy to find the time to review it in the last
> week.  I also have many other things on my plate.
>
> In the future, please allow for, at minimum, 2-4 weeks to let people
> comment, and then feel free to ping us if you still didn't get a reply.
>
> If it's fine by you, I'd like to reopen this bug report.

Ok, please do.

Thanks.

-- 
Thierry

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

bug#66394: [RE] Make register-read-with-preview more useful

[Stefan Kangas]

reopen 66394
thanks

Thierry Volpiatto <thievol@posteo.net> writes:

>> If it's fine by you, I'd like to reopen this bug report.
>
> Ok, please do.

Done, thanks.

bug#66394: [RE] Make register-read-with-preview more useful

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > I don't invite anybody to collaborate on github.
  > The code is stored there and AFAIK anybody can look at it without creating an
  > account, if any interest in this code I will provide a patch on this
  > list, what I did in previous mails without any answer BTW...

That is not as bad.  But if the code is only to look at, would it work
just as well to refer people to the code in the ELPA repo?
be an equally good place to refer people to?

Ifthat method has some drawback compared with referring to github, can
you describe the drawback?

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> When using `copy-to-register`, it is hard to see which register is
> already taken in the preview buffer.  This patch highlight the
> register entered at prompt if it is already taken otherwise a
> minibuffer message is sent to notify user the register is available.
> If any interest here is the patch, feel free to modify if needed.

I am not sure what's the best way to address this kind of problem.  If
your version is accepted, I would vote for an option to get the old
behavior back.  Your intended behavior is safer but requires more keys
(at least confirmation with RET).  Some people might prefer the old way.

I'm also not sure about the visual feedback.  If you use lots of
registers you might miss your register highlighting in the preview
buffer.  Maybe using the minibuffer always for the visual feedback might
be better, I don't know.  Or give only feedback when the register is
already taken?  Or maybe require the user to hit RET two times to
confirm overwriting?

Personally I hacked the code so that I can lock registers I don't want
to overwrite.  I can also restore registers.  That takes away a bit of
the pressure.

There also had been a request to be able to delete register bindings,
but it had been rejected.

In any way there should be some way to allow a cleaner working with
registers.

Just my two cents.


Michael.

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

[Thierry Volpiatto]

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


Hello Michael,

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> When using `copy-to-register`, it is hard to see which register is
>> already taken in the preview buffer.  This patch highlight the
>> register entered at prompt if it is already taken otherwise a
>> minibuffer message is sent to notify user the register is available.
>> If any interest here is the patch, feel free to modify if needed.
>
> I am not sure what's the best way to address this kind of problem.  If
> your version is accepted, I would vote for an option to get the old
> behavior back.  Your intended behavior is safer but requires more keys
> (at least confirmation with RET).  Some people might prefer the old way.

There is only RET as additional key and it is a good thing IMO as it let
the time to user to see what he is doing.  Anyway using a real
minibuffer with its keymap is much better and allows further
modifications in the future to fit the needs of everybody. Using
read-key doesn't allow more alternatives.

> I'm also not sure about the visual feedback.  If you use lots of
> registers you might miss your register highlighting in the preview
> buffer.

It's easy to make the selection always visible, now fixed, thanks.

> Maybe using the minibuffer always for the visual feedback might be
> better, I don't know.  Or give only feedback when the register is
> already taken?  Or maybe require the user to hit RET two times to
> confirm overwriting?

Don't think it is necessary with the register highlighting, and with the
real minibuffer, we must hit RET at least one time to exit wich act as a
confirmation (previously read-key was exiting immediately).
>
> Personally I hacked the code so that I can lock registers I don't want
> to overwrite.  I can also restore registers.  That takes away a bit of
> the pressure.

Ok, that's another approach but doesn't help to see what is available or
not.
Note that now you can use M-n to select in minibuffer the available keys (this
only for setting or modifying a register).

> There also had been a request to be able to delete register bindings,
> but it had been rejected.

You can delete your registers with helm, but this is unrelated to this
thread.

> In any way there should be some way to allow a cleaner working with
> registers.

Probably what I propose is not perfect but it is a first step to have
something better than what we have actually.

Thanks for your feedback.

> Just my two cents.
>
>
> Michael.


-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> There is only RET as additional key and it is a good thing IMO as it let
> the time to user to see what he is doing.  Anyway using a real
> minibuffer with its keymap is much better and allows further
> modifications in the future to fit the needs of everybody. Using
> read-key doesn't allow more alternatives.

For keys like C-a you also need to hit C-q.  It's not 100% compatible.

But wait: What I find confusing is that I also need need to confirm for
`jump-to-register'.  Is this intended?


> Note that now you can use M-n to select in minibuffer the available
> keys (this only for setting or modifying a register).

In Helm or in vanilla Emacs?  I don't see that for M-n in vanilla Emacs.

Oh, and there is a little bug when the register binding list is empty
(e.g. after restarting Emacs): your code errors because Emacs does not
pop up a preview window in that case.


Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> There is only RET as additional key and it is a good thing IMO as it let
>> the time to user to see what he is doing.  Anyway using a real
>> minibuffer with its keymap is much better and allows further
>> modifications in the future to fit the needs of everybody. Using
>> read-key doesn't allow more alternatives.
>
> For keys like C-a you also need to hit C-q.  It's not 100% compatible.

I don't understand, what C-a and C-q have to do here? Also, what C-q is
intended to do in minibuffer?

> But wait: What I find confusing is that I also need need to confirm for
> `jump-to-register'.  Is this intended?

Do you mean RET? If so yes.

>
>> Note that now you can use M-n to select in minibuffer the available
>> keys (this only for setting or modifying a register).
>
> In Helm or in vanilla Emacs?  I don't see that for M-n in vanilla Emacs.

Once the patch is applied, C-x r x M-n (repeat if necessary), same for
C-x r w/n etc...

> Oh, and there is a little bug when the register binding list is empty
> (e.g. after restarting Emacs): your code errors because Emacs does not
> pop up a preview window in that case.

I think you mean when hitting C-n/p or up/down?
It is fixed in last version of the patch (not publied yet).

>
> Michael.


-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> > For keys like C-a you also need to hit C-q.  It's not 100%
> > compatible.
>
> I don't understand, what C-a and C-q have to do here? Also, what C-q
> is intended to do in minibuffer?

Registers are characters.  Control characters are valid registers.  So
you can for example do C-x r s C-a to save the region string into register
`C-a'.  Your patch complicates inputting such registers.  Dunno if
people use such registers, but I wanted to mention this.  It's a bit
harder now to use non-printable characters as registers now.


> > But wait: What I find confusing is that I also need need to confirm for
> > `jump-to-register'.  Is this intended?
>
> Do you mean RET? If so yes.

For jumping?  Why is this useful?


> >> Note that now you can use M-n to select in minibuffer the available
> >> keys (this only for setting or modifying a register).
> >
> > In Helm or in vanilla Emacs?  I don't see that for M-n in vanilla Emacs.
>
> Once the patch is applied, C-x r x M-n (repeat if necessary), same for
> C-x r w/n etc...

Is this part in the patch you had been posting in the first messages?
Because I only get "End of history; no default available" with that
patch installed.


> > Oh, and there is a little bug when the register binding list is empty
> > (e.g. after restarting Emacs): your code errors because Emacs does not
> > pop up a preview window in that case.
>
> I think you mean when hitting C-n/p or up/down?
> It is fixed in last version of the patch (not publied yet).

Let me try to be more precise what I saw: If I start Emacs modified with
your patch and do C-x r s with an active region, I see this message in
the minibuffer:

  Error running timer: (error "No buffer named *Register Preview*")

Maybe that's already what you have fixed.

One more detail: I see "Invalid face reference: helm-match" in the
*Messages* of emacs -Q.  Still using your first patch (Could you please
post the newest version again?).


Thanks so far,

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> > For keys like C-a you also need to hit C-q.  It's not 100%
>> > compatible.
>>
>> I don't understand, what C-a and C-q have to do here? Also, what C-q
>> is intended to do in minibuffer?
>
> Registers are characters.  Control characters are valid registers.  So
> you can for example do C-x r s C-a to save the region string into register
> `C-a'.  Your patch complicates inputting such registers.  Dunno if
> people use such registers, but I wanted to mention this.  It's a bit
> harder now to use non-printable characters as registers now.

Yes, probably this is one downside of this patch, but as you mentionned
one can use C-q C-a if really needed, I for one never used such
registers.

>
>> > But wait: What I find confusing is that I also need need to confirm for
>> > `jump-to-register'.  Is this intended?
>>
>> Do you mean RET? If so yes.
>
> For jumping?  Why is this useful?

Well, actually with original behavior you can jump to a register
recorded as a string, which returns an error of course because the
register is meant to use with insert.  Now the situation is better
because the candidates are filtered but you can still jump to an
unwanted place, read-from-minibuffer lets you the time to see where you
are going.

>
>> >> Note that now you can use M-n to select in minibuffer the available
>> >> keys (this only for setting or modifying a register).
>> >
>> > In Helm or in vanilla Emacs?  I don't see that for M-n in vanilla Emacs.
>>
>> Once the patch is applied, C-x r x M-n (repeat if necessary), same for
>> C-x r w/n etc...
>
> Is this part in the patch you had been posting in the first messages?
> Because I only get "End of history; no default available" with that
> patch installed.

No, I will prepare a patch later, when there is interest in this
feature, for now use the gist I maintain here as mentionned previously:

https://gist.github.com/thierryvolpiatto/2219f99ac96ed1b468fac204bca23b4a

Thanks.

-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> > For jumping?  Why is this useful?
>
> Well, actually with original behavior you can jump to a register
> recorded as a string, which returns an error of course because the
> register is meant to use with insert.  Now the situation is better
> because the candidates are filtered but you can still jump to an
> unwanted place, read-from-minibuffer lets you the time to see where you
> are going.

Ah - ok.  Then that's definitely useful.  Haven't yet tried the complete
code, but I had a look at the link you posted.

It would not be good to carve the current register type system
into stone.  Instead of your `register-type' function I would prefer
something extensible, for the case of new register types being added
(even a normal user might want to do this).  So, when one wants to add a
new register type, it is necessary that one is able to declare, in some
way, whether registers of this type should be included or not for
`insert-register' (and maybe also `jump-to-register').

Instead of a detour via type names a better way seems to be adding new
predicate methods that accept one argument, a register, and should
return non-nil when the argument register can be inserted, or jumped to.

I can try to modify your patch accordingly if you are interested.


Thx,

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> > For jumping?  Why is this useful?
>>
>> Well, actually with original behavior you can jump to a register
>> recorded as a string, which returns an error of course because the
>> register is meant to use with insert.  Now the situation is better
>> because the candidates are filtered but you can still jump to an
>> unwanted place, read-from-minibuffer lets you the time to see where you
>> are going.
>
> Ah - ok.  Then that's definitely useful.  Haven't yet tried the complete
> code, but I had a look at the link you posted.
>
> It would not be good to carve the current register type system
> into stone.  Instead of your `register-type' function I would prefer
> something extensible, for the case of new register types being added
> (even a normal user might want to do this).  So, when one wants to add a
> new register type, it is necessary that one is able to declare, in some
> way, whether registers of this type should be included or not for
> `insert-register' (and maybe also `jump-to-register').
>
> Instead of a detour via type names a better way seems to be adding new
> predicate methods that accept one argument, a register, and should
> return non-nil when the argument register can be inserted, or jumped to.

Not sure to understand what you envisage here, but if you do something
like this you have to make the types used for the two register commands
configurable as well.
Actually we have insert-register that accept '(string number) types and
jump-to-register that accept '(window frame marker) types and this is
hardcoded. I guess you want to make register-type a generic function and
add several method that fit the types we actually have and if one wants
to add a new type he just have to write a new defmethod and add the new
type to the methods suitable for insert or jump.

> I can try to modify your patch accordingly if you are interested.

Yes but I think this should go into another commit on top of my patch to
avoid having a "too big patch".

Thanks.

-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> Yes but I think this should go into another commit on top of my patch
> to avoid having a "too big patch".

Yes, maybe.

Ok, some more remarks (I have looked at that url now...):

* Would it make sense to also allow M-n (known as "future history" for
  `completing-read') to gather the next free register (seems also
  natural - just a thought)

* Multiple times I saw "contains no text" where a better hint would have
  been to say "empty".  Not all registers contain text.  Happens
  e.g. for C-x r SPC R where R is any unused register.

* register-preview-default-keys may be initialized using
  (mapcar #'string (number-sequence ?a ?z)) if you like.


Hope we get more opinions about this interface suggestion.


Thx,

Michael.

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

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Hope we get more opinions about this interface suggestion.

Adding Evgenii to the thread.  AFAIR he had suggested a subset of this
suggestion (the filtering part, in July in emacs-devel).  Maybe he feels
like trying Thierry's approach.  Code is here, just eval if interested:

  https://gist.github.com/thierryvolpiatto/2219f99ac96ed1b468fac204bca23b4a

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Yes but I think this should go into another commit on top of my patch
>> to avoid having a "too big patch".
>
> Yes, maybe.
>
> Ok, some more remarks (I have looked at that url now...):
>
> * Would it make sense to also allow M-n (known as "future history" for
>   `completing-read') to gather the next free register (seems also
>   natural - just a thought)

It is already done ;-)

> * Multiple times I saw "contains no text" where a better hint would have
>   been to say "empty".  Not all registers contain text.  Happens
>   e.g. for C-x r SPC R where R is any unused register.

Yes I used the message vanilla Emacs already use somewhere, but yes
empty seems better.

> * register-preview-default-keys may be initialized using
>   (mapcar #'string (number-sequence ?a ?z)) if you like.

Yes could be done. 

PS: I found a bug in my code with frame configs not appearing in preview,
now fixed, will update the gist right now with your suggestions as well.

Thanks.

-- 
Thierry

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

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> It would not be good to carve the current register type system
> into stone.  Instead of your `register-type' function I would prefer
> something extensible, for the case of new register types being added
> (even a normal user might want to do this).  So, when one wants to add a
> new register type, it is necessary that one is able to declare, in some
> way, whether registers of this type should be included or not for
> `insert-register' (and maybe also `jump-to-register').

A possible solution for this is adding two vars, insert-register-types
and jump-to-register-types and define register-type like this (named
register--type here):

(defun register--type (register)
  ;; Call register/type against the register value.
  (register/type (if (consp (cdr register))
                     (cadr register)
                   (cdr register))))

(cl-defgeneric register/type (regval))

(cl-defmethod register/type ((regval string)) 'string)
(cl-defmethod register/type ((regval number)) 'number)
(cl-defmethod register/type ((regval marker)) 'marker)
(cl-defmethod register/type ((regval window-configuration)) 'window)
(cl-deftype frame-register () '(satisfies frameset-register-p))
(cl-defmethod register/type :extra "frame-register" (regval) 'frame)

;; set a new register and check its type like this:
(register--type (car register-alist))

So if one wants a new register type he has just to add the type to one
of insert-register-types or jump-to-register-types and add a new
defmethod for this type. If the new type in not one of cl-typep he may
have to define a new type like above.


-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> > * Multiple times I saw "contains no text" where a better hint would have
> >   been to say "empty".  Not all registers contain text.  Happens
> >   e.g. for C-x r SPC R where R is any unused register.
>
> Yes I used the message vanilla Emacs already use somewhere, but yes
> empty seems better.

Thanks.  In register.el I see a message like that only for text related
register commands.


Michael.

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> A possible solution for this is adding two vars, insert-register-types
> and jump-to-register-types and define register-type like this (named
> register--type here):
>
> (defun register--type (register)
>   ;; Call register/type against the register value.
>   (register/type (if (consp (cdr register))
>                      (cadr register)
>                    (cdr register))))
>
> (cl-defgeneric register/type (regval))
>
> (cl-defmethod register/type ((regval string)) 'string)
> (cl-defmethod register/type ((regval number)) 'number)
> (cl-defmethod register/type ((regval marker)) 'marker)
> (cl-defmethod register/type ((regval window-configuration)) 'window)
> (cl-deftype frame-register () '(satisfies frameset-register-p))
> (cl-defmethod register/type :extra "frame-register" (regval) 'frame)
>
> ;; set a new register and check its type like this:
> (register--type (car register-alist))

This looks promising.

But I'm not sure whether the detour via type names (instead of an
approach using only generics) is the best solution.  Why not define just
a new generic (register-eligible-for-command-p REG COMMAND) and use that
as predicate in the interactive specs of the commands (providing
`this-command' as second arg)?  Seems simpler to me and still more
extensible and controllable.

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> A possible solution for this is adding two vars, insert-register-types
>> and jump-to-register-types and define register-type like this (named
>> register--type here):
>>
>> (defun register--type (register)
>>   ;; Call register/type against the register value.
>>   (register/type (if (consp (cdr register))
>>                      (cadr register)
>>                    (cdr register))))
>>
>> (cl-defgeneric register/type (regval))
>>
>> (cl-defmethod register/type ((regval string)) 'string)
>> (cl-defmethod register/type ((regval number)) 'number)
>> (cl-defmethod register/type ((regval marker)) 'marker)
>> (cl-defmethod register/type ((regval window-configuration)) 'window)
>> (cl-deftype frame-register () '(satisfies frameset-register-p))
>> (cl-defmethod register/type :extra "frame-register" (regval) 'frame)
>>
>> ;; set a new register and check its type like this:
>> (register--type (car register-alist))
>
> This looks promising.
>
> But I'm not sure whether the detour via type names (instead of an
> approach using only generics) is the best solution.  Why not define just
> a new generic (register-eligible-for-command-p REG COMMAND) and use that
> as predicate in the interactive specs of the commands (providing
> `this-command' as second arg)?  Seems simpler to me and still more
> extensible and controllable.

Not sure to understand what you want to do here.

We don't want to modify each command, and anyway I don't see what
(register-eligible-for-command-p REG COMMAND) would do in the
interactive spec of each command.

There is two things we want to make more flexible:

1) The ability to allow adding a new type of register and
use this type of register to filter out the register-alist according to
the command in use.

2) Allow one to add a new register command
and assign its type, message to use and action it
provide.

Example, if I want to define a new command register-delete:

    (defun register-delete (register)
      (interactive (list (register-read-with-preview "Delete register: ")))
      (setq register-alist (delete register register-alist)))

If I run this command I will have a minibuffer message "Overwrite
register <n>", this is not what I expect, I expect "Delete register
<n>".

So we need something to customize this.
I added a new var and a structure to achieve this, so now one can do in
e.g. its .emacs:

    (with-eval-after-load 'register
      (require 'register-preview)
      (add-to-list 'register-commands-data
                   `(register-delete
                     .
                     ,(make-register-preview-commands
                       :types '(all)
                       :msg "Delete register `%s'"
                       :act 'delete)))
      (defun register-delete (register)
        (interactive (list (register-read-with-preview "Delete register: ")))
        (setq register-alist (delete register register-alist))))


But maybe what you propose is simpler to achieve the two tasks above,
don't know.

-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> Not sure to understand what you want to do here.

Maybe it's easier if I post a patch.  Will try to do this tomorrow.

What I want to do is that `register-read-with-preview' accepts another
argument, a predicate, that is used to filter the registers to be
included in the preview.

And the hierarchy defining which registers support which operations I
want to define implicitly using method specializers, not explicitly
using types.

Because I think using generics is more flexible.  When the type system
you propose defines, for example, that string registers can be inserted,
and appendend and prepended to (we have commands for that), and
then, say, somebody wants to define a wrapper register type "read-only
register" that can hold anything a register can hold but with the
difference that the read-only register can't be modified, you want to
fall back to the filtering of the base type (which can be anything, so
this has to be done dynamically) - but disallow append and prepend for
string registers, for example.

Just an example, but such wrappers can be useful for registers.  You can
define registers with annotations for example: the annotations are
displayed in the preview but don't affect the behavior of the base
register.

Such things are hard to code using a type system.  Of course there are
more changes to be made to allow everything I outlined, but I want that
we don't do anything that limits what could be added to register.el in
the future.

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Not sure to understand what you want to do here.
>
> Maybe it's easier if I post a patch.  Will try to do this tomorrow.

Yes, not sure to understand what you want to do.

> What I want to do is that `register-read-with-preview' accepts another
> argument, a predicate, that is used to filter the registers to be
> included in the preview.

Why do you need another argument as long as you use `this-command`?
You can use e.g. (pcase this-command ('foo #'foo-p) etc...)
Isn't it?

-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> Why do you need another argument as long as you use `this-command`?
> You can use e.g. (pcase this-command ('foo #'foo-p) etc...)

I doubt all potential uses will use `this-command'.

It's cleaner if the command that knows what it wants passes the
information via argument than to make the other function derive it
indirectly from the context (functional style).

Maybe in the future we might need to pass other predicates as well.
What when a user wants to add another register command?  Or if a future
command needs to prompt more than once?

A predicate passed to the function can also be wrapped for further
filtering etc... that's all more controllable and extensible than hiding
the decision in a defun.

Michael.

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

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Why do you need another argument as long as you use `this-command`?
>> You can use e.g. (pcase this-command ('foo #'foo-p) etc...)
>
> I doubt all potential uses will use `this-command'.
>
> It's cleaner if the command that knows what it wants passes the
> information via argument than to make the other function derive it
> indirectly from the context (functional style).

It is what I did initially after realizing it is simpler to use
this-command from register-read-with-preview.

> Maybe in the future we might need to pass other predicates as well.
> What when a user wants to add another register command?

You can with the current register-preview.el add a new command and
control filtering of this command.

-- 
Thierry

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

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

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> > Maybe in the future we might need to pass other predicates as well.
> > What when a user wants to add another register command?
>
> You can with the current register-preview.el add a new command and
> control filtering of this command.

Cool, thanks.  Looks reasonable.

In my perfect world, the `register-commands-data' would not be specified
as a list but also using methods.  Maybe also
`register-preview-get-defaults'.

I will have a closer look tomorrow, had been a little busy so that I
just now returned to this thread.


Regards,

Michael.

bug#66394: [RE] Make register-read-with-preview more useful

[Thierry Volpiatto]

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


Hello Stefan,

Stefan Kangas <stefankangas@gmail.com> writes:

> reopen 66394
> thanks
>
> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>> If it's fine by you, I'd like to reopen this bug report.
>>
>> Ok, please do.
>
> Done, thanks.

More than one month now, what do you want to do with this?
I had constructive exchanges with Michael and the code has been updated
according to those discussions...

Thanks.

-- 
Thierry

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

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

[Thierry Volpiatto]

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

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> > Maybe in the future we might need to pass other predicates as well.
>> > What when a user wants to add another register command?
>>
>> You can with the current register-preview.el add a new command and
>> control filtering of this command.
>
> Cool, thanks.  Looks reasonable.
>
> In my perfect world, the `register-commands-data' would not be specified
> as a list but also using methods.  Maybe also
> `register-preview-get-defaults'.
>
> I will have a closer look tomorrow, had been a little busy so that I
> just now returned to this thread.

Here all the work done so far.

diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..78643d10c17 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -35,6 +35,8 @@
 
 ;; FIXME: Clean up namespace usage!
 
+(declare-function frameset-register-p "frameset")
+
 (cl-defstruct
   (registerv (:constructor nil)
 	     (:constructor registerv--make (&optional data print-func
@@ -91,6 +93,7 @@ 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
@@ -99,6 +102,14 @@ If nil, do not show register previews, unless `help-char' (or a member of
   :type '(choice number (const :tag "No preview unless requested" nil))
   :group 'register)
 
+(defcustom register-preview-default-keys (mapcar #'string (number-sequence ?a ?z))
+  "Default keys for setting a new register."
+  :type '(repeat string))
+
+(defcustom register-use-preview t
+  "Always show register preview when non nil."
+  :type 'boolean)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (alist-get register register-alist))
@@ -128,53 +139,263 @@ See the documentation of the variable `register-alist' for possible VALUEs."
 Called with one argument, a cons (NAME . CONTENTS) as found in `register-alist'.
 The function should return a string, the description of the argument.")
 
-(defun register-preview (buffer &optional show-empty)
+(cl-defstruct register-preview-info
+  "Store data for a specific register command.
+TYPES are the types of register supported.
+MSG is the minibuffer message to send when a register is selected.
+ACT is the type of action the command is doing on register.
+SMATCH accept a boolean value to say if command accept non matching register."
+  types msg act smatch)
+
+(cl-defgeneric register-command-info (command)
+  "Returns a `register-preview-info' object storing data for COMMAND."
+  (ignore command))
+(cl-defmethod register-command-info ((_command (eql insert-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Insert register `%s'"
+   :act 'insert
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql jump-to-register)))
+  (make-register-preview-info
+   :types  '(window frame marker kmacro
+             file buffer file-query)
+   :msg "Jump to register `%s'"
+   :act 'jump
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql view-register)))
+  (make-register-preview-info
+   :types '(all)
+   :msg "View register `%s'"
+   :act 'view
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql append-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Append to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql prepend-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Prepend to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql increment-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Increment register `%s'"
+   :act 'modify
+   :smatch t))
+
+(defun register-preview-forward-line (arg)
+  "Move to next or previous line in register preview buffer.
+If ARG is positive goto next line, if negative to previous.
+Do nothing when defining or executing kmacros."
+  ;; Ensure user enter manually key in minibuffer when recording a macro.
+  (unless (or defining-kbd-macro executing-kbd-macro
+              (not (get-buffer-window "*Register Preview*" 'visible)))
+    (let ((fn (if (> arg 0) #'eobp #'bobp))
+          (posfn (if (> arg 0)
+                     #'point-min
+                     (lambda () (1- (point-max)))))
+          str)
+      (with-current-buffer "*Register Preview*"
+        (let ((ovs (overlays-in (point-min) (point-max)))
+              pos)
+          (goto-char (if ovs
+                         (overlay-start (car ovs))
+                         (point-min)))
+          (setq pos (point))
+          (and ovs (forward-line arg))
+          (when (and (funcall fn)
+                     (or (> arg 0) (eql pos (point))))
+            (goto-char (funcall posfn)))
+          (setq str (buffer-substring-no-properties
+                     (pos-bol) (1+ (pos-bol))))
+          (remove-overlays)
+          (with-selected-window (minibuffer-window)
+            (delete-minibuffer-contents)
+            (insert str)))))))
+
+(defun register-preview-next ()
+  "Goto next line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line 1))
+
+(defun register-preview-previous ()
+  "Goto previous line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line -1))
+
+(defun register-type (register)
+  "Return REGISTER type.
+Current register types actually returned are one of:
+- string
+- number
+- marker
+- buffer
+- file
+- file-query
+- window
+- frame
+- kmacro
+
+One can add new types to a specific command by defining a new `cl-defmethod'
+matching this command. Predicate for type in new `cl-defmethod' should
+satisfy `cl-typep' otherwise the new type should be defined with
+`cl-deftype'."
+  ;; Call register--type against the register value.
+  (register--type (if (consp (cdr register))
+                     (cadr register)
+                   (cdr register))))
+
+(cl-defgeneric register--type (regval)
+  "Returns type of register value REGVAL."
+  (ignore regval))
+
+(cl-defmethod register--type ((_regval string)) 'string)
+(cl-defmethod register--type ((_regval number)) 'number)
+(cl-defmethod register--type ((_regval marker)) 'marker)
+(cl-defmethod register--type ((_regval (eql 'buffer))) 'buffer)
+(cl-defmethod register--type ((_regval (eql 'file))) 'file)
+(cl-defmethod register--type ((_regval (eql 'file-query))) 'file-query)
+(cl-defmethod register--type ((_regval window-configuration)) 'window)
+(cl-deftype frame-register () '(satisfies frameset-register-p))
+(cl-defmethod register--type :extra "frame-register" (_regval) 'frame)
+(cl-deftype kmacro-register () '(satisfies kmacro-register-p))
+(cl-defmethod register--type :extra "kmacro-register" (_regval) 'kmacro)
+
+(defun register-of-type-alist (types)
+  "Filter `register-alist' according to TYPES."
+  (if (memq 'all types)
+      register-alist
+    (cl-loop for register in register-alist
+             when (memq (register-type register) types)
+             collect register)))
+
+(defun register-preview-1 (buffer &optional show-empty types)
   "Pop up a window showing the registers preview in BUFFER.
 If SHOW-EMPTY is non-nil, show the window even if no registers.
+Argument TYPES (a list) specify the types of register to show, when nil show all
+registers, see `register-type' for suitable types. 
 Format of each entry is controlled by the variable `register-preview-function'."
-  (when (or show-empty (consp register-alist))
-    (with-current-buffer-window
-     buffer
-     (cons 'display-buffer-below-selected
-	   '((window-height . fit-window-to-buffer)
-	     (preserve-size . (nil . t))))
-     nil
-     (with-current-buffer standard-output
-       (setq cursor-in-non-selected-windows nil)
-       (mapc (lambda (elem)
-               (when (get-register (car elem))
-                 (insert (funcall register-preview-function elem))))
-             register-alist)))))
+  (let ((registers (register-of-type-alist (or types '(all)))))
+    (when (or show-empty (consp registers))
+      (with-current-buffer-window
+        buffer
+        (cons 'display-buffer-below-selected
+	      '((window-height . fit-window-to-buffer)
+	        (preserve-size . (nil . t))))
+        nil
+        (with-current-buffer standard-output
+          (setq cursor-in-non-selected-windows nil)
+          (mapc (lambda (elem)
+                  (when (get-register (car elem))
+                    (insert (funcall register-preview-function elem))))
+                registers))))))
+
+(cl-defgeneric register-preview-get-defaults (action)
+  "Returns default registers according to ACTION."
+  (ignore action))
+(cl-defmethod register-preview-get-defaults ((_action (eql set)))
+  (cl-loop for s in register-preview-default-keys
+           unless (assoc (string-to-char s) register-alist)
+           collect s))
 
 (defun register-read-with-preview (prompt)
   "Read and return a register name, possibly showing existing registers.
-Prompt with the string PROMPT.  If `register-alist' and
-`register-preview-delay' are both non-nil, display a window
-listing existing registers after `register-preview-delay' seconds.
+Prompt with the string PROMPT.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-	 (timer (when (numberp register-preview-delay)
-		  (run-with-timer register-preview-delay nil
-				  (lambda ()
-				    (unless (get-buffer-window buffer)
-				      (register-preview buffer))))))
-	 (help-chars (cl-loop for c in (cons help-char help-event-list)
-			      when (not (get-register c))
-			      collect c)))
+         (pat "")
+         (map (let ((m (make-sparse-keymap)))
+                (set-keymap-parent m minibuffer-local-map)
+                m))
+         (data (register-command-info this-command))
+         types msg result timer act win strs smatch)
+    (if data
+        (setq types  (register-preview-info-types data)
+              msg    (register-preview-info-msg   data)
+              act    (register-preview-info-act   data)
+              smatch (register-preview-info-smatch data))
+      (setq types '(all)
+            msg   "Overwrite register `%s'"
+            act   'set))
+    (setq strs (mapcar (lambda (x)
+                         (string (car x)))
+                       (register-of-type-alist types)))
+    (when (and (memq act '(insert jump view)) (null strs))
+      (error "No register suitable for `%s'" act))
+    (dolist (k (cons help-char help-event-list))
+      (define-key map
+          (vector k) (lambda ()
+                       (interactive)
+                       (unless (get-buffer-window buffer)
+                         (with-selected-window (minibuffer-selected-window)
+                           (register-preview-1 buffer 'show-empty types))))))
+    (define-key map (kbd "<down>") 'register-preview-next)
+    (define-key map (kbd "<up>")   'register-preview-previous)
+    (define-key map (kbd "C-n")    'register-preview-next)
+    (define-key map (kbd "C-p")    'register-preview-previous)
+    (unless (or executing-kbd-macro (null register-use-preview))
+      (register-preview-1 buffer nil types))
     (unwind-protect
-	(progn
-	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
-		       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))
+         (progn
+           (minibuffer-with-setup-hook
+               (lambda ()
+                 (setq timer
+                       (run-with-idle-timer
+                        0.01 'repeat
+                        (lambda ()
+                          (with-selected-window (minibuffer-window)
+                            (let ((input (minibuffer-contents)))
+                              (when (> (length input) 1)
+                                (let ((new (substring input 1))
+                                      (old (substring input 0 1)))
+                                  (setq input (if (or (null smatch)
+                                                      (member new strs))
+                                                  new old))
+                                  (delete-minibuffer-contents)
+                                  (insert input)))
+                              (when (and smatch (not (string= input ""))
+                                         (not (member input strs)))
+                                (setq input "")
+                                (delete-minibuffer-contents)
+                                (minibuffer-message "Not matching"))
+                              (when (not (string= input pat))
+                                (setq pat input))))
+                          (if (setq win (get-buffer-window buffer))
+                              (with-selected-window win
+                                (let ((ov (make-overlay (point-min) (point-min))))
+                                  (goto-char (point-min))
+                                  (remove-overlays)
+                                  (unless (string= pat "")
+                                    (if (re-search-forward (concat "^" pat) nil t)
+                                        (progn (move-overlay
+                                                ov
+                                                (match-beginning 0) (pos-eol))
+                                               (overlay-put ov 'face 'match)
+                                               (when msg
+                                                 (with-selected-window (minibuffer-window)
+                                                   (minibuffer-message msg pat))))
+                                      (with-selected-window (minibuffer-window)
+                                        (minibuffer-message
+                                         "Register `%s' is empty" pat))))))
+                            (unless (string= pat "")
+                              (if (member pat strs)
+                                  (with-selected-window (minibuffer-window)
+                                    (minibuffer-message msg pat))
+                                (with-selected-window (minibuffer-window)
+                                  (minibuffer-message
+                                   "Register `%s' is empty" pat)))))))))
+             (setq result (read-from-minibuffer
+                           prompt nil map nil nil (register-preview-get-defaults act))))
+           (cl-assert (and result (not (string= result "")))
+                      nil "No register specified")
+           (string-to-char result))
+      (when timer (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))

-- 
Thierry

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

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

[Thierry Volpiatto]

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


Here the full patch attached, the previous was not applying properly.

Ccing also Stefan monnier because for some reasons the patch when
applied doesn't compile (when compiling Emacs) unless we add on
top:

    (cl--generic-prefill-dispatchers 0 (eql 'x) integer)

following the advice of the compiler, but I am not sure it is the way to
do.

Compiling register.el manually without this works correctly.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: register-preview --]
[-- Type: text/x-diff, Size: 16878 bytes --]

From c794d62a45b4c83131e506699f465e54b7dae7b5 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Sun, 19 Nov 2023 20:42:56 +0100
Subject: [PATCH] Improve register-preview (Fix bug#66394)

A minibuffer is used now instead of read-key.
Registers in preview buffer are now filtered according to type of
registers the current command requires.
Navigation with C-n/p or up/down is now provided and update
minibuffer.
Current register is highlighted in preview buffer.

* register.el:

* (register-preview-default-keys): new user var.
* (register-use-preview): Same.
* (register-preview-info): New structure to store various info for
  preview.
* (register-command-info): New generic.
* (register-preview-forward-line): New, provide navigation in preview
  buffer.
* (register-preview-next, register-preview-previous): New, navigation.
* (register-type): New, returns register type.
* (register--type): Generic fn, new, returns register type according
  to value.
* (register-of-type-alist): New, filter register-alist according to
  type.
* (register-preview): Signature changed, use TYPES now.
* (register-preview-get-defaults): New generic, compute defauts
  according to action.
* (register-read-with-preview): Now use read-from-minibuffer and
  minibuffer-setup-hook.
---
 lisp/register.el | 296 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 259 insertions(+), 37 deletions(-)

diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..47f0dfa389c 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -30,11 +30,14 @@
 ;; documented in the Emacs user's manual: (info "(emacs) Registers").
 
 (eval-when-compile (require 'cl-lib))
+(cl--generic-prefill-dispatchers 0 (eql 'x) integer)
 
 ;;; Code:
 
 ;; FIXME: Clean up namespace usage!
 
+(declare-function frameset-register-p "frameset")
+
 (cl-defstruct
   (registerv (:constructor nil)
 	     (:constructor registerv--make (&optional data print-func
@@ -91,6 +94,7 @@ 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
@@ -99,6 +103,14 @@ If nil, do not show register previews, unless `help-char' (or a member of
   :type '(choice number (const :tag "No preview unless requested" nil))
   :group 'register)
 
+(defcustom register-preview-default-keys (mapcar #'string (number-sequence ?a ?z))
+  "Default keys for setting a new register."
+  :type '(repeat string))
+
+(defcustom register-use-preview t
+  "Always show register preview when non nil."
+  :type 'boolean)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (alist-get register register-alist))
@@ -128,53 +140,263 @@ See the documentation of the variable `register-alist' for possible VALUEs."
 Called with one argument, a cons (NAME . CONTENTS) as found in `register-alist'.
 The function should return a string, the description of the argument.")
 
-(defun register-preview (buffer &optional show-empty)
+(cl-defstruct register-preview-info
+  "Store data for a specific register command.
+TYPES are the types of register supported.
+MSG is the minibuffer message to send when a register is selected.
+ACT is the type of action the command is doing on register.
+SMATCH accept a boolean value to say if command accept non matching register."
+  types msg act smatch)
+
+(cl-defgeneric register-command-info (command)
+  "Returns a `register-preview-info' object storing data for COMMAND."
+  (ignore command))
+(cl-defmethod register-command-info ((_command (eql insert-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Insert register `%s'"
+   :act 'insert
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql jump-to-register)))
+  (make-register-preview-info
+   :types  '(window frame marker kmacro
+             file buffer file-query)
+   :msg "Jump to register `%s'"
+   :act 'jump
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql view-register)))
+  (make-register-preview-info
+   :types '(all)
+   :msg "View register `%s'"
+   :act 'view
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql append-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Append to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql prepend-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Prepend to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql increment-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Increment register `%s'"
+   :act 'modify
+   :smatch t))
+
+(defun register-preview-forward-line (arg)
+  "Move to next or previous line in register preview buffer.
+If ARG is positive goto next line, if negative to previous.
+Do nothing when defining or executing kmacros."
+  ;; Ensure user enter manually key in minibuffer when recording a macro.
+  (unless (or defining-kbd-macro executing-kbd-macro
+              (not (get-buffer-window "*Register Preview*" 'visible)))
+    (let ((fn (if (> arg 0) #'eobp #'bobp))
+          (posfn (if (> arg 0)
+                     #'point-min
+                     (lambda () (1- (point-max)))))
+          str)
+      (with-current-buffer "*Register Preview*"
+        (let ((ovs (overlays-in (point-min) (point-max)))
+              pos)
+          (goto-char (if ovs
+                         (overlay-start (car ovs))
+                         (point-min)))
+          (setq pos (point))
+          (and ovs (forward-line arg))
+          (when (and (funcall fn)
+                     (or (> arg 0) (eql pos (point))))
+            (goto-char (funcall posfn)))
+          (setq str (buffer-substring-no-properties
+                     (pos-bol) (1+ (pos-bol))))
+          (remove-overlays)
+          (with-selected-window (minibuffer-window)
+            (delete-minibuffer-contents)
+            (insert str)))))))
+
+(defun register-preview-next ()
+  "Goto next line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line 1))
+
+(defun register-preview-previous ()
+  "Goto previous line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line -1))
+
+(defun register-type (register)
+  "Return REGISTER type.
+Current register types actually returned are one of:
+- string
+- number
+- marker
+- buffer
+- file
+- file-query
+- window
+- frame
+- kmacro
+
+One can add new types to a specific command by defining a new `cl-defmethod'
+matching this command. Predicate for type in new `cl-defmethod' should
+satisfy `cl-typep' otherwise the new type should be defined with
+`cl-deftype'."
+  ;; Call register--type against the register value.
+  (register--type (if (consp (cdr register))
+                     (cadr register)
+                   (cdr register))))
+
+(cl-defgeneric register--type (regval)
+  "Returns type of register value REGVAL."
+  (ignore regval))
+
+(cl-defmethod register--type ((_regval string)) 'string)
+(cl-defmethod register--type ((_regval number)) 'number)
+(cl-defmethod register--type ((_regval marker)) 'marker)
+(cl-defmethod register--type ((_regval (eql 'buffer))) 'buffer)
+(cl-defmethod register--type ((_regval (eql 'file))) 'file)
+(cl-defmethod register--type ((_regval (eql 'file-query))) 'file-query)
+(cl-defmethod register--type ((_regval window-configuration)) 'window)
+(cl-deftype frame-register () '(satisfies frameset-register-p))
+(cl-defmethod register--type :extra "frame-register" (_regval) 'frame)
+(cl-deftype kmacro-register () '(satisfies kmacro-register-p))
+(cl-defmethod register--type :extra "kmacro-register" (_regval) 'kmacro)
+
+(defun register-of-type-alist (types)
+  "Filter `register-alist' according to TYPES."
+  (if (memq 'all types)
+      register-alist
+    (cl-loop for register in register-alist
+             when (memq (register-type register) types)
+             collect register)))
+
+(defun register-preview (buffer &optional show-empty types)
   "Pop up a window showing the registers preview in BUFFER.
 If SHOW-EMPTY is non-nil, show the window even if no registers.
+Argument TYPES (a list) specify the types of register to show, when nil show all
+registers, see `register-type' for suitable types.
 Format of each entry is controlled by the variable `register-preview-function'."
-  (when (or show-empty (consp register-alist))
-    (with-current-buffer-window
-     buffer
-     (cons 'display-buffer-below-selected
-	   '((window-height . fit-window-to-buffer)
-	     (preserve-size . (nil . t))))
-     nil
-     (with-current-buffer standard-output
-       (setq cursor-in-non-selected-windows nil)
-       (mapc (lambda (elem)
-               (when (get-register (car elem))
-                 (insert (funcall register-preview-function elem))))
-             register-alist)))))
+  (let ((registers (register-of-type-alist (or types '(all)))))
+    (when (or show-empty (consp registers))
+      (with-current-buffer-window
+        buffer
+        (cons 'display-buffer-below-selected
+	      '((window-height . fit-window-to-buffer)
+	        (preserve-size . (nil . t))))
+        nil
+        (with-current-buffer standard-output
+          (setq cursor-in-non-selected-windows nil)
+          (mapc (lambda (elem)
+                  (when (get-register (car elem))
+                    (insert (funcall register-preview-function elem))))
+                registers))))))
+
+(cl-defgeneric register-preview-get-defaults (action)
+  "Returns default registers according to ACTION."
+  (ignore action))
+(cl-defmethod register-preview-get-defaults ((_action (eql set)))
+  (cl-loop for s in register-preview-default-keys
+           unless (assoc (string-to-char s) register-alist)
+           collect s))
 
 (defun register-read-with-preview (prompt)
   "Read and return a register name, possibly showing existing registers.
-Prompt with the string PROMPT.  If `register-alist' and
-`register-preview-delay' are both non-nil, display a window
-listing existing registers after `register-preview-delay' seconds.
+Prompt with the string PROMPT.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-	 (timer (when (numberp register-preview-delay)
-		  (run-with-timer register-preview-delay nil
-				  (lambda ()
-				    (unless (get-buffer-window buffer)
-				      (register-preview buffer))))))
-	 (help-chars (cl-loop for c in (cons help-char help-event-list)
-			      when (not (get-register c))
-			      collect c)))
+         (pat "")
+         (map (let ((m (make-sparse-keymap)))
+                (set-keymap-parent m minibuffer-local-map)
+                m))
+         (data (register-command-info this-command))
+         types msg result timer act win strs smatch)
+    (if data
+        (setq types  (register-preview-info-types data)
+              msg    (register-preview-info-msg   data)
+              act    (register-preview-info-act   data)
+              smatch (register-preview-info-smatch data))
+      (setq types '(all)
+            msg   "Overwrite register `%s'"
+            act   'set))
+    (setq strs (mapcar (lambda (x)
+                         (string (car x)))
+                       (register-of-type-alist types)))
+    (when (and (memq act '(insert jump view)) (null strs))
+      (error "No register suitable for `%s'" act))
+    (dolist (k (cons help-char help-event-list))
+      (define-key map
+          (vector k) (lambda ()
+                       (interactive)
+                       (unless (get-buffer-window buffer)
+                         (with-selected-window (minibuffer-selected-window)
+                           (register-preview buffer 'show-empty types))))))
+    (define-key map (kbd "<down>") 'register-preview-next)
+    (define-key map (kbd "<up>")   'register-preview-previous)
+    (define-key map (kbd "C-n")    'register-preview-next)
+    (define-key map (kbd "C-p")    'register-preview-previous)
+    (unless (or executing-kbd-macro (null register-use-preview))
+      (register-preview buffer nil types))
     (unwind-protect
-	(progn
-	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
-		       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))
+         (progn
+           (minibuffer-with-setup-hook
+               (lambda ()
+                 (setq timer
+                       (run-with-idle-timer
+                        0.01 'repeat
+                        (lambda ()
+                          (with-selected-window (minibuffer-window)
+                            (let ((input (minibuffer-contents)))
+                              (when (> (length input) 1)
+                                (let ((new (substring input 1))
+                                      (old (substring input 0 1)))
+                                  (setq input (if (or (null smatch)
+                                                      (member new strs))
+                                                  new old))
+                                  (delete-minibuffer-contents)
+                                  (insert input)))
+                              (when (and smatch (not (string= input ""))
+                                         (not (member input strs)))
+                                (setq input "")
+                                (delete-minibuffer-contents)
+                                (minibuffer-message "Not matching"))
+                              (when (not (string= input pat))
+                                (setq pat input))))
+                          (if (setq win (get-buffer-window buffer))
+                              (with-selected-window win
+                                (let ((ov (make-overlay (point-min) (point-min))))
+                                  (goto-char (point-min))
+                                  (remove-overlays)
+                                  (unless (string= pat "")
+                                    (if (re-search-forward (concat "^" pat) nil t)
+                                        (progn (move-overlay
+                                                ov
+                                                (match-beginning 0) (pos-eol))
+                                               (overlay-put ov 'face 'match)
+                                               (when msg
+                                                 (with-selected-window (minibuffer-window)
+                                                   (minibuffer-message msg pat))))
+                                      (with-selected-window (minibuffer-window)
+                                        (minibuffer-message
+                                         "Register `%s' is empty" pat))))))
+                            (unless (string= pat "")
+                              (if (member pat strs)
+                                  (with-selected-window (minibuffer-window)
+                                    (minibuffer-message msg pat))
+                                (with-selected-window (minibuffer-window)
+                                  (minibuffer-message
+                                   "Register `%s' is empty" pat)))))))))
+             (setq result (read-from-minibuffer
+                           prompt nil map nil nil (register-preview-get-defaults act))))
+           (cl-assert (and result (not (string= result "")))
+                      nil "No register specified")
+           (string-to-char result))
+      (when timer (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))
-- 
2.34.1


[-- Attachment #1.3: Type: text/plain, Size: 16 bytes --]


-- 
Thierry

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

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

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

> Ccing also Stefan monnier because for some reasons the patch when
> applied doesn't compile (when compiling Emacs) unless we add on
> top:
>
>     (cl--generic-prefill-dispatchers 0 (eql 'x) integer)
>
> following the advice of the compiler,

Indeed, this is needed because `register.el` is preloaded and method
dispatcher are generated&compiled "on the fly" but we don't want to
preload the compiler, so we want to pre-compile the dispatchers used
by the preloaded code.

> but I am not sure it is the way to do.

`cl--generic-prefill-dispatchers` is not guaranteed to be defined when
we load `register.el` (it's only defined if we loaded the non-compiled
version of `cl-generic.el`) so the above call should be in
`cl-generic.el` rather than in `register.el`.

I'd put it next to the following block:

    (cl--generic-prefill-dispatchers 0 (eql nil))
    (cl--generic-prefill-dispatchers window-system (eql nil))
    (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--get-selection)
                                     (eql nil))
    (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--set-selection)
                                     (eql nil))

which is already about dispatchers needed to support other files.


        Stefan

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

[Thierry Volpiatto]

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


Hello Stefan,

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Ccing also Stefan monnier because for some reasons the patch when
>> applied doesn't compile (when compiling Emacs) unless we add on
>> top:
>>
>>     (cl--generic-prefill-dispatchers 0 (eql 'x) integer)
>>
>> following the advice of the compiler,
>
> Indeed, this is needed because `register.el` is preloaded and method
> dispatcher are generated&compiled "on the fly" but we don't want to
> preload the compiler, so we want to pre-compile the dispatchers used
> by the preloaded code.
>
>> but I am not sure it is the way to do.
>
> `cl--generic-prefill-dispatchers` is not guaranteed to be defined when
> we load `register.el` (it's only defined if we loaded the non-compiled
> version of `cl-generic.el`) so the above call should be in
> `cl-generic.el` rather than in `register.el`.
>
> I'd put it next to the following block:
>
>     (cl--generic-prefill-dispatchers 0 (eql nil))
>     (cl--generic-prefill-dispatchers window-system (eql nil))
>     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--get-selection)
>                                      (eql nil))
>     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--set-selection)
>                                      (eql nil))
>
> which is already about dispatchers needed to support other files.

I will for now leave the call to `cl--generic-prefill-dispatchers` in
the patch as a reminder that it should be added instead in cl-defgeneric
once merging.

Thanks for your explanation.

-- 
Thierry

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

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

[Eli Zaretskii]

> Cc: Michael Heerdegen <michael_heerdegen@web.de>, stefankangas@gmail.com,
>  66394@debbugs.gnu.org
> From: Thierry Volpiatto <thievol@posteo.net>
> Date: Mon, 20 Nov 2023 18:51:10 +0000
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> >> Ccing also Stefan monnier because for some reasons the patch when
> >> applied doesn't compile (when compiling Emacs) unless we add on
> >> top:
> >>
> >>     (cl--generic-prefill-dispatchers 0 (eql 'x) integer)
> >>
> >> following the advice of the compiler,
> >
> > Indeed, this is needed because `register.el` is preloaded and method
> > dispatcher are generated&compiled "on the fly" but we don't want to
> > preload the compiler, so we want to pre-compile the dispatchers used
> > by the preloaded code.
> >
> >> but I am not sure it is the way to do.
> >
> > `cl--generic-prefill-dispatchers` is not guaranteed to be defined when
> > we load `register.el` (it's only defined if we loaded the non-compiled
> > version of `cl-generic.el`) so the above call should be in
> > `cl-generic.el` rather than in `register.el`.
> >
> > I'd put it next to the following block:
> >
> >     (cl--generic-prefill-dispatchers 0 (eql nil))
> >     (cl--generic-prefill-dispatchers window-system (eql nil))
> >     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--get-selection)
> >                                      (eql nil))
> >     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--set-selection)
> >                                      (eql nil))
> >
> > which is already about dispatchers needed to support other files.
> 
> I will for now leave the call to `cl--generic-prefill-dispatchers` in
> the patch as a reminder that it should be added instead in cl-defgeneric
> once merging.

I tried to install the patch, but it fails to compile:

    ELC      ../lisp/register.elc

  In toplevel form:
  register.el:33:37: Warning: `eql' called with 1 argument, but requires 2
  register.el:33:45: Warning: reference to free variable `integer'

This then fails the build, since 'register' is preloaded.

Thierry, can you please fix the code, so that I could install it?  Or
what am I missing?

P.S. Also, the log message is not according to our conventions.

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

[Thierry Volpiatto]

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


Hello Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: Michael Heerdegen <michael_heerdegen@web.de>, stefankangas@gmail.com,
>>  66394@debbugs.gnu.org
>> From: Thierry Volpiatto <thievol@posteo.net>
>> Date: Mon, 20 Nov 2023 18:51:10 +0000
>> 
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 
>> >> Ccing also Stefan monnier because for some reasons the patch when
>> >> applied doesn't compile (when compiling Emacs) unless we add on
>> >> top:
>> >>
>> >>     (cl--generic-prefill-dispatchers 0 (eql 'x) integer)
>> >>
>> >> following the advice of the compiler,
>> >
>> > Indeed, this is needed because `register.el` is preloaded and method
>> > dispatcher are generated&compiled "on the fly" but we don't want to
>> > preload the compiler, so we want to pre-compile the dispatchers used
>> > by the preloaded code.
>> >
>> >> but I am not sure it is the way to do.
>> >
>> > `cl--generic-prefill-dispatchers` is not guaranteed to be defined when
>> > we load `register.el` (it's only defined if we loaded the non-compiled
>> > version of `cl-generic.el`) so the above call should be in
>> > `cl-generic.el` rather than in `register.el`.
>> >
>> > I'd put it next to the following block:
>> >
>> >     (cl--generic-prefill-dispatchers 0 (eql nil))
>> >     (cl--generic-prefill-dispatchers window-system (eql nil))
>> >     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--get-selection)
>> >                                      (eql nil))
>> >     (cl--generic-prefill-dispatchers (terminal-parameter nil 'xterm--set-selection)
>> >                                      (eql nil))
>> >
>> > which is already about dispatchers needed to support other files.
>> 
>> I will for now leave the call to `cl--generic-prefill-dispatchers` in
>> the patch as a reminder that it should be added instead in cl-defgeneric
>> once merging.
>
> I tried to install the patch, but it fails to compile:
>
>     ELC      ../lisp/register.elc
>
>   In toplevel form:
>   register.el:33:37: Warning: `eql' called with 1 argument, but requires 2
>   register.el:33:45: Warning: reference to free variable `integer'

I have not these warnings.

> This then fails the build, since 'register' is preloaded.

Here it is building fine, this from the last Emacs master from tonight.

> Thierry, can you please fix the code, so that I could install it?  Or
> what am I missing?

Don't know, did you "make clean" first?

NOTE: I leaved the patch like this but it needs the change suggested by
Stefan before merging (or with an extra commit) see above.

> P.S. Also, the log message is not according to our conventions.

I don't remember now what are your conventions for commits, perhaps you
can correct it if needed?

Thanks.

-- 
Thierry

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

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

[Eli Zaretskii]

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>   stefankangas@gmail.com,  66394@debbugs.gnu.org
> Date: Sat, 25 Nov 2023 19:59:26 +0000
> 
> >     ELC      ../lisp/register.elc
> >
> >   In toplevel form:
> >   register.el:33:37: Warning: `eql' called with 1 argument, but requires 2
> >   register.el:33:45: Warning: reference to free variable `integer'
> 
> I have not these warnings.

Strange.  Maybe Stefan can explain how could that happen.

> > This then fails the build, since 'register' is preloaded.
> 
> Here it is building fine, this from the last Emacs master from tonight.

I'm also applying to master, obviously.

> > Thierry, can you please fix the code, so that I could install it?  Or
> > what am I missing?
> 
> Don't know, did you "make clean" first?

How would "make clean" help?  I did remove register.elc, it didn't
help.

> NOTE: I leaved the patch like this but it needs the change suggested by
> Stefan before merging (or with an extra commit) see above.

OK, so I guess I shouldn't have tried to install it.

> > P.S. Also, the log message is not according to our conventions.
> 
> I don't remember now what are your conventions for commits, perhaps you
> can correct it if needed?

Of course, I can correct it.  I just thought that if you are going to
submit a fixed patch, perhaps you could fix the log message as well,
to spare me some manual work.  Never mind now, since I'm not going to
install it yet.

Thanks.

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

[Thierry Volpiatto]

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

Eli Zaretskii <eliz@gnu.org> writes:

> How would "make clean" help?  I did remove register.elc, it didn't
> help.

Not only register is involved here but indirectly cl-generic.

> OK, so I guess I shouldn't have tried to install it.

Here a patch with the change suggested by Stefan applied, slighly
modified though because it fails if I put the call to
cl--generic-prefill-dispatchers at the recommended place.

I recompiled Emacs with this patch with no errors.
 
-- 
Thierry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Improve-register-preview-Fix-bug-66394.patch --]
[-- Type: text/x-diff, Size: 17350 bytes --]

From ccecd34b8307c7168e89289f933e63590ce6fc9c Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Sun, 19 Nov 2023 20:42:56 +0100
Subject: [PATCH] Improve register-preview (Fix bug#66394)

A minibuffer is used now instead of read-key.
Registers in preview buffer are now filtered according to type of
registers the current command requires.
Navigation with C-n/p or up/down is now provided and update
minibuffer.
Current register is highlighted in preview buffer.

* cl-generic.el: Add a call to `cl--generic-prefill-dispatchers` to
  fix build error.

* register.el:

* (register-preview-default-keys): new user var.
* (register-use-preview): Same.
* (register-preview-info): New structure to store various info for
  preview.
* (register-command-info): New generic.
* (register-preview-forward-line): New, provide navigation in preview
  buffer.
* (register-preview-next, register-preview-previous): New, navigation.
* (register-type): New, returns register type.
* (register--type): Generic fn, new, returns register type according
  to value.
* (register-of-type-alist): New, filter register-alist according to
  type.
* (register-preview): Signature changed, use TYPES now.
* (register-preview-get-defaults): New generic, compute defauts
  according to action.
* (register-read-with-preview): Now use read-from-minibuffer and
  minibuffer-setup-hook.
---
 lisp/emacs-lisp/cl-generic.el |   1 +
 lisp/register.el              | 295 +++++++++++++++++++++++++++++-----
 2 files changed, 259 insertions(+), 37 deletions(-)

diff --git a/lisp/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index 56eb83e6f75..0ef0d1e192a 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -1379,6 +1379,7 @@ See the full list and their hierarchy in `cl--typeof-types'."
 (cl--generic-prefill-dispatchers 0 integer)
 (cl--generic-prefill-dispatchers 1 integer)
 (cl--generic-prefill-dispatchers 0 cl--generic-generalizer integer)
+(cl--generic-prefill-dispatchers 0 (eql 'x) integer)
 
 ;;; Dispatch on major mode.
 
diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..61bef503f91 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -35,6 +35,8 @@
 
 ;; FIXME: Clean up namespace usage!
 
+(declare-function frameset-register-p "frameset")
+
 (cl-defstruct
   (registerv (:constructor nil)
 	     (:constructor registerv--make (&optional data print-func
@@ -91,6 +93,7 @@ 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
@@ -99,6 +102,14 @@ If nil, do not show register previews, unless `help-char' (or a member of
   :type '(choice number (const :tag "No preview unless requested" nil))
   :group 'register)
 
+(defcustom register-preview-default-keys (mapcar #'string (number-sequence ?a ?z))
+  "Default keys for setting a new register."
+  :type '(repeat string))
+
+(defcustom register-use-preview t
+  "Always show register preview when non nil."
+  :type 'boolean)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (alist-get register register-alist))
@@ -128,53 +139,263 @@ See the documentation of the variable `register-alist' for possible VALUEs."
 Called with one argument, a cons (NAME . CONTENTS) as found in `register-alist'.
 The function should return a string, the description of the argument.")
 
-(defun register-preview (buffer &optional show-empty)
+(cl-defstruct register-preview-info
+  "Store data for a specific register command.
+TYPES are the types of register supported.
+MSG is the minibuffer message to send when a register is selected.
+ACT is the type of action the command is doing on register.
+SMATCH accept a boolean value to say if command accept non matching register."
+  types msg act smatch)
+
+(cl-defgeneric register-command-info (command)
+  "Returns a `register-preview-info' object storing data for COMMAND."
+  (ignore command))
+(cl-defmethod register-command-info ((_command (eql insert-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Insert register `%s'"
+   :act 'insert
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql jump-to-register)))
+  (make-register-preview-info
+   :types  '(window frame marker kmacro
+             file buffer file-query)
+   :msg "Jump to register `%s'"
+   :act 'jump
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql view-register)))
+  (make-register-preview-info
+   :types '(all)
+   :msg "View register `%s'"
+   :act 'view
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql append-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Append to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql prepend-to-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Prepend to register `%s'"
+   :act 'modify
+   :smatch t))
+(cl-defmethod register-command-info ((_command (eql increment-register)))
+  (make-register-preview-info
+   :types '(string number)
+   :msg "Increment register `%s'"
+   :act 'modify
+   :smatch t))
+
+(defun register-preview-forward-line (arg)
+  "Move to next or previous line in register preview buffer.
+If ARG is positive goto next line, if negative to previous.
+Do nothing when defining or executing kmacros."
+  ;; Ensure user enter manually key in minibuffer when recording a macro.
+  (unless (or defining-kbd-macro executing-kbd-macro
+              (not (get-buffer-window "*Register Preview*" 'visible)))
+    (let ((fn (if (> arg 0) #'eobp #'bobp))
+          (posfn (if (> arg 0)
+                     #'point-min
+                     (lambda () (1- (point-max)))))
+          str)
+      (with-current-buffer "*Register Preview*"
+        (let ((ovs (overlays-in (point-min) (point-max)))
+              pos)
+          (goto-char (if ovs
+                         (overlay-start (car ovs))
+                         (point-min)))
+          (setq pos (point))
+          (and ovs (forward-line arg))
+          (when (and (funcall fn)
+                     (or (> arg 0) (eql pos (point))))
+            (goto-char (funcall posfn)))
+          (setq str (buffer-substring-no-properties
+                     (pos-bol) (1+ (pos-bol))))
+          (remove-overlays)
+          (with-selected-window (minibuffer-window)
+            (delete-minibuffer-contents)
+            (insert str)))))))
+
+(defun register-preview-next ()
+  "Goto next line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line 1))
+
+(defun register-preview-previous ()
+  "Goto previous line in register preview buffer."
+  (interactive)
+  (register-preview-forward-line -1))
+
+(defun register-type (register)
+  "Return REGISTER type.
+Current register types actually returned are one of:
+- string
+- number
+- marker
+- buffer
+- file
+- file-query
+- window
+- frame
+- kmacro
+
+One can add new types to a specific command by defining a new `cl-defmethod'
+matching this command. Predicate for type in new `cl-defmethod' should
+satisfy `cl-typep' otherwise the new type should be defined with
+`cl-deftype'."
+  ;; Call register--type against the register value.
+  (register--type (if (consp (cdr register))
+                     (cadr register)
+                   (cdr register))))
+
+(cl-defgeneric register--type (regval)
+  "Returns type of register value REGVAL."
+  (ignore regval))
+
+(cl-defmethod register--type ((_regval string)) 'string)
+(cl-defmethod register--type ((_regval number)) 'number)
+(cl-defmethod register--type ((_regval marker)) 'marker)
+(cl-defmethod register--type ((_regval (eql 'buffer))) 'buffer)
+(cl-defmethod register--type ((_regval (eql 'file))) 'file)
+(cl-defmethod register--type ((_regval (eql 'file-query))) 'file-query)
+(cl-defmethod register--type ((_regval window-configuration)) 'window)
+(cl-deftype frame-register () '(satisfies frameset-register-p))
+(cl-defmethod register--type :extra "frame-register" (_regval) 'frame)
+(cl-deftype kmacro-register () '(satisfies kmacro-register-p))
+(cl-defmethod register--type :extra "kmacro-register" (_regval) 'kmacro)
+
+(defun register-of-type-alist (types)
+  "Filter `register-alist' according to TYPES."
+  (if (memq 'all types)
+      register-alist
+    (cl-loop for register in register-alist
+             when (memq (register-type register) types)
+             collect register)))
+
+(defun register-preview (buffer &optional show-empty types)
   "Pop up a window showing the registers preview in BUFFER.
 If SHOW-EMPTY is non-nil, show the window even if no registers.
+Argument TYPES (a list) specify the types of register to show, when nil show all
+registers, see `register-type' for suitable types.
 Format of each entry is controlled by the variable `register-preview-function'."
-  (when (or show-empty (consp register-alist))
-    (with-current-buffer-window
-     buffer
-     (cons 'display-buffer-below-selected
-	   '((window-height . fit-window-to-buffer)
-	     (preserve-size . (nil . t))))
-     nil
-     (with-current-buffer standard-output
-       (setq cursor-in-non-selected-windows nil)
-       (mapc (lambda (elem)
-               (when (get-register (car elem))
-                 (insert (funcall register-preview-function elem))))
-             register-alist)))))
+  (let ((registers (register-of-type-alist (or types '(all)))))
+    (when (or show-empty (consp registers))
+      (with-current-buffer-window
+        buffer
+        (cons 'display-buffer-below-selected
+	      '((window-height . fit-window-to-buffer)
+	        (preserve-size . (nil . t))))
+        nil
+        (with-current-buffer standard-output
+          (setq cursor-in-non-selected-windows nil)
+          (mapc (lambda (elem)
+                  (when (get-register (car elem))
+                    (insert (funcall register-preview-function elem))))
+                registers))))))
+
+(cl-defgeneric register-preview-get-defaults (action)
+  "Returns default registers according to ACTION."
+  (ignore action))
+(cl-defmethod register-preview-get-defaults ((_action (eql set)))
+  (cl-loop for s in register-preview-default-keys
+           unless (assoc (string-to-char s) register-alist)
+           collect s))
 
 (defun register-read-with-preview (prompt)
   "Read and return a register name, possibly showing existing registers.
-Prompt with the string PROMPT.  If `register-alist' and
-`register-preview-delay' are both non-nil, display a window
-listing existing registers after `register-preview-delay' seconds.
+Prompt with the string PROMPT.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-	 (timer (when (numberp register-preview-delay)
-		  (run-with-timer register-preview-delay nil
-				  (lambda ()
-				    (unless (get-buffer-window buffer)
-				      (register-preview buffer))))))
-	 (help-chars (cl-loop for c in (cons help-char help-event-list)
-			      when (not (get-register c))
-			      collect c)))
+         (pat "")
+         (map (let ((m (make-sparse-keymap)))
+                (set-keymap-parent m minibuffer-local-map)
+                m))
+         (data (register-command-info this-command))
+         types msg result timer act win strs smatch)
+    (if data
+        (setq types  (register-preview-info-types data)
+              msg    (register-preview-info-msg   data)
+              act    (register-preview-info-act   data)
+              smatch (register-preview-info-smatch data))
+      (setq types '(all)
+            msg   "Overwrite register `%s'"
+            act   'set))
+    (setq strs (mapcar (lambda (x)
+                         (string (car x)))
+                       (register-of-type-alist types)))
+    (when (and (memq act '(insert jump view)) (null strs))
+      (error "No register suitable for `%s'" act))
+    (dolist (k (cons help-char help-event-list))
+      (define-key map
+          (vector k) (lambda ()
+                       (interactive)
+                       (unless (get-buffer-window buffer)
+                         (with-selected-window (minibuffer-selected-window)
+                           (register-preview buffer 'show-empty types))))))
+    (define-key map (kbd "<down>") 'register-preview-next)
+    (define-key map (kbd "<up>")   'register-preview-previous)
+    (define-key map (kbd "C-n")    'register-preview-next)
+    (define-key map (kbd "C-p")    'register-preview-previous)
+    (unless (or executing-kbd-macro (null register-use-preview))
+      (register-preview buffer nil types))
     (unwind-protect
-	(progn
-	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
-		       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))
+         (progn
+           (minibuffer-with-setup-hook
+               (lambda ()
+                 (setq timer
+                       (run-with-idle-timer
+                        0.01 'repeat
+                        (lambda ()
+                          (with-selected-window (minibuffer-window)
+                            (let ((input (minibuffer-contents)))
+                              (when (> (length input) 1)
+                                (let ((new (substring input 1))
+                                      (old (substring input 0 1)))
+                                  (setq input (if (or (null smatch)
+                                                      (member new strs))
+                                                  new old))
+                                  (delete-minibuffer-contents)
+                                  (insert input)))
+                              (when (and smatch (not (string= input ""))
+                                         (not (member input strs)))
+                                (setq input "")
+                                (delete-minibuffer-contents)
+                                (minibuffer-message "Not matching"))
+                              (when (not (string= input pat))
+                                (setq pat input))))
+                          (if (setq win (get-buffer-window buffer))
+                              (with-selected-window win
+                                (let ((ov (make-overlay (point-min) (point-min))))
+                                  (goto-char (point-min))
+                                  (remove-overlays)
+                                  (unless (string= pat "")
+                                    (if (re-search-forward (concat "^" pat) nil t)
+                                        (progn (move-overlay
+                                                ov
+                                                (match-beginning 0) (pos-eol))
+                                               (overlay-put ov 'face 'match)
+                                               (when msg
+                                                 (with-selected-window (minibuffer-window)
+                                                   (minibuffer-message msg pat))))
+                                      (with-selected-window (minibuffer-window)
+                                        (minibuffer-message
+                                         "Register `%s' is empty" pat))))))
+                            (unless (string= pat "")
+                              (if (member pat strs)
+                                  (with-selected-window (minibuffer-window)
+                                    (minibuffer-message msg pat))
+                                (with-selected-window (minibuffer-window)
+                                  (minibuffer-message
+                                   "Register `%s' is empty" pat)))))))))
+             (setq result (read-from-minibuffer
+                           prompt nil map nil nil (register-preview-get-defaults act))))
+           (cl-assert (and result (not (string= result "")))
+                      nil "No register specified")
+           (string-to-char result))
+      (when timer (cancel-timer timer))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))
-- 
2.34.1


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

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

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

>> >   In toplevel form:
>> >   register.el:33:37: Warning: `eql' called with 1 argument, but requires 2
>> >   register.el:33:45: Warning: reference to free variable `integer'
>> I have not these warnings.
> Strange.  Maybe Stefan can explain how could that happen.

It's because of:

    (cl--generic-prefill-dispatchers 0 (eql 'x) integer)

This needs to be in `cl-generic.el` because that's where the
`cl--generic-prefill-dispatchers` macro is defined.  Thierry had it in
`register.el` which worked OK when `register.el` gets compiled with the
bootstrap Emacs where `cl-generic` has not yet been compiled but it
fails when compiled with an Emacs where `cl-generic` has been compiled
because in that case the macro is not defined so the compiler compiles
the above line as a function call, resulting in the above two warnings.


        Stefan

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

[Eli Zaretskii]

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>   stefankangas@gmail.com,  66394@debbugs.gnu.org
> Date: Sat, 25 Nov 2023 21:14:31 +0000
> 
> Here a patch with the change suggested by Stefan applied, slighly
> modified though because it fails if I put the call to
> cl--generic-prefill-dispatchers at the recommended place.
> 
> I recompiled Emacs with this patch with no errors.

Thanks.

Stefan, any further comments, or should I install this as submitted?

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

[Thierry Volpiatto]

[-- Attachment #1: Type: text/plain, Size: 2047 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: Sat, 25 Nov 2023 21:14:31 +0000
>> 
>> Here a patch with the change suggested by Stefan applied, slighly
>> modified though because it fails if I put the call to
>> cl--generic-prefill-dispatchers at the recommended place.
>> 
>> I recompiled Emacs with this patch with no errors.
>
> Thanks.
>
> Stefan, any further comments, or should I install this as submitted?

Do you want the ability to jump to more than one line at the time before
merging, (e.g. C-u 3 C-n and C-u 3 C-p) or is it ok like this for you?

diff --git a/lisp/register.el b/lisp/register.el
index 61bef503f91..bca967a4efe 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -207,7 +207,7 @@ Do nothing when defining or executing kmacros."
                          (overlay-start (car ovs))
                          (point-min)))
           (setq pos (point))
-          (and ovs (forward-line arg))
+          (forward-line (if ovs arg (1- arg)))
           (when (and (funcall fn)
                      (or (> arg 0) (eql pos (point))))
             (goto-char (funcall posfn)))
@@ -218,15 +218,15 @@ Do nothing when defining or executing kmacros."
             (delete-minibuffer-contents)
             (insert str)))))))
 
-(defun register-preview-next ()
+(defun register-preview-next (&optional arg)
   "Goto next line in register preview buffer."
-  (interactive)
-  (register-preview-forward-line 1))
+  (interactive "p")
+  (register-preview-forward-line arg))
 
-(defun register-preview-previous ()
+(defun register-preview-previous (&optional arg)
   "Goto previous line in register preview buffer."
-  (interactive)
-  (register-preview-forward-line -1))
+  (interactive "p")
+  (register-preview-forward-line (- arg)))
 
 (defun register-type (register)
   "Return REGISTER type.
 
-- 
Thierry

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

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

[Eli Zaretskii]

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>   stefankangas@gmail.com,  66394@debbugs.gnu.org
> Date: Sat, 25 Nov 2023 21:14:31 +0000
> 
> Here a patch with the change suggested by Stefan applied, slighly
> modified though because it fails if I put the call to
> cl--generic-prefill-dispatchers at the recommended place.
> 
> I recompiled Emacs with this patch with no errors.

Thanks, installed on master.

After this, a test in register-tests.el fails:

  Running 1 tests (2023-11-29 09:01:59-0500, selector ‘(not (or (tag :unstable) (tag :nativecomp)))’)
  Point to register: a
  Test register-test-bug27634 backtrace:
    signal(ert-test-failed (((should (equal 'quit (condition-case err (c
    ert-fail(((should (equal 'quit (condition-case err (call-interactive
    (if (unwind-protect (setq value-2 (apply fn-0 args-1)) (setq form-de
    (let (form-description-4) (if (unwind-protect (setq value-2 (apply f
    (let ((value-2 'ert-form-evaluation-aborted-3)) (let (form-descripti
    (let* ((fn-0 #'equal) (args-1 (condition-case err (let ((signal-hook
    (progn (fset 'read-key #'ignore) (let* ((fn-0 #'equal) (args-1 (cond
    (unwind-protect (progn (fset 'read-key #'ignore) (let* ((fn-0 #'equa
    (let* ((vnew event) (old (symbol-function 'read-key)) (register-alis
    (let ((event (car tail))) (let* ((vnew event) (old (symbol-function
    (while tail (let ((event (car tail))) (let* ((vnew event) (old (symb
    (let ((tail (list 7 'escape 27))) (while tail (let ((event (car tail
    (closure (t) nil (let ((tail (list 7 'escape 27))) (while tail (let
    ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
    ert-run-test(#s(ert-test :name register-test-bug27634 :documentation
    ert-run-or-rerun-test(#s(ert--stats :selector ... :tests ... :test-m
    ert-run-tests((not (or (tag :unstable) (tag :nativecomp))) #f(compil
    ert-run-tests-batch((not (or (tag :unstable) (tag :nativecomp))))
    ert-run-tests-batch-and-exit((not (or (tag :unstable) (tag :nativeco
    eval((ert-run-tests-batch-and-exit '(not (or (tag :unstable) (tag :n
    command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/register-tests.el" "
    command-line()
    normal-top-level()
  Test register-test-bug27634 condition:
      (ert-test-failed
       ((should (equal 'quit (condition-case err ... ...))) :form
	(equal quit #<marker in no buffer>) :value nil :explanation
	(different-types quit #<marker in no buffer>)))
     FAILED  1/1  register-test-bug27634 (1.758961 sec) at lisp/register-tests.el:30

The response "a" to the "Point to register:" question is something I
needed to type; previously the test never asked any questions (and it
shouldn't, AFAIU).

Could you please see if the test suite needs some adaptations to these
changes?

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

[Thierry Volpiatto]

[-- Attachment #1: Type: text/plain, Size: 1386 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: Sat, 25 Nov 2023 21:14:31 +0000
>> 
>> Here a patch with the change suggested by Stefan applied, slighly
>> modified though because it fails if I put the call to
>> cl--generic-prefill-dispatchers at the recommended place.
>> 
>> I recompiled Emacs with this patch with no errors.
>
> Thanks, installed on master.

Thanks.

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

> Could you please see if the test suite needs some adaptations to these
> changes?

If you really need this test, it needs a complete rewrite.

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.

Thanks.

-- 
Thierry

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

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

[Eli Zaretskii]

> 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?  If this cannot happen, then
perhaps the whole test file should be removed?

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.

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

Thanks.

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

[Thierry Volpiatto]

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

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

[Thierry Volpiatto]

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

Eli Zaretskii <eliz@gnu.org> writes:

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

Here again the patchs (0002 with a little modification).
Let me know if I can commit them.

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: 1206 bytes --]

From 0fa70dad21d3475d3a5dae54a09d8a9e60b668ae 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".
However the key description is still displayed in a display property.

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

diff --git a/lisp/register.el b/lisp/register.el
index 88d0e8e1d10..46ec38821e5 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -131,7 +131,8 @@ 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))
+	  (propertize (string (car r))
+                      'display (single-key-description (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 408126b6d56a0cc36f621348212e16d0715fd671 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 cd6e66f955d20d31686a617ed8a5cd043585c71f 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 --]

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

[Eli Zaretskii]

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: monnier@iro.umontreal.ca,  michael_heerdegen@web.de,
>   stefankangas@gmail.com,  66394@debbugs.gnu.org
> Date: Sat, 02 Dec 2023 05:51:45 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > 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.
> 
> Here again the patchs (0002 with a little modification).
> Let me know if I can commit them.

Please install on the master branch, and thanks.

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

[Thierry Volpiatto]

[-- Attachment #1: Type: text/plain, Size: 662 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: Sat, 02 Dec 2023 05:51:45 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > 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.
>> 
>> Here again the patchs (0002 with a little modification).
>> Let me know if I can commit them.
>
> Please install on the master branch, and thanks.

Done, thanks.

-- 
Thierry

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

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

[Bastien]

I use registers ~100 times a day, so enhancements here are very
welcome, thanks!

Thierry Volpiatto <thievol@posteo.net> writes:

> A minibuffer is used now instead of read-key.

I wonder about this, though. It badly hinders my usual flow, where I
do remember what registers I use and like to store new ones quickly.

When a register is empty, I believe it's more efficient to just read
the key and store the content in the register directly.

E.g. if the "a" contains "A string" and "b" is an empty register:

- C-x r s would display the preview and copy the region to the "b"
  register as soon as the "b" key is hit (using read-key).

- C-x r s would display the preview and if the user hits "a", it will
  warn about overwriting the existing register and RET can confirm.

This supposes using read-key by default and switch to using a
minibuffer when the user hits keys for existing registers.

What do you think?

-- 
 Bastien

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

[Thierry Volpiatto]

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

Bastien <bzg@gnu.org> writes:

> I use registers ~100 times a day, so enhancements here are very
> welcome, thanks!
>
> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> A minibuffer is used now instead of read-key.
>
> I wonder about this, though. It badly hinders my usual flow, where I
> do remember what registers I use and like to store new ones quickly.
>
> When a register is empty, I believe it's more efficient to just read
> the key and store the content in the register directly.
>
> E.g. if the "a" contains "A string" and "b" is an empty register:
>
> - C-x r s would display the preview and copy the region to the "b"
>   register as soon as the "b" key is hit (using read-key).

I suggest you use M-n RET instead if you want to be sure you don't
overwrite a register.
Also don't forget you can now use C-n/p or <up>/<down> to navigate in preview.

> - C-x r s would display the preview and if the user hits "a", it will
>   warn about overwriting the existing register and RET can confirm.

It is what it is doing actually with minibuffer.  Hitting "a" highlight
register "a" and send a message "overwriting register", then you can hit
RET if you want to overwrite.

> This supposes using read-key by default and switch to using a
> minibuffer when the user hits keys for existing registers.
>
> What do you think?

I think using read-key+minibuffer would be very complicated and would
need much more code, this for a small benefit: Saving one key (RET).
Also I think hitting RET in any case is better as it does a kind of
"confirm I want to do this".
Also using read-key leads to bug like we had previously as we must mimic
a keymap which is often wrong.

Thanks.

-- 
Thierry

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

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

[Bastien Guerry]

Hi Thierry,

thanks for your anwer.

Thierry Volpiatto <thievol@posteo.net> writes:

>> - C-x r s would display the preview and copy the region to the "b"
>>   register as soon as the "b" key is hit (using read-key).
>
> I suggest you use M-n RET instead if you want to be sure you don't
> overwrite a register.

What I am suggesting is to store the register _as soon as_ the user
hits the "b" key.

Since the recent changes, I need to hit one additional keystroke for
zero benefit, which is a net less when you use registers a lot.

I use "a", "b", "c" registers for quick copy and paste and can easily
remember them; when I need more, I use register-list.el.

> It is what it is doing actually with minibuffer.  Hitting "a" highlight
> register "a" and send a message "overwriting register", then you can hit
> RET if you want to overwrite.

This might be useful in some cases. I don't suggest to change this. I
suggest to allow the previous behavior for empty registers.

> I think using read-key+minibuffer would be very complicated and would
> need much more code, this for a small benefit: Saving one key (RET).

I would say this is not a small benefit.

> Also I think hitting RET in any case is better as it does a kind of
> "confirm I want to do this".

IMHO confirmation is good for cases where mistakes can have bad
consequences.  I don't see them when using an empty register.

> Also using read-key leads to bug like we had previously as we must mimic
> a keymap which is often wrong.

I know there are always trade-offs. I just wanted to report the slight
"eww" moment I had wrt this UX change, which I still think is wrong.

If we set this issue aside, I wonder if read-key could be augmented so
that certain keystrokes let the user enter in "editing mode" (a bit
like when users hit C-s then C-e to edit the search string.) I can see
several situations where a read-key prompt would benefit from allowing
to switch to a minibuffer prompt with all the flexibility it provides:

- Allowing for confirmation when overwriting a register is one;

- Allowing to hit two keystrokes to facilitate navigation for C-h:
  e.g. `C-h k l' would list keybindings; `C-h k d` would describe a
  keybinding, etc.

This touches explorations that perhaps Jonas made while designing
transient, so I'm adding him to this conversation.
  
-- 
 Bastien Guerry

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

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

Bastien Guerry <bzg@gnu.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>> - C-x r s would display the preview and copy the region to the "b"
>>>   register as soon as the "b" key is hit (using read-key).
>>
>> I suggest you use M-n RET instead if you want to be sure you don't
>> overwrite a register.
>
> What I am suggesting is to store the register _as soon as_ the user
> hits the "b" key.
>
> Since the recent changes, I need to hit one additional keystroke for
> zero benefit, which is a net less when you use registers a lot.
>
> I use "a", "b", "c" registers for quick copy and paste and can easily
> remember them; when I need more, I use register-list.el.
>
>> It is what it is doing actually with minibuffer.  Hitting "a" highlight
>> register "a" and send a message "overwriting register", then you can hit
>> RET if you want to overwrite.
>
> This might be useful in some cases. I don't suggest to change this. I
> suggest to allow the previous behavior for empty registers.
>
>> I think using read-key+minibuffer would be very complicated and would
>> need much more code, this for a small benefit: Saving one key (RET).
>
> I would say this is not a small benefit.

FWIW, I second Bastien's request to restore the existing behavior as the
default, or at least provide it as an option.  I think Michael requested
that as well in the beginning of this thread.

This patch brings some nice benefits, but it also presents a regression
in terms of UX, perhaps we could avoid that or make it opt-in?  It would
also be great if there were a NEWS entry that clearly describes the user
visible changes.


Thanks,

Eshel

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

[Thierry Volpiatto]

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


Hi Bastien and Eshel,

Bastien Guerry <bzg@gnu.org> writes:

> Hi Thierry,
>
> thanks for your anwer.
>
> Thierry Volpiatto <thievol@posteo.net> writes:
>
>>> - C-x r s would display the preview and copy the region to the "b"
>>>   register as soon as the "b" key is hit (using read-key).
>>
>> I suggest you use M-n RET instead if you want to be sure you don't
>> overwrite a register.
>
> What I am suggesting is to store the register _as soon as_ the user
> hits the "b" key.
>
> Since the recent changes, I need to hit one additional keystroke for
> zero benefit, which is a net less when you use registers a lot.
>
> I use "a", "b", "c" registers for quick copy and paste and can easily
> remember them; when I need more, I use register-list.el.

I see, you want to go fast.
Using read-key+minibuffer, as I said would be a pain to implement,
hopefully (IIUC) we can use only minibuffer and exit immediately and it
is simple to implement (3 lines):

1) Set register-use-preview to nil.
2) Apply this patch on register.el:

@@ -388,6 +388,3 @@
-                              (if (member pat strs)
-                                  (with-selected-window (minibuffer-window)
-                                    (minibuffer-message msg pat))
-                                (with-selected-window (minibuffer-window)
-                                  (minibuffer-message
-                                   "Register `%s' is empty" pat)))))))))
+                              (with-selected-window (minibuffer-window)
+                                (setq result pat)
+                                (exit-minibuffer))))))))

3) Try your "a", "b", "c" registers and also to set a new register, if
needed you can call C-h to have a preview, in this case you will have to
hit RET.

Let me know if this is acceptable for you.

Thanks.


-- 
Thierry

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

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

[Thierry Volpiatto]

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

Thierry Volpiatto <thievol@posteo.net> writes:

> Hi Bastien and Eshel,
>
> Bastien Guerry <bzg@gnu.org> writes:
>
>> Hi Thierry,
>>
>> thanks for your anwer.
>>
>> Thierry Volpiatto <thievol@posteo.net> writes:
>>
>>>> - C-x r s would display the preview and copy the region to the "b"
>>>>   register as soon as the "b" key is hit (using read-key).
>>>
>>> I suggest you use M-n RET instead if you want to be sure you don't
>>> overwrite a register.
>>
>> What I am suggesting is to store the register _as soon as_ the user
>> hits the "b" key.
>>
>> Since the recent changes, I need to hit one additional keystroke for
>> zero benefit, which is a net less when you use registers a lot.
>>
>> I use "a", "b", "c" registers for quick copy and paste and can easily
>> remember them; when I need more, I use register-list.el.
>
> I see, you want to go fast.
> Using read-key+minibuffer, as I said would be a pain to implement,
> hopefully (IIUC) we can use only minibuffer and exit immediately and it
> is simple to implement (3 lines):
>
> 1) Set register-use-preview to nil.
> 2) Apply this patch on register.el:
>
> @@ -388,6 +388,3 @@
> -                              (if (member pat strs)
> -                                  (with-selected-window (minibuffer-window)
> -                                    (minibuffer-message msg pat))
> -                                (with-selected-window (minibuffer-window)
> -                                  (minibuffer-message
> -                                   "Register `%s' is empty" pat)))))))))
> +                              (with-selected-window (minibuffer-window)
> +                                (setq result pat)
> +                                (exit-minibuffer))))))))
>
> 3) Try your "a", "b", "c" registers and also to set a new register, if
> needed you can call C-h to have a preview, in this case you will have to
> hit RET.
>
> Let me know if this is acceptable for you.
>
> Thanks.

Even better is this patch which ask for confirmation when overwriting a
register but not when jumping, inserting or setting a new register:

@@ -388,6 +388,10 @@
-                              (if (member pat strs)
-                                  (with-selected-window (minibuffer-window)
-                                    (minibuffer-message msg pat))
-                                (with-selected-window (minibuffer-window)
-                                  (minibuffer-message
-                                   "Register `%s' is empty" pat)))))))))
+                              (with-selected-window (minibuffer-window)
+                                (if (and (member pat strs) (memq act '(set modify)))
+                                    (with-selected-window (minibuffer-window)
+                                      (minibuffer-message msg pat))
+                                  ;; An empty register or an existing
+                                  ;; one but the action is insert or
+                                  ;; jump, don't ask for confirmation
+                                  ;; and exit immediately.
+                                  (setq result pat)
+                                  (exit-minibuffer)))))))))


-- 
Thierry

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

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

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

Thierry Volpiatto <thievol@posteo.net> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Hi Bastien and Eshel,
>>
>> I see, you want to go fast.

Thanks, but this not only about speed.
Using a minibuffer to read a key has other disadvantages: it limits
the range of keys that can be easily used as registers since now
specifying the `C-a` register is much harder (as Michael mentioned).
This also makes register commands less convenient or and even
impossible (when `enable-recursive-minibuffers` is nil) to use inside
an existing minibuffer.

>> Using read-key+minibuffer, as I said would be a pain to implement,
>> hopefully (IIUC) we can use only minibuffer and exit immediately and it
>> is simple to implement (3 lines):
>>
>> 1) Set register-use-preview to nil.

Hmm, but this disables the automatic register preview, which I still want
to see, preferably after a short delay, just like before.

>> 2) Apply this patch on register.el:
>> ...
>>
>> Let me know if this is acceptable for you.
>
> Even better is this patch which ask for confirmation when overwriting a
> register but not when jumping, inserting or setting a new register:
>
> ...

That's improves things a bit, but see above.  I can see how confirmation
could be helpful for some people, but not everyone would appreciate the
added friction.  I basically suggest reverting to the previous behavior
(or otherwise restoring full compatibility), and making any involvement
of the minibuffer in reading registers optional, for example by
providing a new user option `register-read-from-minibuffer`.


Best,

Eshel

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

[Bastien Guerry]

Thierry Volpiatto <thievol@posteo.net> writes:

> Even better is this patch which ask for confirmation when overwriting a
> register but not when jumping, inserting or setting a new register:

Yes, this goes in the right direction, thanks!

The patch works fine when `register-use-preview' is nil. (I wonder if
the prompt "Register `%s' is empty" is not still displayed, though: I
believe I've seen it flash.)

When `register-use-preview' is t (the default), it skips confirmation
only when setting the _first_ empty register: when setting another
empty register, confirmation reappears.  Can you reproduce this bug?

-- 
 Bastien Guerry

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

[Thierry Volpiatto]

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

Bastien Guerry <bzg@gnu.org> writes:

> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Even better is this patch which ask for confirmation when overwriting a
>> register but not when jumping, inserting or setting a new register:
>
> Yes, this goes in the right direction, thanks!
>
> The patch works fine when `register-use-preview' is nil. (I wonder if
> the prompt "Register `%s' is empty" is not still displayed, though: I
> believe I've seen it flash.)
>
> When `register-use-preview' is t (the default), it skips confirmation
> only when setting the _first_ empty register: when setting another
> empty register, confirmation reappears.  Can you reproduce this bug?

No, can you give me a recipe.

Thanks.

-- 
Thierry

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

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

[Thierry Volpiatto]

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


Hello Eli,

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: Sat, 02 Dec 2023 05:51:45 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > 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.
>> 
>> Here again the patchs (0002 with a little modification).
>> Let me know if I can commit them.
>
> Please install on the master branch, and thanks.

Here another change related to the discussion in previous posts.
If no objections I will merge it in next days.

Thanks.

-- 
Thierry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Exit-with-no-confirmation-RET-when-register-use-prev.patch --]
[-- Type: text/x-diff, Size: 2247 bytes --]

From ff8f43a39e2cee1f71629194d44c7459f2b90d79 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Sun, 3 Dec 2023 15:21:50 +0100
Subject: [PATCH] Exit with no confirmation (RET) when register-use-preview

is non nil and .

This is done by exiting minibuffer when selected register is empty or
when just jumping or inserting.

* lisp/register.el (register-read-with-preview): Do it.
---
 lisp/register.el | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/lisp/register.el b/lisp/register.el
index 46ec38821e5..a38b531dfc9 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -385,12 +385,16 @@ display such a window regardless."
                                         (minibuffer-message
                                          "Register `%s' is empty" pat))))))
                             (unless (string= pat "")
-                              (if (member pat strs)
-                                  (with-selected-window (minibuffer-window)
-                                    (minibuffer-message msg pat))
-                                (with-selected-window (minibuffer-window)
-                                  (minibuffer-message
-                                   "Register `%s' is empty" pat)))))))))
+                              (with-selected-window (minibuffer-window)
+                                (if (and (member pat strs) (memq act '(set modify)))
+                                    (with-selected-window (minibuffer-window)
+                                      (minibuffer-message msg pat))
+                                  ;; An empty register or an existing
+                                  ;; one but the action is insert or
+                                  ;; jump, don't ask for confirmation
+                                  ;; and exit immediately (bug#66394).
+                                  (setq result pat)
+                                  (exit-minibuffer)))))))))
              (setq result (read-from-minibuffer
                            prompt nil map nil nil (register-preview-get-defaults act))))
            (cl-assert (and result (not (string= result "")))
-- 
2.34.1


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

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

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

Hi Thierry,

Thierry Volpiatto <thievol@posteo.net> writes:

> Here another change related to the discussion in previous posts.
> If no objections I will merge it in next days.

This seems to disregard my comments entirely, I'm sure that's not
intentional, but I'd appreciate it if you could consider them.

To reiterate, I wrote:

> Using a minibuffer to read a key has other disadvantages [beyond the
> extra friction of confirming with RET]: it limits the range of keys
> that can be easily used as registers since now specifying the `C-a`
> register is much harder (as Michael mentioned).  This also makes
> register commands less convenient and even impossible (when
> `enable-recursive-minibuffers` is nil) to use inside an existing
> minibuffer.

So it'd be great to have the previous behavior available in Emacs 30.


Thanks,

Eshel

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

[Thierry Volpiatto]

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

Eshel Yaron <me@eshelyaron.com> writes:

> Hi Thierry,
>
> Thierry Volpiatto <thievol@posteo.net> writes:
>
>> Here another change related to the discussion in previous posts.
>> If no objections I will merge it in next days.
>
> This seems to disregard my comments entirely,

It doesn't.

> I'm sure that's not intentional, but I'd appreciate it if you could
> consider them.

This finish to fix the version of register-preview I wrote (the one you
don't like) regarding what bastien asked for.

> So it'd be great to have the previous behavior available in Emacs 30.

Sorry but I wont write this, it is not complicated to write but needs
works and attention and I spent enough time on this.

The only thing you mentionned I agree with is the necessity now to use C-q
to insert key sequence (note that C-n/p will insert it alone), but it is
not a big annoyance right? (most people don't use this, I don't for
one).  For the preview buffer not visible, note that you can pop to it
at any moment with C-h.  Perhaps you will get used to the new behavior
after some time, otherwise it is easy to revert completely my commits
(it is the development branch of emacs after all).

-- 
Thierry

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

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

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

Thierry Volpiatto <thievol@posteo.net> writes:

> Eshel Yaron <me@eshelyaron.com> writes:
>
>> So it'd be great to have the previous behavior available in Emacs 30.
>
> Sorry but I wont write this, it is not complicated to write but needs
> works and attention and I spent enough time on this.
>
> The only thing you mentionned I agree with is the necessity now to use C-q
> to insert key sequence (note that C-n/p will insert it alone), but it is
> not a big annoyance right? (most people don't use this, I don't for
> one).  For the preview buffer not visible, note that you can pop to it
> at any moment with C-h.

What about the fact that `C-x r s` and friends by default no longer work
in the minibuffer?

> Perhaps you will get used to the new behavior after some time,

Why can't I, and other users, have the previous behavior, though?  It's
great to innovate with new alternatives, but why should we break user
workflows in the process, without as much as a NEWS entry to warn them?

> otherwise it is easy to revert completely my commits (it is the
> development branch of emacs after all).

Seeing as you are not willing to make this change backward compatible, I
think that would make sense.  I don't have commit rights to emacs.git,
so I can't do that myself, though.

I do think it shouldn't be that hard to extend the previous
implementation to _optionally_ ask for confirmation before overwriting
register contents, without using the minibuffer.  That way we'd have the
new behavior that you want to introduce without the added breakage.
Would you be willing to test such a patch if I write it?


Thanks,

Eshel

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

[Eli Zaretskii]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: michael_heerdegen@web.de,  Eli Zaretskii <eliz@gnu.org>,
>   stefankangas@gmail.com,  monnier@iro.umontreal.ca,  66394@debbugs.gnu.org
> Date: Sun, 03 Dec 2023 19:29:19 +0100
> 
> > otherwise it is easy to revert completely my commits (it is the
> > development branch of emacs after all).
> 
> Seeing as you are not willing to make this change backward compatible, I
> think that would make sense.  I don't have commit rights to emacs.git,
> so I can't do that myself, though.

Thierry also said:

> > So it'd be great to have the previous behavior available in Emacs 30.
> 
> Sorry but I wont write this, it is not complicated to write but needs
> works and attention and I spent enough time on this.

So maybe a better way forward is for someone, perhaps you Eshel, to
add whatever is needed to provide optionally the previous behavior?

Would you like to work on that?

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

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

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

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: michael_heerdegen@web.de,  Eli Zaretskii <eliz@gnu.org>,
>>   stefankangas@gmail.com,  monnier@iro.umontreal.ca,  66394@debbugs.gnu.org
>> Date: Sun, 03 Dec 2023 19:29:19 +0100
>> 
>> > otherwise it is easy to revert completely my commits (it is the
>> > development branch of emacs after all).
>> 
>> Seeing as you are not willing to make this change backward compatible, I
>> think that would make sense.  I don't have commit rights to emacs.git,
>> so I can't do that myself, though.
>
> Thierry also said:
>
>> > So it'd be great to have the previous behavior available in Emacs 30.
>> 
>> Sorry but I wont write this, it is not complicated to write but needs
>> works and attention and I spent enough time on this.
>
> So maybe a better way forward is for someone, perhaps you Eshel, to
> add whatever is needed to provide optionally the previous behavior?
>
> Would you like to work on that?

Sure.  I'm attaching two patches, the first reverts to the previous
implementation, and the second adds optional (on by default)
confirmation and highlighting in the *Register Preview* buffer when you
are about to overwrite the contents of a register.

The idea is to provide the nice of enhancements from Thierry's patch via
more minimal changes, without switching to a minibuffer based approach,
and without breaking any existing behavior.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Revert-recent-register-preview-changes.patch --]
[-- Type: text/x-patch, Size: 19163 bytes --]

From 220c600dd8b57de5ff44974ecfddd6f36dc9c3cd Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Sun, 3 Dec 2023 20:02:42 +0100
Subject: [PATCH 1/2] Revert recent register preview changes

This reverts commits cd6e66f955d20d31686a617ed8a5cd043585c71f,
408126b6d56a0cc36f621348212e16d0715fd671,
0fa70dad21d3475d3a5dae54a09d8a9e60b668ae,
3df81fb5dc5809cab7843e5358c17d0039b55eb1,
589e6ae1fb983bfba42f20906773555037246e45.
---
 doc/emacs/regs.texi           |   5 +-
 etc/NEWS                      |   5 -
 lisp/emacs-lisp/cl-generic.el |   1 -
 lisp/register.el              | 298 +++++-----------------------------
 test/lisp/register-tests.el   |  43 +++++
 5 files changed, 84 insertions(+), 268 deletions(-)
 create mode 100644 test/lisp/register-tests.el

diff --git a/doc/emacs/regs.texi b/doc/emacs/regs.texi
index 5e5b7ae2b16..e52f68dd18e 100644
--- a/doc/emacs/regs.texi
+++ b/doc/emacs/regs.texi
@@ -16,8 +16,9 @@ Registers
 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{C-d} by using for
-example @key{C-q} @samp{C-d}.
+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}).
 
 @findex view-register
   A register can store a position, a piece of text, a rectangle, a
diff --git a/etc/NEWS b/etc/NEWS
index 29f4e5c0b66..af8e1049483 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1154,11 +1154,6 @@ 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/emacs-lisp/cl-generic.el b/lisp/emacs-lisp/cl-generic.el
index 0ef0d1e192a..56eb83e6f75 100644
--- a/lisp/emacs-lisp/cl-generic.el
+++ b/lisp/emacs-lisp/cl-generic.el
@@ -1379,7 +1379,6 @@ cl-generic-generalizers
 (cl--generic-prefill-dispatchers 0 integer)
 (cl--generic-prefill-dispatchers 1 integer)
 (cl--generic-prefill-dispatchers 0 cl--generic-generalizer integer)
-(cl--generic-prefill-dispatchers 0 (eql 'x) integer)
 
 ;;; Dispatch on major mode.
 
diff --git a/lisp/register.el b/lisp/register.el
index 46ec38821e5..ca6de450993 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -35,8 +35,6 @@
 
 ;; FIXME: Clean up namespace usage!
 
-(declare-function frameset-register-p "frameset")
-
 (cl-defstruct
   (registerv (:constructor nil)
 	     (:constructor registerv--make (&optional data print-func
@@ -100,15 +98,6 @@ register-preview-delay
   :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."
-  :type '(repeat string))
-
-(defcustom register-use-preview t
-  "Always show register preview when non nil."
-  :type 'boolean)
 
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
@@ -131,8 +120,7 @@ register-describe-oneline
 (defun register-preview-default (r)
   "Function that is the default value of the variable `register-preview-function'."
   (format "%s: %s\n"
-	  (propertize (string (car r))
-                      'display (single-key-description (car r)))
+	  (single-key-description (car r))
 	  (register-describe-oneline (car r))))
 
 (defvar register-preview-function #'register-preview-default
@@ -140,263 +128,53 @@ register-preview-function
 Called with one argument, a cons (NAME . CONTENTS) as found in `register-alist'.
 The function should return a string, the description of the argument.")
 
-(cl-defstruct register-preview-info
-  "Store data for a specific register command.
-TYPES are the types of register supported.
-MSG is the minibuffer message to send when a register is selected.
-ACT is the type of action the command is doing on register.
-SMATCH accept a boolean value to say if command accept non matching register."
-  types msg act smatch)
-
-(cl-defgeneric register-command-info (command)
-  "Returns a `register-preview-info' object storing data for COMMAND."
-  (ignore command))
-(cl-defmethod register-command-info ((_command (eql insert-register)))
-  (make-register-preview-info
-   :types '(string number)
-   :msg "Insert register `%s'"
-   :act 'insert
-   :smatch t))
-(cl-defmethod register-command-info ((_command (eql jump-to-register)))
-  (make-register-preview-info
-   :types  '(window frame marker kmacro
-             file buffer file-query)
-   :msg "Jump to register `%s'"
-   :act 'jump
-   :smatch t))
-(cl-defmethod register-command-info ((_command (eql view-register)))
-  (make-register-preview-info
-   :types '(all)
-   :msg "View register `%s'"
-   :act 'view
-   :smatch t))
-(cl-defmethod register-command-info ((_command (eql append-to-register)))
-  (make-register-preview-info
-   :types '(string number)
-   :msg "Append to register `%s'"
-   :act 'modify
-   :smatch t))
-(cl-defmethod register-command-info ((_command (eql prepend-to-register)))
-  (make-register-preview-info
-   :types '(string number)
-   :msg "Prepend to register `%s'"
-   :act 'modify
-   :smatch t))
-(cl-defmethod register-command-info ((_command (eql increment-register)))
-  (make-register-preview-info
-   :types '(string number)
-   :msg "Increment register `%s'"
-   :act 'modify
-   :smatch t))
-
-(defun register-preview-forward-line (arg)
-  "Move to next or previous line in register preview buffer.
-If ARG is positive goto next line, if negative to previous.
-Do nothing when defining or executing kmacros."
-  ;; Ensure user enter manually key in minibuffer when recording a macro.
-  (unless (or defining-kbd-macro executing-kbd-macro
-              (not (get-buffer-window "*Register Preview*" 'visible)))
-    (let ((fn (if (> arg 0) #'eobp #'bobp))
-          (posfn (if (> arg 0)
-                     #'point-min
-                     (lambda () (1- (point-max)))))
-          str)
-      (with-current-buffer "*Register Preview*"
-        (let ((ovs (overlays-in (point-min) (point-max)))
-              pos)
-          (goto-char (if ovs
-                         (overlay-start (car ovs))
-                         (point-min)))
-          (setq pos (point))
-          (and ovs (forward-line arg))
-          (when (and (funcall fn)
-                     (or (> arg 0) (eql pos (point))))
-            (goto-char (funcall posfn)))
-          (setq str (buffer-substring-no-properties
-                     (pos-bol) (1+ (pos-bol))))
-          (remove-overlays)
-          (with-selected-window (minibuffer-window)
-            (delete-minibuffer-contents)
-            (insert str)))))))
-
-(defun register-preview-next ()
-  "Goto next line in register preview buffer."
-  (interactive)
-  (register-preview-forward-line 1))
-
-(defun register-preview-previous ()
-  "Goto previous line in register preview buffer."
-  (interactive)
-  (register-preview-forward-line -1))
-
-(defun register-type (register)
-  "Return REGISTER type.
-Current register types actually returned are one of:
-- string
-- number
-- marker
-- buffer
-- file
-- file-query
-- window
-- frame
-- kmacro
-
-One can add new types to a specific command by defining a new `cl-defmethod'
-matching this command. Predicate for type in new `cl-defmethod' should
-satisfy `cl-typep' otherwise the new type should be defined with
-`cl-deftype'."
-  ;; Call register--type against the register value.
-  (register--type (if (consp (cdr register))
-                     (cadr register)
-                   (cdr register))))
-
-(cl-defgeneric register--type (regval)
-  "Returns type of register value REGVAL."
-  (ignore regval))
-
-(cl-defmethod register--type ((_regval string)) 'string)
-(cl-defmethod register--type ((_regval number)) 'number)
-(cl-defmethod register--type ((_regval marker)) 'marker)
-(cl-defmethod register--type ((_regval (eql 'buffer))) 'buffer)
-(cl-defmethod register--type ((_regval (eql 'file))) 'file)
-(cl-defmethod register--type ((_regval (eql 'file-query))) 'file-query)
-(cl-defmethod register--type ((_regval window-configuration)) 'window)
-(cl-deftype frame-register () '(satisfies frameset-register-p))
-(cl-defmethod register--type :extra "frame-register" (_regval) 'frame)
-(cl-deftype kmacro-register () '(satisfies kmacro-register-p))
-(cl-defmethod register--type :extra "kmacro-register" (_regval) 'kmacro)
-
-(defun register-of-type-alist (types)
-  "Filter `register-alist' according to TYPES."
-  (if (memq 'all types)
-      register-alist
-    (cl-loop for register in register-alist
-             when (memq (register-type register) types)
-             collect register)))
-
-(defun register-preview (buffer &optional show-empty types)
+(defun register-preview (buffer &optional show-empty)
   "Pop up a window showing the registers preview in BUFFER.
 If SHOW-EMPTY is non-nil, show the window even if no registers.
-Argument TYPES (a list) specify the types of register to show, when nil show all
-registers, see `register-type' for suitable types.
 Format of each entry is controlled by the variable `register-preview-function'."
-  (let ((registers (register-of-type-alist (or types '(all)))))
-    (when (or show-empty (consp registers))
-      (with-current-buffer-window
-        buffer
-        (cons 'display-buffer-below-selected
-	      '((window-height . fit-window-to-buffer)
-	        (preserve-size . (nil . t))))
-        nil
-        (with-current-buffer standard-output
-          (setq cursor-in-non-selected-windows nil)
-          (mapc (lambda (elem)
-                  (when (get-register (car elem))
-                    (insert (funcall register-preview-function elem))))
-                registers))))))
-
-(cl-defgeneric register-preview-get-defaults (action)
-  "Returns default registers according to ACTION."
-  (ignore action))
-(cl-defmethod register-preview-get-defaults ((_action (eql set)))
-  (cl-loop for s in register-preview-default-keys
-           unless (assoc (string-to-char s) register-alist)
-           collect s))
+  (when (or show-empty (consp register-alist))
+    (with-current-buffer-window
+     buffer
+     (cons 'display-buffer-below-selected
+	   '((window-height . fit-window-to-buffer)
+	     (preserve-size . (nil . t))))
+     nil
+     (with-current-buffer standard-output
+       (setq cursor-in-non-selected-windows nil)
+       (mapc (lambda (elem)
+               (when (get-register (car elem))
+                 (insert (funcall register-preview-function elem))))
+             register-alist)))))
 
 (defun register-read-with-preview (prompt)
   "Read and return a register name, possibly showing existing registers.
-Prompt with the string PROMPT.
+Prompt with the string PROMPT.  If `register-alist' and
+`register-preview-delay' are both non-nil, display a window
+listing existing registers after `register-preview-delay' seconds.
 If `help-char' (or a member of `help-event-list') is pressed,
 display such a window regardless."
   (let* ((buffer "*Register Preview*")
-         (pat "")
-         (map (let ((m (make-sparse-keymap)))
-                (set-keymap-parent m minibuffer-local-map)
-                m))
-         (data (register-command-info this-command))
-         types msg result timer act win strs smatch)
-    (if data
-        (setq types  (register-preview-info-types data)
-              msg    (register-preview-info-msg   data)
-              act    (register-preview-info-act   data)
-              smatch (register-preview-info-smatch data))
-      (setq types '(all)
-            msg   "Overwrite register `%s'"
-            act   'set))
-    (setq strs (mapcar (lambda (x)
-                         (string (car x)))
-                       (register-of-type-alist types)))
-    (when (and (memq act '(insert jump view)) (null strs))
-      (error "No register suitable for `%s'" act))
-    (dolist (k (cons help-char help-event-list))
-      (define-key map
-          (vector k) (lambda ()
-                       (interactive)
-                       (unless (get-buffer-window buffer)
-                         (with-selected-window (minibuffer-selected-window)
-                           (register-preview buffer 'show-empty types))))))
-    (define-key map (kbd "<down>") 'register-preview-next)
-    (define-key map (kbd "<up>")   'register-preview-previous)
-    (define-key map (kbd "C-n")    'register-preview-next)
-    (define-key map (kbd "C-p")    'register-preview-previous)
-    (unless (or executing-kbd-macro (null register-use-preview))
-      (register-preview buffer nil types))
+	 (timer (when (numberp register-preview-delay)
+		  (run-with-timer register-preview-delay nil
+				  (lambda ()
+				    (unless (get-buffer-window buffer)
+				      (register-preview buffer))))))
+	 (help-chars (cl-loop for c in (cons help-char help-event-list)
+			      when (not (get-register c))
+			      collect c)))
     (unwind-protect
-         (progn
-           (minibuffer-with-setup-hook
-               (lambda ()
-                 (setq timer
-                       (run-with-idle-timer
-                        0.01 'repeat
-                        (lambda ()
-                          (with-selected-window (minibuffer-window)
-                            (let ((input (minibuffer-contents)))
-                              (when (> (length input) 1)
-                                (let ((new (substring input 1))
-                                      (old (substring input 0 1)))
-                                  (setq input (if (or (null smatch)
-                                                      (member new strs))
-                                                  new old))
-                                  (delete-minibuffer-contents)
-                                  (insert input)))
-                              (when (and smatch (not (string= input ""))
-                                         (not (member input strs)))
-                                (setq input "")
-                                (delete-minibuffer-contents)
-                                (minibuffer-message "Not matching"))
-                              (when (not (string= input pat))
-                                (setq pat input))))
-                          (if (setq win (get-buffer-window buffer))
-                              (with-selected-window win
-                                (let ((ov (make-overlay (point-min) (point-min))))
-                                  (goto-char (point-min))
-                                  (remove-overlays)
-                                  (unless (string= pat "")
-                                    (if (re-search-forward (concat "^" pat) nil t)
-                                        (progn (move-overlay
-                                                ov
-                                                (match-beginning 0) (pos-eol))
-                                               (overlay-put ov 'face 'match)
-                                               (when msg
-                                                 (with-selected-window (minibuffer-window)
-                                                   (minibuffer-message msg pat))))
-                                      (with-selected-window (minibuffer-window)
-                                        (minibuffer-message
-                                         "Register `%s' is empty" pat))))))
-                            (unless (string= pat "")
-                              (if (member pat strs)
-                                  (with-selected-window (minibuffer-window)
-                                    (minibuffer-message msg pat))
-                                (with-selected-window (minibuffer-window)
-                                  (minibuffer-message
-                                   "Register `%s' is empty" pat)))))))))
-             (setq result (read-from-minibuffer
-                           prompt nil map nil nil (register-preview-get-defaults act))))
-           (cl-assert (and result (not (string= result "")))
-                      nil "No register specified")
-           (string-to-char result))
-      (when timer (cancel-timer timer))
+	(progn
+	  (while (memq (read-key (propertize prompt 'face 'minibuffer-prompt))
+		       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))
       (let ((w (get-buffer-window buffer)))
         (and (window-live-p w) (delete-window w)))
       (and (get-buffer buffer) (kill-buffer buffer)))))
diff --git a/test/lisp/register-tests.el b/test/lisp/register-tests.el
new file mode 100644
index 00000000000..6283d1c31e0
--- /dev/null
+++ b/test/lisp/register-tests.el
@@ -0,0 +1,43 @@
+;;; 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.42.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-Optionally-ask-for-confirmation-before-overwriting-r.patch --]
[-- Type: text/x-patch, Size: 13524 bytes --]

From d1538aadc4f3d0da6a8c550248f8d348edb96116 Mon Sep 17 00:00:00 2001
From: Eshel Yaron <me@eshelyaron.com>
Date: Sun, 3 Dec 2023 20:44:16 +0100
Subject: [PATCH 2/2] Optionally ask for confirmation before overwriting
 registers

Commands can now call 'register-read-with-preview' with optional
argument CONFIRM to ask the user for confirmation if they choose a
register that is already in use, subject to new user option
'register-confirm-overwrite'.  Commands that write to registers are
adapted to make use of this new argument.  When asking for
confirmation, Emacs also highlights the selected register in
the *Register Preview* buffer.

* lisp/register.el (register-confirm-overwrite): New user option.
(register-preview): New optional argument HIGHLIGHT.
(register-read-with-preview): Use them.  New optional arg CONFIRM.
(point-to-register,window-configuration-to-register)
(frame-configuration-to-register,number-to-register)
(copy-to-register,copy-rectangle-to-register)
* lisp/textmodes/picture.el (picture-clear-rectangle-to-register)
* lisp/calc/calc-yank.el (calc-copy-to-register)
* lisp/cedet/semantic/senator.el (senator-copy-tag-to-register)
* lisp/frameset.el (frameset-to-register)
* lisp/kmacro.el (kmacro-to-register)
* lisp/play/gametree.el (gametree-layout-to-register): Use new arg.
* doc/lispref/text.texi (Registers): Update.
* etc/NEWS: Announce.
---
 doc/lispref/text.texi          |  8 +++--
 etc/NEWS                       |  6 ++++
 lisp/calc/calc-yank.el         |  2 +-
 lisp/cedet/semantic/senator.el |  2 +-
 lisp/frameset.el               |  2 +-
 lisp/kmacro.el                 |  2 +-
 lisp/play/gametree.el          |  2 +-
 lisp/register.el               | 62 ++++++++++++++++++++++++----------
 lisp/textmodes/picture.el      |  2 +-
 9 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
index 5d05ef18d4f..9f5b846b92d 100644
--- a/doc/lispref/text.texi
+++ b/doc/lispref/text.texi
@@ -4710,7 +4710,7 @@ Registers
 changed in the future.
 @end deffn
 
-@defun register-read-with-preview prompt
+@defun register-read-with-preview prompt &optional confirm
 @cindex register preview
 This function reads and returns a register name, prompting with
 @var{prompt} and possibly showing a preview of the existing registers
@@ -4718,8 +4718,10 @@ Registers
 the delay specified by the user option @code{register-preview-delay},
 if its value and @code{register-alist} are both non-@code{nil}.  The
 preview is also shown if the user requests help (e.g., by typing the
-help character).  We recommend that all interactive commands which
-read register names use this function.
+help character).  If optional argument @var{confirm} is
+non-@code{nil}, this function asks for confirmation before returning a
+register that is already in use.  We recommend that all interactive
+commands which read register names use this function.
 @end defun
 
 @node Transposition
diff --git a/etc/NEWS b/etc/NEWS
index af8e1049483..0617c8dc218 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -1085,6 +1085,12 @@ macros with many lines, such as from 'kmacro-edit-lossage'.
 
 ** Miscellaneous
 
++++
+*** New user option 'register-confirm-overwrite'.
+Emacs now defaults to asking for confirmation before overwriting
+registers with existing contents.  To disable such confirmation,
+customize this option to nil.
+
 ---
 *** Webjump now assumes URIs are HTTPS instead of HTTP.
 For links in 'webjump-sites' without an explicit URI scheme, it was
diff --git a/lisp/calc/calc-yank.el b/lisp/calc/calc-yank.el
index a2a91dc8fb8..ed1a8e1c046 100644
--- a/lisp/calc/calc-yank.el
+++ b/lisp/calc/calc-yank.el
@@ -281,7 +281,7 @@ calc-copy-to-register
 With prefix arg, delete as well.
 
 Interactively, reads the register using `register-read-with-preview'."
-  (interactive (list (register-read-with-preview "Copy to register: ")
+  (interactive (list (register-read-with-preview "Copy to register: " t)
 		     (region-beginning) (region-end)
 		     current-prefix-arg))
   (if (eq major-mode 'calc-mode)
diff --git a/lisp/cedet/semantic/senator.el b/lisp/cedet/semantic/senator.el
index ca4334eaff5..2c1fc4fda3b 100644
--- a/lisp/cedet/semantic/senator.el
+++ b/lisp/cedet/semantic/senator.el
@@ -736,7 +736,7 @@ senator-copy-tag-to-register
 kill ring.
 
 Interactively, reads the register using `register-read-with-preview'."
-  (interactive (list (register-read-with-preview "Tag to register: ")
+  (interactive (list (register-read-with-preview "Tag to register: " t)
                      current-prefix-arg))
   (semantic-fetch-tags)
   (let ((ft (semantic-obtain-foreign-tag)))
diff --git a/lisp/frameset.el b/lisp/frameset.el
index 224746bbfe3..63ff4668541 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -1451,7 +1451,7 @@ frameset-to-register
 Argument is a character, naming the register.
 
 Interactively, reads the register using `register-read-with-preview'."
-  (interactive (list (register-read-with-preview "Frameset to register: ")))
+  (interactive (list (register-read-with-preview "Frameset to register: " t)))
   (set-register register
 		(frameset-make-register
                  (frameset-save nil
diff --git a/lisp/kmacro.el b/lisp/kmacro.el
index 588b2d14943..a7aa2c88508 100644
--- a/lisp/kmacro.el
+++ b/lisp/kmacro.el
@@ -967,7 +967,7 @@ kmacro-to-register
   (interactive
    (progn
      (or last-kbd-macro (error "No keyboard macro defined"))
-     (list (register-read-with-preview "Save to register: "))))
+     (list (register-read-with-preview "Save to register: " t))))
   (set-register r (kmacro-ring-head)))
 
 
diff --git a/lisp/play/gametree.el b/lisp/play/gametree.el
index 971d8ea70ca..e46770af2da 100644
--- a/lisp/play/gametree.el
+++ b/lisp/play/gametree.el
@@ -523,7 +523,7 @@ gametree-layout-to-register
 Argument is a character, naming the register.
 
 Interactively, reads the register using `register-read-with-preview'."
-  (interactive (list (register-read-with-preview "Layout to register: ")))
+  (interactive (list (register-read-with-preview "Layout to register: " t)))
   (save-excursion
     (goto-char (point-min))
     (set-register register
diff --git a/lisp/register.el b/lisp/register.el
index ca6de450993..4e400fbff2c 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -99,6 +99,12 @@ register-preview-delay
   :type '(choice number (const :tag "No preview unless requested" nil))
   :group 'register)
 
+(defcustom register-confirm-overwrite t
+  "Whether to ask for confirmation before overwriting register contents."
+  :version "30.1"
+  :type 'boolean
+  :group 'register)
+
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
   (alist-get register register-alist))
@@ -128,10 +134,12 @@ register-preview-function
 Called with one argument, a cons (NAME . CONTENTS) as found in `register-alist'.
 The function should return a string, the description of the argument.")
 
-(defun register-preview (buffer &optional show-empty)
+(defun register-preview (buffer &optional show-empty highlight)
   "Pop up a window showing the registers preview in BUFFER.
 If SHOW-EMPTY is non-nil, show the window even if no registers.
-Format of each entry is controlled by the variable `register-preview-function'."
+Optional argument HIGHLIGHT says to highlight the description of
+a register with that name.  Format of each entry is controlled by
+the variable `register-preview-function'."
   (when (or show-empty (consp register-alist))
     (with-current-buffer-window
      buffer
@@ -140,19 +148,26 @@ register-preview
 	     (preserve-size . (nil . t))))
      nil
      (with-current-buffer standard-output
+       (delete-region (point-min) (point-max))
        (setq cursor-in-non-selected-windows nil)
        (mapc (lambda (elem)
-               (when (get-register (car elem))
-                 (insert (funcall register-preview-function elem))))
+               (when-let ((name (car elem))
+                          (reg (get-register name))
+                          (desc (funcall register-preview-function elem)))
+                 (when (equal highlight name)
+                   (add-face-text-property 0 (length desc) 'match nil desc))
+                 (insert desc)))
              register-alist)))))
 
-(defun register-read-with-preview (prompt)
+(defun register-read-with-preview (prompt &optional confirm)
   "Read and return a register name, possibly showing existing registers.
-Prompt with the string PROMPT.  If `register-alist' and
+Prompt with the string PROMPT.  Optional argument CONFIRM says to
+ask for confirmation if the register is already in use and
+`register-confirm-overwrite' is non-nil.  If `register-alist' and
 `register-preview-delay' are both non-nil, display a window
-listing existing registers after `register-preview-delay' seconds.
-If `help-char' (or a member of `help-event-list') is pressed,
-display such a window regardless."
+listing existing registers after `register-preview-delay'
+seconds.  If `help-char' (or a member of `help-event-list') is
+pressed, display such a window regardless."
   (let* ((buffer "*Register Preview*")
 	 (timer (when (numberp register-preview-delay)
 		  (run-with-timer register-preview-delay nil
@@ -168,10 +183,20 @@ 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))
+          (cond
+           ((or (eq ?\C-g last-input-event)
+                (eq 'escape last-input-event)
+                (eq ?\C-\[ last-input-event))
             (keyboard-quit))
+           ((and (get-register last-input-event)
+                 confirm register-confirm-overwrite
+                 (not (progn
+                        (register-preview buffer nil last-input-event)
+                        (y-or-n-p (substitute-quotes
+                                   (format "Overwrite register `%s'?"
+                                           (single-key-description
+                                            last-input-event))))))
+                 (user-error "Register already in use"))))
 	  (if (characterp last-input-event) last-input-event
 	    (error "Non-character input-event")))
       (and (timerp timer) (cancel-timer timer))
@@ -189,7 +214,8 @@ point-to-register
   (interactive (list (register-read-with-preview
                       (if current-prefix-arg
                           "Frame configuration to register: "
-                        "Point to register: "))
+                        "Point to register: ")
+                      t)
                      current-prefix-arg))
   ;; Turn the marker into a file-ref if the buffer is killed.
   (add-hook 'kill-buffer-hook 'register-swap-out nil t)
@@ -204,7 +230,7 @@ window-configuration-to-register
 
 Interactively, prompt for REGISTER using `register-read-with-preview'."
   (interactive (list (register-read-with-preview
-		      "Window configuration to register: ")
+		      "Window configuration to register: " t)
 		     current-prefix-arg))
   ;; current-window-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
@@ -222,7 +248,7 @@ frame-configuration-to-register
 
 Interactively, prompt for REGISTER using `register-read-with-preview'."
   (interactive (list (register-read-with-preview
-		      "Frame configuration to register: ")
+		      "Frame configuration to register: " t)
 		     current-prefix-arg))
   ;; current-frame-configuration does not include the value
   ;; of point in the current buffer, so record that separately.
@@ -316,7 +342,7 @@ number-to-register
 
 Interactively, prompt for REGISTER using `register-read-with-preview'."
   (interactive (list current-prefix-arg
-		     (register-read-with-preview "Number to register: ")))
+		     (register-read-with-preview "Number to register: " t)))
   (set-register register
 		(if number
 		    (prefix-numeric-value number)
@@ -527,7 +553,7 @@ copy-to-register
 Interactively, prompt for REGISTER using `register-read-with-preview'
 and use mark and point as START and END; REGION is always non-nil in
 this case."
-  (interactive (list (register-read-with-preview "Copy to register: ")
+  (interactive (list (register-read-with-preview "Copy to register: " t)
 		     (region-beginning)
 		     (region-end)
 		     current-prefix-arg
@@ -605,7 +631,7 @@ copy-rectangle-to-register
 Interactively, prompt for REGISTER using `register-read-with-preview',
 and use mark and point as START and END."
   (interactive (list (register-read-with-preview
-		      "Copy rectangle to register: ")
+		      "Copy rectangle to register: " t)
 		     (region-beginning)
 		     (region-end)
 		     current-prefix-arg))
diff --git a/lisp/textmodes/picture.el b/lisp/textmodes/picture.el
index f98c3963b6f..efa59e0682f 100644
--- a/lisp/textmodes/picture.el
+++ b/lisp/textmodes/picture.el
@@ -503,7 +503,7 @@ picture-clear-rectangle-to-register
 
 Interactively, reads the register using `register-read-with-preview'."
   (interactive (list (region-beginning) (region-end)
-		     (register-read-with-preview "Rectangle to register: ")
+		     (register-read-with-preview "Rectangle to register: " t)
 		     current-prefix-arg))
   (set-register register (picture-snarf-rectangle start end killp)))
 
-- 
2.42.0

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

[Thierry Volpiatto]

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

Eshel Yaron <me@eshelyaron.com> writes:

> The idea is to provide the nice of enhancements from Thierry's patch via
> more minimal changes,

Your patch fails to provide all the enhancements I had provided.

- No filtering.
- No navigation.
- No default registers.
- No possibility to configure a new command added to register.
- You reintroduced the old implementation which was not wrote correctly
about handling various keys, particularly C-g.
- About overwriting, now you have to press "y" to confirm instead of
pressing RET.

-- 
Thierry

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

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

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

Hi Thierry,


Thierry Volpiatto <thievol@posteo.net> writes:

> Eshel Yaron <me@eshelyaron.com> writes:
>
>> The idea is to provide the nice of enhancements from Thierry's patch via
>> more minimal changes,
>
> Your patch fails to provide all the enhancements I had provided.

Thanks for testing.

Indeed, I only reimplemented the parts I saw as clearly beneficial.
Most importantly, my patch improves stuff without breaking other stuff.

Perhaps you can explain your use case for the rest of the changes, and
if there's a good and compatible way to add them I'll be happy to look
into it at some point.

> - No filtering.
> - No navigation.
> - No default registers.
> - No possibility to configure a new command added to register.

If you could elaborate about these bullets, and explain their use,
that'd be great.

> - You reintroduced the old implementation which was not wrote correctly
> about handling various keys, particularly C-g.

How so?  Works well over here, AFAICT, as it did in Emacs 29.

> - About overwriting, now you have to press "y" to confirm instead of
> pressing RET.

Yes, I think `y-or-n-p` is more user friendly here, don't you?


Cheers,

Eshel