GNU bug report logs -
#66394
29.1; Make register-read-with-preview more useful
Previous Next
Reported by: Thierry Volpiatto <thievol <at> posteo.net>
Date: Sat, 7 Oct 2023 19:07:01 UTC
Severity: normal
Found in version 29.1
Done: Eli Zaretskii <eliz <at> gnu.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66394 in the body.
You can then email your comments to 66394 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 07 Oct 2023 19:07:01 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Thierry Volpiatto <thievol <at> posteo.net>:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org.
(Sat, 07 Oct 2023 19:07:01 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 08 Oct 2023 06:50:02 GMT)
Full text and
rfc822 format available.
Message #8 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 12 Oct 2023 06:54:03 GMT)
Full text and
rfc822 format available.
Message #11 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 14 Oct 2023 02:06:01 GMT)
Full text and
rfc822 format available.
Message #14 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[[[ 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)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 14 Oct 2023 06:07:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Richard Stallman <rms <at> 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
[signature.asc (application/pgp-signature, inline)]
Reply sent
to
Thierry Volpiatto <thievol <at> posteo.net>:
You have taken responsibility.
(Sun, 15 Oct 2023 08:00:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Thierry Volpiatto <thievol <at> posteo.net>:
bug acknowledged by developer.
(Sun, 15 Oct 2023 08:00:02 GMT)
Full text and
rfc822 format available.
Message #22 received at 66394-done <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Closed for no interest in this feature.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 15 Oct 2023 08:19:02 GMT)
Full text and
rfc822 format available.
Message #25 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 15 Oct 2023 10:07:02 GMT)
Full text and
rfc822 format available.
Message #28 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 15 Oct 2023 12:57:02 GMT)
Full text and
rfc822 format available.
Message #31 received at 66394 <at> debbugs.gnu.org (full text, mbox):
reopen 66394
thanks
Thierry Volpiatto <thievol <at> posteo.net> writes:
>> If it's fine by you, I'd like to reopen this bug report.
>
> Ok, please do.
Done, thanks.
Did not alter fixed versions and reopened.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sun, 15 Oct 2023 12:57:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 16 Oct 2023 02:05:01 GMT)
Full text and
rfc822 format available.
Message #36 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[[[ 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)
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 19 Oct 2023 02:44:02 GMT)
Full text and
rfc822 format available.
Message #39 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 19 Oct 2023 06:38:01 GMT)
Full text and
rfc822 format available.
Message #42 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Michael,
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 20 Oct 2023 05:02:02 GMT)
Full text and
rfc822 format available.
Message #45 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 20 Oct 2023 06:04:02 GMT)
Full text and
rfc822 format available.
Message #48 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 21 Oct 2023 01:11:01 GMT)
Full text and
rfc822 format available.
Message #51 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 21 Oct 2023 03:57:02 GMT)
Full text and
rfc822 format available.
Message #54 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 23 Oct 2023 04:11:02 GMT)
Full text and
rfc822 format available.
Message #57 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 23 Oct 2023 05:31:01 GMT)
Full text and
rfc822 format available.
Message #60 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 24 Oct 2023 03:44:02 GMT)
Full text and
rfc822 format available.
Message #63 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 24 Oct 2023 03:55:02 GMT)
Full text and
rfc822 format available.
Message #66 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Michael Heerdegen <michael_heerdegen <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 24 Oct 2023 05:41:02 GMT)
Full text and
rfc822 format available.
Message #69 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 24 Oct 2023 07:29:02 GMT)
Full text and
rfc822 format available.
Message #72 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 25 Oct 2023 03:56:01 GMT)
Full text and
rfc822 format available.
Message #75 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 25 Oct 2023 04:11:01 GMT)
Full text and
rfc822 format available.
Message #78 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 25 Oct 2023 07:11:01 GMT)
Full text and
rfc822 format available.
Message #81 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 26 Oct 2023 04:19:02 GMT)
Full text and
rfc822 format available.
Message #84 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 26 Oct 2023 08:03:01 GMT)
Full text and
rfc822 format available.
Message #87 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 27 Oct 2023 01:29:02 GMT)
Full text and
rfc822 format available.
Message #90 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 27 Oct 2023 04:29:01 GMT)
Full text and
rfc822 format available.
Message #93 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 03 Nov 2023 05:00:02 GMT)
Full text and
rfc822 format available.
Message #96 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 18 Nov 2023 18:41:01 GMT)
Full text and
rfc822 format available.
Message #99 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Stefan,
Stefan Kangas <stefankangas <at> gmail.com> writes:
> reopen 66394
> thanks
>
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 19 Nov 2023 19:38:01 GMT)
Full text and
rfc822 format available.
Message #102 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Michael Heerdegen <michael_heerdegen <at> web.de> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 20 Nov 2023 06:01:01 GMT)
Full text and
rfc822 format available.
Message #105 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
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.
[0001-Improve-register-preview-Fix-bug-66394.patch (text/x-diff, attachment)]
[Message part 3 (text/plain, inline)]
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 20 Nov 2023 17:37:02 GMT)
Full text and
rfc822 format available.
Message #108 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 20 Nov 2023 18:52:02 GMT)
Full text and
rfc822 format available.
Message #111 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Stefan,
Stefan Monnier <monnier <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 25 Nov 2023 10:24:01 GMT)
Full text and
rfc822 format available.
Message #114 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, stefankangas <at> gmail.com,
> 66394 <at> debbugs.gnu.org
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Mon, 20 Nov 2023 18:51:10 +0000
>
> Stefan Monnier <monnier <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 25 Nov 2023 20:00:02 GMT)
Full text and
rfc822 format available.
Message #117 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Eli,
Eli Zaretskii <eliz <at> gnu.org> writes:
>> Cc: Michael Heerdegen <michael_heerdegen <at> web.de>, stefankangas <at> gmail.com,
>> 66394 <at> debbugs.gnu.org
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Date: Mon, 20 Nov 2023 18:51:10 +0000
>>
>> Stefan Monnier <monnier <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 25 Nov 2023 20:11:02 GMT)
Full text and
rfc822 format available.
Message #120 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 25 Nov 2023 21:15:02 GMT)
Full text and
rfc822 format available.
Message #123 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> 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
[0001-Improve-register-preview-Fix-bug-66394.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 25 Nov 2023 21:39:01 GMT)
Full text and
rfc822 format available.
Message #126 received at 66394 <at> debbugs.gnu.org (full text, mbox):
>> > 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 26 Nov 2023 10:39:02 GMT)
Full text and
rfc822 format available.
Message #129 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 26 Nov 2023 16:47:02 GMT)
Full text and
rfc822 format available.
Message #132 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 29 Nov 2023 14:05:02 GMT)
Full text and
rfc822 format available.
Message #135 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 29 Nov 2023 18:20:02 GMT)
Full text and
rfc822 format available.
Message #138 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 30 Nov 2023 06:01:01 GMT)
Full text and
rfc822 format available.
Message #141 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
> Date: Wed, 29 Nov 2023 18:18:40 +0000
>
> Eli Zaretskii <eliz <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 30 Nov 2023 10:23:01 GMT)
Full text and
rfc822 format available.
Message #144 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
>> Date: Wed, 29 Nov 2023 18:18:40 +0000
>>
>> Eli Zaretskii <eliz <at> 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
[0001-Make-register-preview-delay-obsolete.patch (text/x-diff, attachment)]
[0002-Fix-register-preview-default.patch (text/x-diff, attachment)]
[0003-Update-register-manual.patch (text/x-diff, attachment)]
[0004-Delete-register-tests.el-now-no-more-needed.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 05:53:02 GMT)
Full text and
rfc822 format available.
Message #147 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> 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
[0001-Make-register-preview-delay-obsolete.patch (text/x-diff, attachment)]
[0002-Fix-register-preview-default.patch (text/x-diff, attachment)]
[0003-Update-register-manual.patch (text/x-diff, attachment)]
[0004-Delete-register-tests.el-now-no-more-needed.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 07:51:01 GMT)
Full text and
rfc822 format available.
Message #150 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
> Date: Sat, 02 Dec 2023 05:51:45 +0000
>
> Eli Zaretskii <eliz <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 08:09:02 GMT)
Full text and
rfc822 format available.
Message #153 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
>> Date: Sat, 02 Dec 2023 05:51:45 +0000
>>
>> Eli Zaretskii <eliz <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 09:26:02 GMT)
Full text and
rfc822 format available.
Message #156 received at 66394 <at> debbugs.gnu.org (full text, mbox):
I use registers ~100 times a day, so enhancements here are very
welcome, thanks!
Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 09:54:01 GMT)
Full text and
rfc822 format available.
Message #159 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Bastien <bzg <at> gnu.org> writes:
> I use registers ~100 times a day, so enhancements here are very
> welcome, thanks!
>
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 10:39:02 GMT)
Full text and
rfc822 format available.
Message #162 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Hi Thierry,
thanks for your anwer.
Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 10:55:02 GMT)
Full text and
rfc822 format available.
Message #165 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Bastien Guerry <bzg <at> gnu.org> writes:
> Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 11:57:02 GMT)
Full text and
rfc822 format available.
Message #168 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Bastien and Eshel,
Bastien Guerry <bzg <at> gnu.org> writes:
> Hi Thierry,
>
> thanks for your anwer.
>
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 12:44:02 GMT)
Full text and
rfc822 format available.
Message #171 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Hi Bastien and Eshel,
>
> Bastien Guerry <bzg <at> gnu.org> writes:
>
>> Hi Thierry,
>>
>> thanks for your anwer.
>>
>> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 13:04:01 GMT)
Full text and
rfc822 format available.
Message #174 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 13:52:02 GMT)
Full text and
rfc822 format available.
Message #177 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 02 Dec 2023 15:03:01 GMT)
Full text and
rfc822 format available.
Message #180 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Bastien Guerry <bzg <at> gnu.org> writes:
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 14:37:02 GMT)
Full text and
rfc822 format available.
Message #183 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Eli,
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
>> Date: Sat, 02 Dec 2023 05:51:45 +0000
>>
>> Eli Zaretskii <eliz <at> 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
[0001-Exit-with-no-confirmation-RET-when-register-use-prev.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 15:06:02 GMT)
Full text and
rfc822 format available.
Message #186 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Hi Thierry,
Thierry Volpiatto <thievol <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 16:49:01 GMT)
Full text and
rfc822 format available.
Message #189 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eshel Yaron <me <at> eshelyaron.com> writes:
> Hi Thierry,
>
> Thierry Volpiatto <thievol <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 18:30:02 GMT)
Full text and
rfc822 format available.
Message #192 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Eshel Yaron <me <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 18:41:02 GMT)
Full text and
rfc822 format available.
Message #195 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Eshel Yaron <me <at> eshelyaron.com>
> Cc: michael_heerdegen <at> web.de, Eli Zaretskii <eliz <at> gnu.org>,
> stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca, 66394 <at> 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?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 03 Dec 2023 21:24:01 GMT)
Full text and
rfc822 format available.
Message #198 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi Eli,
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Eshel Yaron <me <at> eshelyaron.com>
>> Cc: michael_heerdegen <at> web.de, Eli Zaretskii <eliz <at> gnu.org>,
>> stefankangas <at> gmail.com, monnier <at> iro.umontreal.ca, 66394 <at> 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.
[0001-Revert-recent-register-preview-changes.patch (text/x-patch, attachment)]
[0002-Optionally-ask-for-confirmation-before-overwriting-r.patch (text/x-patch, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 04 Dec 2023 07:31:02 GMT)
Full text and
rfc822 format available.
Message #201 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eshel Yaron <me <at> 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
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 04 Dec 2023 07:59:01 GMT)
Full text and
rfc822 format available.
Message #204 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Hi Thierry,
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Eshel Yaron <me <at> 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
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 05 Dec 2023 07:35:02 GMT)
Full text and
rfc822 format available.
Message #207 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Michael Heerdegen <michael_heerdegen <at> web.de> 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.
> But wait: What I find confusing is that I also need need to confirm for
>`jump-to-register'. Is this intended?
Please consider revising the patch to maintain backward compatibility. I
find the fact that jump-to-register works without confirmation to be a
very useful feature.
Thank you!
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 05 Dec 2023 07:40:01 GMT)
Full text and
rfc822 format available.
Message #210 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Michael Heerdegen <michael_heerdegen <at> web.de>
>> 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.
>But wait: What I find confusing is that I also need need to confirm for
>`jump-to-register'. Is this intended?
Please consider revising the patch to maintain backward compatibility. I
consider the fact that jump-to-register works without confirmation to be
an important feature.
Thank you!
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 05 Dec 2023 07:45:02 GMT)
Full text and
rfc822 format available.
Message #213 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Tino Calancha <tino.calancha <at> gmail.com> writes:
> Please consider revising the patch to maintain backward compatibility. I
> consider the fact that jump-to-register works without confirmation to be
> an important feature.
Same for `point-to-register`. I prefer the bahavior w/o a confirmation
need.
Thank you!
Tino
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 06:56:02 GMT)
Full text and
rfc822 format available.
Message #216 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Eli,
I tried to fullfill the requests on Emacs-devel about overwriting a
register with no confirmation, this will happen when
register-use-preview is nil or 'never. Even with this configuration we
still have filtering when preview buffer is visible and in any case we
have defaults with M-n available when the register command is of type
'set'.
So now to resume the situation:
1) register-use-preview == t
We have a register-preview buffer with navigation and selected register
highlighted, when the register is selected we have to press RET to do
the action on this register. Defaults are provided for registers of
type set. Registers are filtered depending on the command used.
2) register-use-preview == nil
We have a register-preview buffer with no navigation and no
highlighting. No confirmation with RET is requested, as soon as you
enter the register name, minibuffer is exited immediately. Filtering
and defaults for register of type set are still provided.
3) register-use-preview == 'never
Same behavior as 2) but there is no preview buffer at all, however if
user type C-h, a preview buffer is provided behaving like 1) i.e. RET is
requested when selecting a register.
4) For registers which are key sequence e.g. C-a
For entering manually in minibuffer such a register one have to hit C-d
C-a. This is now the only thing that differ from Emacs-29.1 behavior.
Let me know if I can push this patch.
I think all this is a real improvement for Emacs despite the several
complaints we had recently, however if all the arrangements I did are
not enough, we could add a new variable register-read-preview-function
and reinstall the old code exactly as it was (it's easy, let me know if
you want this I will provide the patch).
Thanks.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 09:32:02 GMT)
Full text and
rfc822 format available.
Message #219 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Eli,
here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
I made it the default for now.
The only customization one have to do to make a change is through
register-use-preview which have now three options:
- basic (default - same as Emacs-29- )
- nil (simple preview with filtering, default and no confirm)
- never (same as nil but without any preview)
- t (the improved version with navigation etc...)
I don't know if you want to restore previous behavior (emacs-29), if so this patch
will apply against the previous one.
(0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
Thanks.
--
Thierry
[0001-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 09:59:01 GMT)
Full text and
rfc822 format available.
Message #222 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Hello Eli,
>
> here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
> I made it the default for now.
> The only customization one have to do to make a change is through
> register-use-preview which have now three options:
>
> - basic (default - same as Emacs-29- )
> - nil (simple preview with filtering, default and no confirm)
> - never (same as nil but without any preview)
> - t (the improved version with navigation etc...)
>
> I don't know if you want to restore previous behavior (emacs-29), if so this patch
> will apply against the previous one.
> (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
>
> Thanks.
BTW I saw the thread on Reddit just now. I have no more account there
to reply, but may be it is better to no reply to those people.
Even if it's easier to say than doing it, you should ignore all this
(and the usual trolls on emacs-dev) and continue the good job as you
always did, change nothing.
Respectfully,
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 12:32:02 GMT)
Full text and
rfc822 format available.
Message #225 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
> Date: Mon, 11 Dec 2023 09:30:50 +0000
>
> here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
> I made it the default for now.
> The only customization one have to do to make a change is through
> register-use-preview which have now three options:
>
> - basic (default - same as Emacs-29- )
> - nil (simple preview with filtering, default and no confirm)
> - never (same as nil but without any preview)
> - t (the improved version with navigation etc...)
>
> I don't know if you want to restore previous behavior (emacs-29), if so this patch
> will apply against the previous one.
> (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
Thank you for all your efforts in this matter.
I asked on emacs-devel that people try this and your previous
proposal, and provide opinions. (Those CC'ed here are also invited,
of course.) Let's wait for their responses for a couple of days
before we make the decision how to proceed.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 13:12:01 GMT)
Full text and
rfc822 format available.
Message #228 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
>> Date: Mon, 11 Dec 2023 09:30:50 +0000
>>
>> here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
>> I made it the default for now.
>> The only customization one have to do to make a change is through
>> register-use-preview which have now three options:
>>
>> - basic (default - same as Emacs-29- )
>> - nil (simple preview with filtering, default and no confirm)
>> - never (same as nil but without any preview)
>> - t (the improved version with navigation etc...)
>>
>> I don't know if you want to restore previous behavior (emacs-29), if so this patch
>> will apply against the previous one.
>> (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
>
> Thank you for all your efforts in this matter.
>
> I asked on emacs-devel that people try this and your previous
> proposal, and provide opinions. (Those CC'ed here are also invited,
> of course.) Let's wait for their responses for a couple of days
> before we make the decision how to proceed.
Thanks, there is no hurry, let me know if something else is needed, I
will make the needed changes in NEWS and regs.texi once the changes will be
accepted.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 17:33:02 GMT)
Full text and
rfc822 format available.
Message #231 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
>>> From: Thierry Volpiatto <thievol <at> posteo.net>
>>> Cc: monnier <at> iro.umontreal.ca, michael_heerdegen <at> web.de,
>>> stefankangas <at> gmail.com, 66394 <at> debbugs.gnu.org
>>> Date: Mon, 11 Dec 2023 09:30:50 +0000
>>>
>>> here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
>>> I made it the default for now.
>>> The only customization one have to do to make a change is through
>>> register-use-preview which have now three options:
>>>
>>> - basic (default - same as Emacs-29- )
>>> - nil (simple preview with filtering, default and no confirm)
>>> - never (same as nil but without any preview)
>>> - t (the improved version with navigation etc...)
>>>
>>> I don't know if you want to restore previous behavior (emacs-29), if so this patch
>>> will apply against the previous one.
>>> (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
>>
>> Thank you for all your efforts in this matter.
>>
>> I asked on emacs-devel that people try this and your previous
>> proposal, and provide opinions. (Those CC'ed here are also invited,
>> of course.) Let's wait for their responses for a couple of days
>> before we make the decision how to proceed.
>
> Thanks, there is no hurry, let me know if something else is needed, I
> will make the needed changes in NEWS and regs.texi once the changes will be
> accepted.
I made a little error in one of the patch when merging from a working
file for emacs-29.1, attaching here the two patches again with error
corrected (read `register-use-preview` instead of
`register-preview-use-preview`), sorry for this.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 11 Dec 2023 23:37:02 GMT)
Full text and
rfc822 format available.
Message #234 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 11/12/2023 19:32, Thierry Volpiatto wrote:
> I made a little error in one of the patch when merging from a working
> file for emacs-29.1, attaching here the two patches again with error
> corrected (read `register-use-preview` instead of
> `register-preview-use-preview`), sorry for this.
JFYI, the second patch doesn't apply cleanly, which may make testing
them a bit more difficult.
It's because of commit 598ab9ca10d35d6 by Eli with documentation
updates. When reverted, the second patch applies without problem.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 05:48:02 GMT)
Full text and
rfc822 format available.
Message #237 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hi,
I could live with the patch proposed in
https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-12/msg00644.html
if the default value for
was 'never. The new behaviour would be opt-in and I might be tempted to
test it out
and eventually switch in the future.
Thx, /PA
--
Fragen sind nicht da, um beantwortet zu werden,
Fragen sind da um gestellt zu werden
Georg Kreisler
Headaches with a Juju log:
unit-basic-16: 09:17:36 WARNING juju.worker.uniter.operation we should run
a leader-deposed hook here, but we can't yet
[Message part 2 (text/html, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 06:07:02 GMT)
Full text and
rfc822 format available.
Message #240 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Confirmation, we already have bookmarks which are the "longer"
variants. Making registers confirm by default, makes registers on
par, as slow to use as bookmarks to use. If those who wish to use
registers with confirmation, why not use bookmarks?
What becomes the difference between registers and bookmarks with this
change (confirmation part) other than bookmarks being saved to a file?
Some people seemed to have lots and lots (dozens if not more) of
registers, what heppens if you restart Emacs? Do you save registers
between invocations of Emacs? In that case, it sounds like one should
be using bookmarks and not registers.
TLDR: Making the default to "confirm" (preview is not the main
problem) makes registers just like bookmarks.
here the last patch that provide Emacs-29 previous behavior as asked on emacs-dev,
I made it the default for now.
The only customization one have to do to make a change is through
register-use-preview which have now three options:
- basic (default - same as Emacs-29- )
- nil (simple preview with filtering, default and no confirm)
- never (same as nil but without any preview)
- t (the improved version with navigation etc...)
I think the variable is being overloaded. Shouldn't this just control
the usage of preview, not how interactive the behaviour of registers
are?
People seem to be confusing what it does already, since they think
that registers did not have preview before this change (and that being
the main feaature that they seem to be after -- not confirmation).
Which makes it hard to disucss this change, since it conflates
multiple topics into one thread.
"basic" seems to not be the same behaviour as in 29 (from the looks --
I cannot apply the patch), even Emacs 25 has register preview by
default, which is a useful feature, which seems to correspond to
'nil'.
The discussion of this change is strange and why people are upset, we
first change the behaviour silently and then try to justify the old
one. It should be the other way round.
Why is confirmation in registers so important to break old trusted
behaviour, when we already have a similar feature (bookmarks) that
those who wish to be "sure" can't use that instead?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 06:30:02 GMT)
Full text and
rfc822 format available.
Message #243 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 11/12/2023 19:32, Thierry Volpiatto wrote:
>> I made a little error in one of the patch when merging from a working
>> file for emacs-29.1, attaching here the two patches again with error
>> corrected (read `register-use-preview` instead of
>> `register-preview-use-preview`), sorry for this.
>
> JFYI, the second patch doesn't apply cleanly, which may make testing
> them a bit more difficult.
>
> It's because of commit 598ab9ca10d35d6 by Eli with documentation
> updates. When reverted, the second patch applies without problem.
Dmitry thanks, here the patches again after rebasing.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 09:32:02 GMT)
Full text and
rfc822 format available.
Message #246 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>
>> On 11/12/2023 19:32, Thierry Volpiatto wrote:
>>> I made a little error in one of the patch when merging from a working
>>> file for emacs-29.1, attaching here the two patches again with error
>>> corrected (read `register-use-preview` instead of
>>> `register-preview-use-preview`), sorry for this.
>>
>> JFYI, the second patch doesn't apply cleanly, which may make testing
>> them a bit more difficult.
>>
>> It's because of commit 598ab9ca10d35d6 by Eli with documentation
>> updates. When reverted, the second patch applies without problem.
>
> Dmitry thanks, here the patches again after rebasing.
I found another (merge) error in patch 0001 with defmethod names (read
register-command-info instead of register-preview-command-info), here
the patches corrected. Without this changes, and with
register-use-preview == nil, confirmation is needed for copy-to-register
and friends, now fixed.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 10:18:02 GMT)
Full text and
rfc822 format available.
Message #249 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Thierry Volpiatto <thievol <at> posteo.net> writes:
>
>> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>>
>>> On 11/12/2023 19:32, Thierry Volpiatto wrote:
>>>> I made a little error in one of the patch when merging from a working
>>>> file for emacs-29.1, attaching here the two patches again with error
>>>> corrected (read `register-use-preview` instead of
>>>> `register-preview-use-preview`), sorry for this.
>>>
>>> JFYI, the second patch doesn't apply cleanly, which may make testing
>>> them a bit more difficult.
>>>
>>> It's because of commit 598ab9ca10d35d6 by Eli with documentation
>>> updates. When reverted, the second patch applies without problem.
>>
>> Dmitry thanks, here the patches again after rebasing.
>
> I found another (merge) error in patch 0001 with defmethod names (read
> register-command-info instead of register-preview-command-info), here
> the patches corrected. Without this changes, and with
> register-use-preview == nil, confirmation is needed for copy-to-register
> and friends, now fixed.
Here a third patch that allows more precise configuration if needed.
For example if you have register-use-preview == nil, no confirmation
with RET is needed everywhere, even when overwriting a register. If you
want to have a confirmation when overwriting but no confirmation when
jumping/inserting you can use this in addition of register-use-preview
== nil:
(cl-defmethod register-command-info :after ((_command (eql copy-to-register)))
(make-register-preview-info
:types '(all)
:msg "Copy to register `%s'"
:act 'set))
You can modify other commands as well to your needs (point-to-register
etc...) with same method.
Sending here the serie of 3 patches to avoid errors when applying
patches.
I have also some minor errors (english in docstring, unnneded quotes
etc...) that I will fix later when polishing final patches.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[0003-Allow-users-overriding-register-use-preview-behavior.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 10:50:02 GMT)
Full text and
rfc822 format available.
Message #252 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> posteo.net> writes:
>
> ...
>
> I don't know if you want to restore previous behavior (emacs-29), if so this patch
> will apply against the previous one.
> (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
>
> Thanks.
Hi - I'd love to give this patch a try but I can't get it to apply
against master, either with or without the other patch:
Don't Panic! ~/src/emacs $ patch -p1 < 0001-Provide-emacs-29-behavior-for-register-preview.patch
patching file lisp/register.el
Hunk #1 FAILED at 100.
Hunk #2 succeeded at 138 (offset 1 line).
Hunk #3 succeeded at 302 (offset -41 lines).
Hunk #4 succeeded at 362 (offset -41 lines).
Hunk #5 succeeded at 433 (offset -42 lines).
1 out of 5 hunks FAILED -- saving rejects to file lisp/register.el.rej
I assume I'm doing something wrong somewhere but don't know what :-(
Many thanks!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 12:02:01 GMT)
Full text and
rfc822 format available.
Message #255 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Pedro Andres Aranda Gutierrez <paaguti <at> gmail.com>
> Date: Tue, 12 Dec 2023 06:46:51 +0100
>
> I could live with the patch proposed in
> https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-12/msg00644.html
> if the default value for
Thanks, but please don't cross-post. Post your opinions and
responses either toe emacs-devel or (preferred) 66394 <at> debbugs.gnu.org,
but not to both.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 12:17:02 GMT)
Full text and
rfc822 format available.
Message #258 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Steve Perry <stp <at> envs.net>
> Cc: eliz <at> gnu.org, michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org,
> monnier <at> iro.umontreal.ca,
> stefankangas <at> gmail.com
> Date: Tue, 12 Dec 2023 09:37:45 +0000
>
> Thierry Volpiatto <thievol <at> posteo.net> writes:
> >
> > ...
> >
> > I don't know if you want to restore previous behavior (emacs-29), if so this patch
> > will apply against the previous one.
> > (0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch).
> >
> > Thanks.
>
> Hi - I'd love to give this patch a try but I can't get it to apply
> against master, either with or without the other patch:
>
> Don't Panic! ~/src/emacs $ patch -p1 < 0001-Provide-emacs-29-behavior-for-register-preview.patch
> patching file lisp/register.el
> Hunk #1 FAILED at 100.
> Hunk #2 succeeded at 138 (offset 1 line).
> Hunk #3 succeeded at 302 (offset -41 lines).
> Hunk #4 succeeded at 362 (offset -41 lines).
> Hunk #5 succeeded at 433 (offset -42 lines).
> 1 out of 5 hunks FAILED -- saving rejects to file lisp/register.el.rej
>
> I assume I'm doing something wrong somewhere but don't know what :-(
I don't think it's your fault. Please try applying the patch named
0002-Provide-emacs-29-behavior-for-register-preview.patch from this
message:
https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-12/msg00704.html
Thanks.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 16:46:01 GMT)
Full text and
rfc822 format available.
Message #261 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Here a third patch that allows more precise configuration if needed.
> For example if you have register-use-preview == nil, no confirmation
> with RET is needed everywhere, even when overwriting a register. If you
> want to have a confirmation when overwriting but no confirmation when
> jumping/inserting you can use this in addition of register-use-preview
> == nil:
Finally patch 0003 is not needed, one can override default with:
(cl-defmethod register-command-info ((_command (eql copy-to-register)))
(make-register-preview-info
:types '(all)
:msg "Copy to register `%s'"
:act 'set
:noconfirm nil))
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 12 Dec 2023 20:14:03 GMT)
Full text and
rfc822 format available.
Message #264 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Eli Zaretskii <eliz <at> gnu.org> writes:
>
> I don't think it's your fault. Please try applying the patch named
> 0002-Provide-emacs-29-behavior-for-register-preview.patch from this
> message:
>
> https://lists.gnu.org/archive/html/bug-gnu-emacs/2023-12/msg00704.html
>
> Thanks.
Ah, perfect. Cheers!
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 01:48:02 GMT)
Full text and
rfc822 format available.
Message #267 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 12/12/2023 12:16, Thierry Volpiatto wrote:
> Here a third patch that allows more precise configuration if needed.
> For example if you have register-use-preview == nil, no confirmation
> with RET is needed everywhere, even when overwriting a register. If you
> want to have a confirmation when overwriting but no confirmation when
> jumping/inserting you can use this in addition of register-use-preview
I was actually thinking this might be a good default behavior: no
confirmation in all cases except when overwriting. It's a change from
the current one still, but a smaller one.
But never mind me, I have very little practice using registers.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 02:11:02 GMT)
Full text and
rfc822 format available.
Message #270 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 12/12/2023 11:31, Thierry Volpiatto wrote:
> Thierry Volpiatto<thievol <at> posteo.net> writes:
>
>> Dmitry Gutov<dmitry <at> gutov.dev> writes:
>>
>>> On 11/12/2023 19:32, Thierry Volpiatto wrote:
>>>> I made a little error in one of the patch when merging from a working
>>>> file for emacs-29.1, attaching here the two patches again with error
>>>> corrected (read `register-use-preview` instead of
>>>> `register-preview-use-preview`), sorry for this.
>>> JFYI, the second patch doesn't apply cleanly, which may make testing
>>> them a bit more difficult.
>>>
>>> It's because of commit 598ab9ca10d35d6 by Eli with documentation
>>> updates. When reverted, the second patch applies without problem.
>> Dmitry thanks, here the patches again after rebasing.
> I found another (merge) error in patch 0001 with defmethod names (read
> register-command-info instead of register-preview-command-info), here
> the patches corrected. Without this changes, and with
> register-use-preview == nil, confirmation is needed for copy-to-register
> and friends, now fixed.
I did some testing.
Something I'm seeing with my custom config, but not with 'emacs -Q': the
register preview is eating my windows. :-) No matter if I choose
something, or C-g out of it.
To reproduce:
(defun split-window-prefer-side-by-side (&optional window)
(let ((split-height-threshold (and (< (window-width window)
split-width-threshold)
split-height-threshold)))
(split-window-sensibly window)))
(setq split-window-preferred-function 'split-window-prefer-side-by-side)
Then split the frame horizontally or vertically, create a register and
jump to it.
The problem is that with this setup the preview is not creating a new
window but reuses an existing one. And then deletes it at the end.
Again, I'm not a heavy user of registers, so if this is easy enough to
fix, great. If not, never mind.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 05:32:01 GMT)
Full text and
rfc822 format available.
Message #273 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 12/12/2023 11:31, Thierry Volpiatto wrote:
>> Thierry Volpiatto<thievol <at> posteo.net> writes:
>>
>>> Dmitry Gutov<dmitry <at> gutov.dev> writes:
>>>
>>>> On 11/12/2023 19:32, Thierry Volpiatto wrote:
>>>>> I made a little error in one of the patch when merging from a working
>>>>> file for emacs-29.1, attaching here the two patches again with error
>>>>> corrected (read `register-use-preview` instead of
>>>>> `register-preview-use-preview`), sorry for this.
>>>> JFYI, the second patch doesn't apply cleanly, which may make testing
>>>> them a bit more difficult.
>>>>
>>>> It's because of commit 598ab9ca10d35d6 by Eli with documentation
>>>> updates. When reverted, the second patch applies without problem.
>>> Dmitry thanks, here the patches again after rebasing.
>> I found another (merge) error in patch 0001 with defmethod names (read
>> register-command-info instead of register-preview-command-info), here
>> the patches corrected. Without this changes, and with
>> register-use-preview == nil, confirmation is needed for copy-to-register
>> and friends, now fixed.
>
> I did some testing.
>
> Something I'm seeing with my custom config, but not with 'emacs -Q':
> the register preview is eating my windows. :-) No matter if I choose
> something, or C-g out of it.
>
> To reproduce:
>
> (defun split-window-prefer-side-by-side (&optional window)
> (let ((split-height-threshold (and (< (window-width window)
> split-width-threshold)
> split-height-threshold)))
> (split-window-sensibly window)))
>
> (setq split-window-preferred-function 'split-window-prefer-side-by-side)
>
> Then split the frame horizontally or vertically, create a register and
> jump to it.
>
> The problem is that with this setup the preview is not creating a new
> window but reuses an existing one. And then deletes it at the end.
I think I can fix this, will look at it as soon as possible.
Thanks.
> Again, I'm not a heavy user of registers, so if this is easy enough to
> fix, great. If not, never mind.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 05:36:02 GMT)
Full text and
rfc822 format available.
Message #276 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 12/12/2023 12:16, Thierry Volpiatto wrote:
>> Here a third patch that allows more precise configuration if needed.
>> For example if you have register-use-preview == nil, no confirmation
>> with RET is needed everywhere, even when overwriting a register. If you
>> want to have a confirmation when overwriting but no confirmation when
>> jumping/inserting you can use this in addition of register-use-preview
>
> I was actually thinking this might be a good default behavior: no
> confirmation in all cases except when overwriting. It's a change from
> the current one still, but a smaller one.
I think too it would be a good default.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 07:40:02 GMT)
Full text and
rfc822 format available.
Message #279 received at 66394 <at> debbugs.gnu.org (full text, mbox):
>> I was actually thinking this might be a good default behavior: no
>> confirmation in all cases except when overwriting. It's a change from
>> the current one still, but a smaller one.
Note that when used within a keyboard macro, it's annoying if it
sometimes requires confirmation and sometimes not.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 07:46:02 GMT)
Full text and
rfc822 format available.
Message #282 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: Eli Zaretskii <eliz <at> gnu.org>, michael_heerdegen <at> web.de,
> 66394 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com
> Date: Thu, 14 Dec 2023 05:34:36 +0000
>
> Dmitry Gutov <dmitry <at> gutov.dev> writes:
>
> > On 12/12/2023 12:16, Thierry Volpiatto wrote:
> >> Here a third patch that allows more precise configuration if needed.
> >> For example if you have register-use-preview == nil, no confirmation
> >> with RET is needed everywhere, even when overwriting a register. If you
> >> want to have a confirmation when overwriting but no confirmation when
> >> jumping/inserting you can use this in addition of register-use-preview
> >
> > I was actually thinking this might be a good default behavior: no
> > confirmation in all cases except when overwriting. It's a change from
> > the current one still, but a smaller one.
>
> I think too it would be a good default.
What do others think about making this the default?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 08:25:02 GMT)
Full text and
rfc822 format available.
Message #285 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Dmitry Gutov <dmitry <at> gutov.dev>, Eli Zaretskii <eliz <at> gnu.org>,
> michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org, stefankangas <at> gmail.com
> Date: Thu, 14 Dec 2023 02:38:56 -0500
>
> >> I was actually thinking this might be a good default behavior: no
> >> confirmation in all cases except when overwriting. It's a change from
> >> the current one still, but a smaller one.
>
> Note that when used within a keyboard macro, it's annoying if it
> sometimes requires confirmation and sometimes not.
Perhaps we could avoid the confirmation request when the command is
run from a macro?
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 15:51:02 GMT)
Full text and
rfc822 format available.
Message #288 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> What do others think about making this the default?
My understanding is that usage style of registers varies a lot, where
some people use them extensively, so any change that makes them
heavier/slower (such a requiring a confirmation) will alienate them.
For that reason I'd opt to keep the defaults as close to the previous
behavior, focusing on making the new features discoverable rather than
enabled by default.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 18:00:03 GMT)
Full text and
rfc822 format available.
Message #291 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> What do others think about making this the default?
>
> My understanding is that usage style of registers varies a lot, where
> some people use them extensively, so any change that makes them
> heavier/slower (such a requiring a confirmation) will alienate them.
My impression is that these people use a workflow based on few register
(I guess maximum 5) because it is hard to read the register preview as
it was before, so they work with for example only "a", "b" and "c", they
always overwrite them and as there is only few registers they can
always remember what they do.
But when you have many registers, the time you spend scrutinizing the
preview buffer is worst than the time you spend hitting RET or even
navigating the preview buffer with C-n/p.
>
> For that reason I'd opt to keep the defaults as close to the previous
> behavior, focusing on making the new features discoverable rather than
> enabled by default.
So we could have the default exactly as before (register-use-preview ==
basic) and disable the behavior of register-preview == nil, keeping only
t and 'never.
t would provide the fully featured preview buffer (with RET everywhere to confirm).
And 'never would be the same but with no preview buffer (but still available
with C-h).
As I have actually a problem unsolved with register-use-preview == nil
with kmacros, this would save me some more work.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 19:20:01 GMT)
Full text and
rfc822 format available.
Message #294 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On Dez 14 2023, Thierry Volpiatto wrote:
> My impression is that these people use a workflow based on few register
> (I guess maximum 5) because it is hard to read the register preview as
> it was before, so they work with for example only "a", "b" and "c", they
> always overwrite them and as there is only few registers they can
> always remember what they do.
The right way to handle that is to use mnemonic letters and punctuation
as registers.
It is even impossible now to use both lower and upper case letters.
That is a serious bug.
Registers need to be fast and accessible. The current interface is slow
and cumbersome.
--
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 19:40:02 GMT)
Full text and
rfc822 format available.
Message #297 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Thierry Volpiatto <thievol <at> posteo.net> writes:
> From 386457b493e3d2ebdb605dd8a8ce3c323847f537 Mon Sep 17 00:00:00 2001
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Mon, 11 Dec 2023 07:02:40 +0100
> Subject: [PATCH 1/2] Don't confirm with RET even when overwriting in register
> commands
The preview here when copying to and inserting from registers is really
nice.
However, I think we should do what Stefan Monnier suggested: keep the
old default (with regards to which keys to press), and make it
discoverable for those that want it.
That said, I think the new feature, if made optional, will clearly be
popular among at least a section of users. So it's a nice addition and
extension of Emacs' capabilities.
> From 7b634a3359565dea755392324055443eb02aa070 Mon Sep 17 00:00:00 2001
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Tue, 12 Dec 2023 07:24:32 +0100
> Subject: [PATCH 2/2] Provide emacs-29 behavior for register-preview
This one is okay in terms of the keys I need to press (i.e. there is no
change from emacs-29, by default), but why not keep the preview from the
first one? That part I think was a really good addition, and also
non-intrusive. It makes me feel more in control, as I'm not inserting
or overwriting stuff blindly. We might want an option to turn it off,
though.
I hope that helps.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 14 Dec 2023 20:30:02 GMT)
Full text and
rfc822 format available.
Message #300 received at 66394 <at> debbugs.gnu.org (full text, mbox):
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> What do others think about making this the default?
>
> My understanding is that usage style of registers varies a lot, where
> some people use them extensively, so any change that makes them
> heavier/slower (such a requiring a confirmation) will alienate them.
> For that reason I'd opt to keep the defaults as close to the previous
> behavior, focusing on making the new features discoverable rather than
> enabled by default.
+1
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 15 Dec 2023 14:47:02 GMT)
Full text and
rfc822 format available.
Message #303 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Kangas <stefankangas <at> gmail.com> writes:
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>> What do others think about making this the default?
>>
>> My understanding is that usage style of registers varies a lot, where
>> some people use them extensively, so any change that makes them
>> heavier/slower (such a requiring a confirmation) will alienate them.
>> For that reason I'd opt to keep the defaults as close to the previous
>> behavior, focusing on making the new features discoverable rather than
>> enabled by default.
>
> +1
I just fixed a bug with register-use-preview = nil about its usage
with kmacros.
So I guess it is now possible to make it the default.
In these patches (attached) I also fixed the problem of Dmitri with windows
configuration (let me know if it works), BTW I guess this bug was
already here before these changes.
Thanks all for your feedback.
PS: I didn't have the time to look in delayed preview request, will do
as soon as possible.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[0003-Save-and-restore-win-confs-after-register-commands.patch (text/x-diff, attachment)]
[0004-Fix-register-commands-in-kmacros.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 15 Dec 2023 15:19:02 GMT)
Full text and
rfc822 format available.
Message #306 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> @@ -429,7 +429,12 @@ Format of each entry is controlled by the variable `register-preview-function'."
> Prompt with the string PROMPT.
> If `help-char' (or a member of `help-event-list') is pressed,
> display such a window regardless."
> - (funcall register--read-with-preview-function prompt))
> + (let ((register--read-with-preview-function
> + (if (and executing-kbd-macro
> + (memq register-use-preview '(nil never)))
> + #'register-read-with-preview-basic
> + (default-value 'register--read-with-preview-function))))
> + (funcall register--read-with-preview-function prompt)))
Questions/comments:
- Why did you change from using
`register--read-with-preview-function` to using
(default-value 'register--read-with-preview-function) ?
[ The answer should presumably be in the commit message but
I couldn't find it there. ]
- Why let-bind `register--read-with-preview-function`
rather than using a local lexical var?
[ The answer should probably be in a comment in the code. ]
- Making the behavior dependent on `executing-kbd-macro` is generally
undesirable, so it should be accompanied with a comment in the code
explaining why we need it (with enough detail that someone
sufficiently motivated could potentially "fix" the code to remove this
dependency, or alternatively to convince that someone else that this
dependency is actually desirable here).
-- Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 15 Dec 2023 18:37:02 GMT)
Full text and
rfc822 format available.
Message #309 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> @@ -429,7 +429,12 @@ Format of each entry is controlled by the variable `register-preview-function'."
>> Prompt with the string PROMPT.
>> If `help-char' (or a member of `help-event-list') is pressed,
>> display such a window regardless."
>> - (funcall register--read-with-preview-function prompt))
>> + (let ((register--read-with-preview-function
>> + (if (and executing-kbd-macro
>> + (memq register-use-preview '(nil never)))
>> + #'register-read-with-preview-basic
>> + (default-value 'register--read-with-preview-function))))
>> + (funcall register--read-with-preview-function prompt)))
>
> Questions/comments:
>
> - Why did you change from using
> `register--read-with-preview-function` to using
> (default-value 'register--read-with-preview-function) ?
> [ The answer should presumably be in the commit message but
> I couldn't find it there. ]
>
> - Why let-bind `register--read-with-preview-function`
> rather than using a local lexical var?
> [ The answer should probably be in a comment in the code. ]
To answer to your 1) and 2) questions, I guess what you
suggest is something like this (indeed better):
diff --git a/lisp/register.el b/lisp/register.el
index 15ed5c0a53b..2444f88794e 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -429,12 +429,11 @@ Format of each entry is controlled by the variable `register-preview-function'."
Prompt with the string PROMPT.
If `help-char' (or a member of `help-event-list') is pressed,
display such a window regardless."
- (let ((register--read-with-preview-function
- (if (and executing-kbd-macro
- (memq register-use-preview '(nil never)))
- #'register-read-with-preview-basic
- (default-value 'register--read-with-preview-function))))
- (funcall register--read-with-preview-function prompt)))
+ (let ((fn (if (and executing-kbd-macro
+ (memq register-use-preview '(nil never)))
+ #'register-read-with-preview-basic
+ register--read-with-preview-function)))
+ (funcall fn prompt)))
(defun register-read-with-preview-basic (prompt)
"Read and return a register name, possibly showing existing registers.
> - Making the behavior dependent on `executing-kbd-macro` is generally
> undesirable, so it should be accompanied with a comment in the code
> explaining why we need it (with enough detail that someone
> sufficiently motivated could potentially "fix" the code to remove this
> dependency, or alternatively to convince that someone else that this
> dependency is actually desirable here).
The explanation is in the commit message. To resume, when we are not
using RET to exit minibuffer, we use `exit-minibuffer` from the timer
function in minibuffer-setup-hook, BTW when you have a macro using
e.g. "C-x r i, C-n, C-a, C-x r +", "C-n and C-a" are running
immediately BEFORE the minibuffer is exited so they run in minibuffer
and have no effect in your macro that run in current-buffer.
Is such a comment sufficiently explicit? (will add in next patch if so).
If you have a better fix for this I take ;-).
The problem with such a fix (as I did) is that we can't have an hybrid
version of preview i.e. one that use RET to confirm overwrite and no RET
for other things.
For example if one add a configuration like below to modify behavior
with *-use-preview == nil the macro will fail to execute properly.
(cl-defmethod register-command-info ((_command (eql increment-register)))
(make-register-preview-info
:types '(all)
:msg "Increment to register `%s'"
:act 'set
:noconfirm nil))
Thanks Stefan for reviewing this.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Fri, 15 Dec 2023 23:32:01 GMT)
Full text and
rfc822 format available.
Message #312 received at 66394 <at> debbugs.gnu.org (full text, mbox):
>> - Why did you change from using
>> `register--read-with-preview-function` to using
>> (default-value 'register--read-with-preview-function) ?
>> [ The answer should presumably be in the commit message but
>> I couldn't find it there. ]
>>
>> - Why let-bind `register--read-with-preview-function`
>> rather than using a local lexical var?
>> [ The answer should probably be in a comment in the code. ]
>
> To answer to your 1) and 2) questions, I guess what you
> suggest is something like this (indeed better):
>
> diff --git a/lisp/register.el b/lisp/register.el
> index 15ed5c0a53b..2444f88794e 100644
> --- a/lisp/register.el
> +++ b/lisp/register.el
> @@ -429,12 +429,11 @@ Format of each entry is controlled by the variable `register-preview-function'."
> Prompt with the string PROMPT.
> If `help-char' (or a member of `help-event-list') is pressed,
> display such a window regardless."
> - (let ((register--read-with-preview-function
> - (if (and executing-kbd-macro
> - (memq register-use-preview '(nil never)))
> - #'register-read-with-preview-basic
> - (default-value 'register--read-with-preview-function))))
> - (funcall register--read-with-preview-function prompt)))
> + (let ((fn (if (and executing-kbd-macro
> + (memq register-use-preview '(nil never)))
> + #'register-read-with-preview-basic
> + register--read-with-preview-function)))
> + (funcall fn prompt)))
>
> (defun register-read-with-preview-basic (prompt)
> "Read and return a register name, possibly showing existing registers.
LGTM, thanks.
>> - Making the behavior dependent on `executing-kbd-macro` is generally
>> undesirable, so it should be accompanied with a comment in the code
>> explaining why we need it (with enough detail that someone
>> sufficiently motivated could potentially "fix" the code to remove this
>> dependency, or alternatively to convince that someone else that this
>> dependency is actually desirable here).
>
> The explanation is in the commit message.
Then please move it into a comment in the code.
> To resume, when we are not
> using RET to exit minibuffer, we use `exit-minibuffer` from the timer
> function in minibuffer-setup-hook, BTW when you have a macro using
> e.g. "C-x r i, C-n, C-a, C-x r +", "C-n and C-a" are running
> immediately BEFORE the minibuffer is exited so they run in minibuffer
> and have no effect in your macro that run in current-buffer.
> Is such a comment sufficiently explicit? (will add in next patch if so).
Sounds good, yes.
> If you have a better fix for this I take ;-).
I haven't looked enough at the code to have a better suggestion.
From what I see the only way to have a "better fix" would be to forego
the use of asynchronous code (i.e. of a timer). I don't know why you're
using a timer, but usually people don't use timer just because they're
pretty, so I assume you've considered other options already (such as
using an `after-change-function` or a `post-command-hook` instead of
a timer).
[ FWIW, I suspect we have a similar problem in `read-key`.
Maybe that's the reason why people refrain from using `read-key`?
I can't see any comment in `read-key` mentioning that it doesn't work
in kmacros, but indeed it uses a timer just like you do, so it
probably fails in the exact same way. ]
> The problem with such a fix (as I did) is that we can't have an hybrid
> version of preview i.e. one that use RET to confirm overwrite and no RET
> for other things.
> For example if one add a configuration like below to modify behavior
> with *-use-preview == nil the macro will fail to execute properly.
You might want to add a short hint about that problem in the comment.
> Thanks Stefan for reviewing this.
I looked only at the kmacro part (because I thought maybe it had to do
with kmacro's use of OClosures), sorry.
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 13:19:01 GMT)
Full text and
rfc822 format available.
Message #315 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> - Why did you change from using
>>> `register--read-with-preview-function` to using
>>> (default-value 'register--read-with-preview-function) ?
>>> [ The answer should presumably be in the commit message but
>>> I couldn't find it there. ]
>>>
>>> - Why let-bind `register--read-with-preview-function`
>>> rather than using a local lexical var?
>>> [ The answer should probably be in a comment in the code. ]
>>
>> To answer to your 1) and 2) questions, I guess what you
>> suggest is something like this (indeed better):
>>
>> diff --git a/lisp/register.el b/lisp/register.el
>> index 15ed5c0a53b..2444f88794e 100644
>> --- a/lisp/register.el
>> +++ b/lisp/register.el
>> @@ -429,12 +429,11 @@ Format of each entry is controlled by the variable `register-preview-function'."
>> Prompt with the string PROMPT.
>> If `help-char' (or a member of `help-event-list') is pressed,
>> display such a window regardless."
>> - (let ((register--read-with-preview-function
>> - (if (and executing-kbd-macro
>> - (memq register-use-preview '(nil never)))
>> - #'register-read-with-preview-basic
>> - (default-value 'register--read-with-preview-function))))
>> - (funcall register--read-with-preview-function prompt)))
>> + (let ((fn (if (and executing-kbd-macro
>> + (memq register-use-preview '(nil never)))
>> + #'register-read-with-preview-basic
>> + register--read-with-preview-function)))
>> + (funcall fn prompt)))
>>
>> (defun register-read-with-preview-basic (prompt)
>> "Read and return a register name, possibly showing existing registers.
>
> LGTM, thanks.
>
>>> - Making the behavior dependent on `executing-kbd-macro` is generally
>>> undesirable, so it should be accompanied with a comment in the code
>>> explaining why we need it (with enough detail that someone
>>> sufficiently motivated could potentially "fix" the code to remove this
>>> dependency, or alternatively to convince that someone else that this
>>> dependency is actually desirable here).
>>
>> The explanation is in the commit message.
>
> Then please move it into a comment in the code.
Done.
>> To resume, when we are not
>> using RET to exit minibuffer, we use `exit-minibuffer` from the timer
>> function in minibuffer-setup-hook, BTW when you have a macro using
>> e.g. "C-x r i, C-n, C-a, C-x r +", "C-n and C-a" are running
>> immediately BEFORE the minibuffer is exited so they run in minibuffer
>> and have no effect in your macro that run in current-buffer.
>> Is such a comment sufficiently explicit? (will add in next patch if so).
>
> Sounds good, yes.
>
>> If you have a better fix for this I take ;-).
>
> I haven't looked enough at the code to have a better suggestion.
> From what I see the only way to have a "better fix" would be to forego
> the use of asynchronous code (i.e. of a timer). I don't know why you're
> using a timer, but usually people don't use timer just because they're
> pretty, so I assume you've considered other options already (such as
> using an `after-change-function` or a `post-command-hook` instead of
> a timer).
It should be possible to use post-command-hook, I didn't use it because
it makes harder the communication between the minibuffer and the preview
buffer.
> [ FWIW, I suspect we have a similar problem in `read-key`.
> Maybe that's the reason why people refrain from using `read-key`?
> I can't see any comment in `read-key` mentioning that it doesn't work
> in kmacros, but indeed it uses a timer just like you do, so it
> probably fails in the exact same way. ]
>
>> The problem with such a fix (as I did) is that we can't have an hybrid
>> version of preview i.e. one that use RET to confirm overwrite and no RET
>> for other things.
>> For example if one add a configuration like below to modify behavior
>> with *-use-preview == nil the macro will fail to execute properly.
>
> You might want to add a short hint about that problem in the comment.
Ok, will add this in next patch.
>> Thanks Stefan for reviewing this.
>
> I looked only at the kmacro part (because I thought maybe it had to do
> with kmacro's use of OClosures), sorry.
Anyway your comments were useful, as always, thanks.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 15:08:02 GMT)
Full text and
rfc822 format available.
Message #318 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 15/12/2023 16:45, Thierry Volpiatto wrote:
> In these patches (attached) I also fixed the problem of Dmitri with windows
> configuration (let me know if it works), BTW I guess this bug was
> already here before these changes.
My bad: it is indeed present in Emacs 29 as well.
It's still there in the new patches (just tested a couple of times), but
since it's not a regression, it might not be your problem to handle.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 15:33:02 GMT)
Full text and
rfc822 format available.
Message #321 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> It should be possible to use post-command-hook, I didn't use it because
> it makes harder the communication between the minibuffer and the preview
> buffer.
The patch below seems to work for my extremely limited testing.
Stefan
diff --git a/lisp/register.el b/lisp/register.el
index fa4bbcf483f..31466a42b0a 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -328,7 +328,7 @@ register-read-with-preview
m))
(data (register-command-info this-command))
(enable-recursive-minibuffers t)
- types msg result timer act win strs smatch)
+ types msg result act win strs smatch)
(if data
(setq types (register-preview-info-types data)
msg (register-preview-info-msg data)
@@ -360,9 +360,7 @@ register-read-with-preview
(progn
(minibuffer-with-setup-hook
(lambda ()
- (setq timer
- (run-with-idle-timer
- 0.01 'repeat
+ (add-hook 'post-command-hook
(lambda ()
(with-selected-window (minibuffer-window)
(let ((input (minibuffer-contents)))
@@ -408,13 +406,13 @@ register-read-with-preview
;; jump, don't ask for confirmation
;; and exit immediately (bug#66394).
(setq result pat)
- (exit-minibuffer)))))))))
+ (exit-minibuffer))))))
+ nil 'local))
(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 buf)))
(and (window-live-p w) (delete-window w)))
(and (get-buffer buf) (kill-buffer buf)))))
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 20:21:02 GMT)
Full text and
rfc822 format available.
Message #324 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Dmitry Gutov <dmitry <at> gutov.dev> writes:
> On 15/12/2023 16:45, Thierry Volpiatto wrote:
>> In these patches (attached) I also fixed the problem of Dmitri with windows
>> configuration (let me know if it works), BTW I guess this bug was
>> already here before these changes.
>
> My bad: it is indeed present in Emacs 29 as well.
>
> It's still there in the new patches (just tested a couple of times),
Really? I tested it from emacs -Q with your recipe and it was working.
Hmm, I will try again as soon as possible.
> but since it's not a regression, it might not be your problem to
> handle.
Yes indeed, but while we are at it, if we can fix it it is better.
Thanks.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 20:41:02 GMT)
Full text and
rfc822 format available.
Message #327 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> It should be possible to use post-command-hook, I didn't use it because
>> it makes harder the communication between the minibuffer and the preview
>> buffer.
>
> The patch below seems to work for my extremely limited testing.
Here the serie of patches with your patch included.
Let me know if the commit is incorrect (patch 0004).
I will test this extensively next week, too tired now to do something
useful.
Thanks.
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[0003-Save-and-restore-win-confs-after-register-commands.patch (text/x-diff, attachment)]
[0004-Fix-issue-with-register-commands-in-kmacro.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sat, 16 Dec 2023 23:29:02 GMT)
Full text and
rfc822 format available.
Message #330 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 16/12/2023 22:20, Thierry Volpiatto wrote:
> Dmitry Gutov<dmitry <at> gutov.dev> writes:
>
>> On 15/12/2023 16:45, Thierry Volpiatto wrote:
>>> In these patches (attached) I also fixed the problem of Dmitri with windows
>>> configuration (let me know if it works), BTW I guess this bug was
>>> already here before these changes.
>> My bad: it is indeed present in Emacs 29 as well.
>>
>> It's still there in the new patches (just tested a couple of times),
> Really? I tested it from emacs -Q with your recipe and it was working.
> Hmm, I will try again as soon as possible.
Thanks.
The problem occurs whenever an existing window is reused to show the
registers' preview, rather than split.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Sun, 17 Dec 2023 23:22:01 GMT)
Full text and
rfc822 format available.
Message #333 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From 6a15a8b7b795b8377ef3537dc7bbd5ba26c0e159 Mon Sep 17 00:00:00 2001
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Date: Thu, 14 Dec 2023 16:23:26 +0100
> Subject: [PATCH 3/4] Save and restore win confs after register commands
>
> When several windows are open, the register preview may eat other
> windows and break the current window configuration.
>
> * lisp/register.el (register-read-with-preview-fancy): Do it.
> ---
> lisp/register.el | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/lisp/register.el b/lisp/register.el
> index 8f0c6a7105d..1fadbe81056 100644
> --- a/lisp/register.el
> +++ b/lisp/register.el
> @@ -473,6 +473,7 @@ display such a window regardless."
> (buffer1 "*Register quick preview*")
> (buf (if register-use-preview buffer buffer1))
> (pat "")
> + (winconf (current-window-configuration))
> (map (let ((m (make-sparse-keymap)))
> (set-keymap-parent m minibuffer-local-map)
> m))
> @@ -573,6 +574,7 @@ display such a window regardless."
> nil "No register specified")
> (string-to-char result))
> (when timer (cancel-timer timer))
> + (set-window-configuration winconf)
> (let ((w (get-buffer-window buf)))
> (and (window-live-p w) (delete-window w)))
> (and (get-buffer buf) (kill-buffer buf)))))
While save&restore of window configuration often works well, it
misbehaves in various cases (such as when the buffer is displayed in
another frame, which case case `set-window-configuration` will do you
no good).
The official way to undo a `display-buffer` is with
`quit-(restore-)window`. See (info "(elisp)Quitting Windows").
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 18 Dec 2023 05:16:02 GMT)
Full text and
rfc822 format available.
Message #336 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> From 6a15a8b7b795b8377ef3537dc7bbd5ba26c0e159 Mon Sep 17 00:00:00 2001
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Date: Thu, 14 Dec 2023 16:23:26 +0100
>> Subject: [PATCH 3/4] Save and restore win confs after register commands
>>
>> When several windows are open, the register preview may eat other
>> windows and break the current window configuration.
>>
>> * lisp/register.el (register-read-with-preview-fancy): Do it.
>> ---
>> lisp/register.el | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/lisp/register.el b/lisp/register.el
>> index 8f0c6a7105d..1fadbe81056 100644
>> --- a/lisp/register.el
>> +++ b/lisp/register.el
>> @@ -473,6 +473,7 @@ display such a window regardless."
>> (buffer1 "*Register quick preview*")
>> (buf (if register-use-preview buffer buffer1))
>> (pat "")
>> + (winconf (current-window-configuration))
>> (map (let ((m (make-sparse-keymap)))
>> (set-keymap-parent m minibuffer-local-map)
>> m))
>> @@ -573,6 +574,7 @@ display such a window regardless."
>> nil "No register specified")
>> (string-to-char result))
>> (when timer (cancel-timer timer))
>> + (set-window-configuration winconf)
>> (let ((w (get-buffer-window buf)))
>> (and (window-live-p w) (delete-window w)))
>> (and (get-buffer buf) (kill-buffer buf)))))
>
> While save&restore of window configuration often works well, it
> misbehaves in various cases (such as when the buffer is displayed in
> another frame, which case case `set-window-configuration` will do you
> no good).
Yes, I thought this would be enough, but no, there is as well another
problem in register-preview and register-preview-1 with this code which
is as well not working well if there is multiple windows.
(with-current-buffer-window
buffer
(cons 'display-buffer-below-selected
'((window-height . fit-window-to-buffer)
(preserve-size . (nil . t))))
[...]
At least it should use display-buffer-at-bottom, but I guess the best
would be to make this configurable in one way or the other.
>
> The official way to undo a `display-buffer` is with
> `quit-(restore-)window`. See (info "(elisp)Quitting Windows").
Yes, this is not what the current code is using, see the last unwind
form of register-read-with-preview-basic/fancy.
But as Dmitri said, it is not bugs introduced by these patches, I
thought it would be simple to fix while we were at it but no. I guess I
will follow Dmitri advice and not fix this in this serie of patches.
Perhaps we could make another bug report once these patches are applied
(I will remove the patch doing save/restore win conf) ?
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 18 Dec 2023 06:19:01 GMT)
Full text and
rfc822 format available.
Message #339 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
> While save&restore of window configuration often works well, it
> misbehaves in various cases (such as when the buffer is displayed in
> another frame, which case case `set-window-configuration` will do you
> no good).
>
> The official way to undo a `display-buffer` is with
> `quit-(restore-)window`. See (info "(elisp)Quitting Windows").
So here the serie of patches updated (save/restore winconf patch
dropped).
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[0003-Fix-issue-with-register-commands-in-kmacro.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 18 Dec 2023 13:22:02 GMT)
Full text and
rfc822 format available.
Message #342 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> But as Dmitri said, it is not bugs introduced by these patches, I
> thought it would be simple to fix while we were at it but no. I guess I
> will follow Dmitri advice and not fix this in this serie of patches.
Wise choice :-)
> Perhaps we could make another bug report once these patches are applied
> (I will remove the patch doing save/restore win conf) ?
Dmitry, can you do the honors (since you mentioned the bug first)?
Stefan
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 18 Dec 2023 18:12:02 GMT)
Full text and
rfc822 format available.
Message #345 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> But as Dmitri said, it is not bugs introduced by these patches, I
>> thought it would be simple to fix while we were at it but no. I guess I
>> will follow Dmitri advice and not fix this in this serie of patches.
>
> Wise choice :-)
When dropping this change while rebasing I didn't realize the author
name of the last patch has changed (from Stefan to Thierry), it is fixed
(git amend) here.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Mon, 18 Dec 2023 18:23:02 GMT)
Full text and
rfc822 format available.
Message #348 received at 66394 <at> debbugs.gnu.org (full text, mbox):
On 18/12/2023 15:20, Stefan Monnier wrote:
>> But as Dmitri said, it is not bugs introduced by these patches, I
>> thought it would be simple to fix while we were at it but no. I guess I
>> will follow Dmitri advice and not fix this in this serie of patches.
> Wise choice 😄
>
>> Perhaps we could make another bug report once these patches are applied
>> (I will remove the patch doing save/restore win conf) ?
> Dmitry, can you do the honors (since you mentioned the bug first)?
No problem: bug#67882.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 19 Dec 2023 17:42:02 GMT)
Full text and
rfc822 format available.
Message #351 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Hello Stefan,
Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>> It should be possible to use post-command-hook, I didn't use it because
>> it makes harder the communication between the minibuffer and the preview
>> buffer.
>
> The patch below seems to work for my extremely limited testing.
Could have some time to test register with your post-command-hook patch.
I had to make on more little change to make it working in all
conditions, here what I tried so far:
1) (customize-set-variable 'register-use-preview t)
We have here confirmation with RET everywhere, works fine in all
commands and test macro.
2) (customize-set-variable 'register-use-preview nil)
We have here no confirmation (RET) at all, works fine in all commands
and test macro.
3) (customize-set-variable 'register-use-preview nil) and modification
with a defmethod so that we have no confirmation in insert/jump and
confirmation with increment-register:
(cl-defmethod register-command-info ((_command (eql increment-register)))
(make-register-preview-info
:types '(all)
:msg "Increment to register `%s'"
:act 'set
:smatch t
:noconfirm t))
Works fine everywhere and in test macro as well.
4) (customize-set-variable 'register-use-preview 'never)
Same behavior as in 2) and 3) in same conditions.
[The test macro was adding a number at beginning and end of each lines in
a text and increment this number at every turn, this involve
insert-register (twice) and increment-register (once).]
So it seems we have now something working fine in all conditions :-)
Here the serie of patches (the first 3 are unchanged).
--
Thierry
[0001-Don-t-confirm-with-RET-even-when-overwriting-in-regi.patch (text/x-diff, attachment)]
[0002-Provide-emacs-29-behavior-for-register-preview.patch (text/x-diff, attachment)]
[0003-Fix-issue-with-register-commands-in-kmacro.patch (text/x-diff, attachment)]
[0004-Fix-condition-in-register-read-with-preview-fancy.patch (text/x-diff, attachment)]
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Tue, 19 Dec 2023 17:48:02 GMT)
Full text and
rfc822 format available.
Message #354 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Thierry Volpiatto <thievol <at> posteo.net> writes:
> Hello Stefan,
>
> Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>
>>> It should be possible to use post-command-hook, I didn't use it because
>>> it makes harder the communication between the minibuffer and the preview
>>> buffer.
>>
>> The patch below seems to work for my extremely limited testing.
> (cl-defmethod register-command-info ((_command (eql increment-register)))
> (make-register-preview-info
> :types '(all)
> :msg "Increment to register `%s'"
> :act 'set
> :smatch t
> :noconfirm t))
Copied the wrong one, the test was with :noconfirm == nil:
(cl-defmethod register-command-info ((_command (eql increment-register)))
(make-register-preview-info
:types '(all)
:msg "Increment to register `%s'"
:act 'set
:smatch t
:noconfirm nil))
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 20 Dec 2023 12:07:02 GMT)
Full text and
rfc822 format available.
Message #357 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
> dmitry <at> gutov.dev, michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org
> Date: Tue, 19 Dec 2023 17:40:59 +0000
>
> So it seems we have now something working fine in all conditions :-)
>
> Here the serie of patches (the first 3 are unchanged).
Thank you for all your efforts in this matter.
I have only 2 minor comments:
. I suggest to rename all the "*basic" symbols to "*traditional"
instead (including such words in some doc strings)
. I think we need to update the NEWS entry, and in particular
describe there the two different "styles" of entering the register
names and the preview modes
(I'm okay with doing these minor changes myself, once you install the
changes.)
Thanks again.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Wed, 20 Dec 2023 17:24:02 GMT)
Full text and
rfc822 format available.
Message #360 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: Stefan Kangas <stefankangas <at> gmail.com>, Eli Zaretskii <eliz <at> gnu.org>,
>> dmitry <at> gutov.dev, michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org
>> Date: Tue, 19 Dec 2023 17:40:59 +0000
>>
>> So it seems we have now something working fine in all conditions :-)
>>
>> Here the serie of patches (the first 3 are unchanged).
>
> Thank you for all your efforts in this matter.
You are welcome.
> I have only 2 minor comments:
>
> . I suggest to rename all the "*basic" symbols to "*traditional"
> instead (including such words in some doc strings)
Ok, done. Have a look though to verify if docstring is correct.
> . I think we need to update the NEWS entry, and in particular
> describe there the two different "styles" of entering the register
> names and the preview modes
I would prefer you do this, it will be better written.
> (I'm okay with doing these minor changes myself, once you install the
> changes.)
Ok, now merged, Thanks.
> Thanks again.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 21 Dec 2023 11:49:02 GMT)
Full text and
rfc822 format available.
Message #363 received at 66394 <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com, dmitry <at> gutov.dev,
> michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org
> Date: Wed, 20 Dec 2023 17:23:14 +0000
>
> > . I suggest to rename all the "*basic" symbols to "*traditional"
> > instead (including such words in some doc strings)
>
> Ok, done. Have a look though to verify if docstring is correct.
>
> > . I think we need to update the NEWS entry, and in particular
> > describe there the two different "styles" of entering the register
> > names and the preview modes
>
> I would prefer you do this, it will be better written.
>
> > (I'm okay with doing these minor changes myself, once you install the
> > changes.)
>
> Ok, now merged, Thanks.
Thanks, I have a question:
The doc string of register-use-preview says:
This has no effect when the value of `register--read-with-preview-function'
is `register-read-with-preview-traditional'.
However, customizing register-use-preview to the value 'traditional
has the exact effect of setting register--read-with-preview-function
to register-read-with-preview-traditional. OTOH, when
register-use-preview is set to 'traditional, one can set it to a
different value, and then the behavior will change. So I wonder what
that sentence is about and what I missed? Is that perhaps a leftover
from previous versions, and should now be deleted?
I updated the documentation and NEWS. I'd appreciate if you could
spare a few minutes to review the changes and point out any
inaccuracies or mistakes.
Information forwarded
to
bug-gnu-emacs <at> gnu.org:
bug#66394; Package
emacs.
(Thu, 21 Dec 2023 18:05:01 GMT)
Full text and
rfc822 format available.
Message #366 received at 66394 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
Eli Zaretskii <eliz <at> gnu.org> writes:
>> From: Thierry Volpiatto <thievol <at> posteo.net>
>> Cc: monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com, dmitry <at> gutov.dev,
>> michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org
>> Date: Wed, 20 Dec 2023 17:23:14 +0000
>>
>> > . I suggest to rename all the "*basic" symbols to "*traditional"
>> > instead (including such words in some doc strings)
>>
>> Ok, done. Have a look though to verify if docstring is correct.
>>
>> > . I think we need to update the NEWS entry, and in particular
>> > describe there the two different "styles" of entering the register
>> > names and the preview modes
>>
>> I would prefer you do this, it will be better written.
>>
>> > (I'm okay with doing these minor changes myself, once you install the
>> > changes.)
>>
>> Ok, now merged, Thanks.
>
> Thanks, I have a question:
>
> The doc string of register-use-preview says:
>
> This has no effect when the value of `register--read-with-preview-function'
> is `register-read-with-preview-traditional'.
>
> However, customizing register-use-preview to the value 'traditional
> has the exact effect of setting register--read-with-preview-function
> to register-read-with-preview-traditional. OTOH, when
> register-use-preview is set to 'traditional, one can set it to a
> different value, and then the behavior will change. So I wonder what
> that sentence is about and what I missed? Is that perhaps a leftover
> from previous versions, and should now be deleted?
Yes exactly, it can be deleted.
> I updated the documentation and NEWS. I'd appreciate if you could
> spare a few minutes to review the changes and point out any
> inaccuracies or mistakes.
About the documentation or register-use-preview in the manual:
Another difference with the old behavior (traditional) is that the
preview is filtered according to type of registers used by command. For
example insert-register show only the registers that can be inserted,
not windows, markers etc... Same for jump.
- "Returns a function to format a register for previewing.
-This according to the value of READ-PREVIEW-FUNCTION.")
+ "Return a function to format a register for previewing.
+This is according to the value of `read-preview-function'.")
Here READ-PREVIEW-FUNCTION is the argument of function and not a
variable, so IMO it should be upcased and not quoted.
(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.
+TYPES are the supported types of registers.
+MSG is the minibuffer message to show 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."
+SMATCH accept a boolean value to say if the command accepts non-matching
+registers."
types msg act smatch noconfirm)
NOCONFIRM is not documented (my fault).
When set to nil don't request confirmation with RET.
Otherwise all looks good to me, thanks.
--
Thierry
[signature.asc (application/pgp-signature, inline)]
Reply sent
to
Eli Zaretskii <eliz <at> gnu.org>:
You have taken responsibility.
(Sat, 23 Dec 2023 10:50:02 GMT)
Full text and
rfc822 format available.
Notification sent
to
Thierry Volpiatto <thievol <at> posteo.net>:
bug acknowledged by developer.
(Sat, 23 Dec 2023 10:50:02 GMT)
Full text and
rfc822 format available.
Message #371 received at 66394-done <at> debbugs.gnu.org (full text, mbox):
> From: Thierry Volpiatto <thievol <at> posteo.net>
> Cc: monnier <at> iro.umontreal.ca, stefankangas <at> gmail.com, dmitry <at> gutov.dev,
> michael_heerdegen <at> web.de, 66394 <at> debbugs.gnu.org
> Date: Thu, 21 Dec 2023 18:04:02 +0000
>
> > Thanks, I have a question:
> >
> > The doc string of register-use-preview says:
> >
> > This has no effect when the value of `register--read-with-preview-function'
> > is `register-read-with-preview-traditional'.
> >
> > However, customizing register-use-preview to the value 'traditional
> > has the exact effect of setting register--read-with-preview-function
> > to register-read-with-preview-traditional. OTOH, when
> > register-use-preview is set to 'traditional, one can set it to a
> > different value, and then the behavior will change. So I wonder what
> > that sentence is about and what I missed? Is that perhaps a leftover
> > from previous versions, and should now be deleted?
>
> Yes exactly, it can be deleted.
>
> > I updated the documentation and NEWS. I'd appreciate if you could
> > spare a few minutes to review the changes and point out any
> > inaccuracies or mistakes.
>
> About the documentation or register-use-preview in the manual:
>
> Another difference with the old behavior (traditional) is that the
> preview is filtered according to type of registers used by command. For
> example insert-register show only the registers that can be inserted,
> not windows, markers etc... Same for jump.
>
> - "Returns a function to format a register for previewing.
> -This according to the value of READ-PREVIEW-FUNCTION.")
> + "Return a function to format a register for previewing.
> +This is according to the value of `read-preview-function'.")
>
> Here READ-PREVIEW-FUNCTION is the argument of function and not a
> variable, so IMO it should be upcased and not quoted.
>
> (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.
> +TYPES are the supported types of registers.
> +MSG is the minibuffer message to show 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."
> +SMATCH accept a boolean value to say if the command accepts non-matching
> +registers."
> types msg act smatch noconfirm)
>
> NOCONFIRM is not documented (my fault).
> When set to nil don't request confirmation with RET.
>
> Otherwise all looks good to me, thanks.
Thanks, I fixed all of the above, and I'm therefore closing this bug.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org.
(Sat, 20 Jan 2024 12:24:25 GMT)
Full text and
rfc822 format available.
This bug report was last modified 1 year and 167 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.