unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
@ 2023-12-07 22:33 Kun Liu
  2023-12-08  6:32 ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Kun Liu @ 2023-12-07 22:33 UTC (permalink / raw)
  To: 67702

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

1) Save some text in register a
2) M-x query-replace
3) in the minibuffer, type C-x r i
4) got error: "byte-code: Command attempted to use minibuffer while in
minibuffer"


In GNU Emacs 30.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version
 3.24.24, cairo version 1.16.0) of 2023-12-07 built on debian
Repository revision: 3b1fd42732f7ca5b2db6ad6e75af1c037e1c54e4
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-native-compilation --with-tree-sitter'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBOTF LIBSELINUX LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY INOTIFY
PDUMPER PNG RSVG SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER X11 XDBE XIM XINPUT2 XPM GTK3 ZLIB

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

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr compile comint ansi-osc ansi-color ring comp-run
bytecomp byte-compile comp-common rx emacsbug message mailcap yank-media
puny dired dired-loaddefs rfc822 mml mml-sec password-cache epa derived
epg rfc6068 epg-config gnus-util text-property-search time-date subr-x
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils 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 touch-screen 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
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process
native-compile emacs)

Memory information:
((conses 16 67807 9163) (symbols 48 6580 0) (strings 32 17139 1182)
 (string-bytes 1 532845) (vectors 16 12759)
 (vector-slots 8 294006 13213) (floats 8 23 63) (intervals 56 266 0)
 (buffers 992 12))

[-- Attachment #2: Type: text/html, Size: 3642 bytes --]

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-07 22:33 bug#67702: 30.0.50; insert-register can no longer be used in minibuffer Kun Liu
@ 2023-12-08  6:32 ` Eli Zaretskii
  2023-12-08  7:04   ` Thierry Volpiatto
  0 siblings, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-12-08  6:32 UTC (permalink / raw)
  To: Kun Liu, Thierry Volpiatto; +Cc: 67702

> From: Kun Liu <kun.liu@gmail.com>
> Date: Thu, 7 Dec 2023 14:33:33 -0800
> 
> 1) Save some text in register a
> 2) M-x query-replace
> 3) in the minibuffer, type C-x r i
> 4) got error: "byte-code: Command attempted to use minibuffer while in minibuffer"

I guess register-read-with-preview should temporarily bind
enable-recursive-minibuffers to a non-nil value?  Adding Thierry to
the discussion.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  6:32 ` Eli Zaretskii
@ 2023-12-08  7:04   ` Thierry Volpiatto
  2023-12-08  7:14     ` Thierry Volpiatto
  2023-12-08  7:15     ` Eli Zaretskii
  0 siblings, 2 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08  7:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Kun Liu, 67702

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Kun Liu <kun.liu@gmail.com>
>> Date: Thu, 7 Dec 2023 14:33:33 -0800
>> 
>> 1) Save some text in register a
>> 2) M-x query-replace
>> 3) in the minibuffer, type C-x r i
>> 4) got error: "byte-code: Command attempted to use minibuffer while in minibuffer"
>
> I guess register-read-with-preview should temporarily bind
> enable-recursive-minibuffers to a non-nil value?

Yes, do you want me to install this change?
I have also a pending patch to apply for bug#66394 (see it there).

> Adding Thierry to the discussion.


-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:04   ` Thierry Volpiatto
@ 2023-12-08  7:14     ` Thierry Volpiatto
  2023-12-08  7:15     ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08  7:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Kun Liu, 67702


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

Thierry Volpiatto <thievol@posteo.net> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Kun Liu <kun.liu@gmail.com>
>>> Date: Thu, 7 Dec 2023 14:33:33 -0800
>>> 
>>> 1) Save some text in register a
>>> 2) M-x query-replace
>>> 3) in the minibuffer, type C-x r i
>>> 4) got error: "byte-code: Command attempted to use minibuffer while in minibuffer"
>>
>> I guess register-read-with-preview should temporarily bind
>> enable-recursive-minibuffers to a non-nil value?
>
> Yes, do you want me to install this change?

Here a patch attached.

-- 
Thierry

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: 0001-Allow-inserting-registers-in-minibuffer-bug-67702.patch --]
[-- Type: text/x-diff, Size: 899 bytes --]

From 09c00dd82d377972bb9cc8b789156993f27c1026 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Fri, 8 Dec 2023 08:09:38 +0100
Subject: [PATCH] Allow inserting registers in minibuffer (bug#67702)

* lisp/register.el (register-read-with-preview): Bind
`enable-recursive-minibuffers`.
---
 lisp/register.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/register.el b/lisp/register.el
index a38b531dfc9..ba00f296af9 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -315,6 +315,7 @@ display such a window regardless."
                 (set-keymap-parent m minibuffer-local-map)
                 m))
          (data (register-command-info this-command))
+         (enable-recursive-minibuffers t)
          types msg result timer act win strs smatch)
     (if data
         (setq types  (register-preview-info-types data)
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:04   ` Thierry Volpiatto
  2023-12-08  7:14     ` Thierry Volpiatto
@ 2023-12-08  7:15     ` Eli Zaretskii
  2023-12-08  7:31       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  1 sibling, 1 reply; 19+ messages in thread
From: Eli Zaretskii @ 2023-12-08  7:15 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: kun.liu, 67702

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: Kun Liu <kun.liu@gmail.com>,  67702@debbugs.gnu.org
> Date: Fri, 08 Dec 2023 07:04:53 +0000
> 
> >> 1) Save some text in register a
> >> 2) M-x query-replace
> >> 3) in the minibuffer, type C-x r i
> >> 4) got error: "byte-code: Command attempted to use minibuffer while in minibuffer"
> >
> > I guess register-read-with-preview should temporarily bind
> > enable-recursive-minibuffers to a non-nil value?
> 
> Yes, do you want me to install this change?

If you think that's the correct solution, sure.

> I have also a pending patch to apply for bug#66394 (see it there).

You said you will install that if no one objects.  I'm not sure what
was the conclusion of the discussion with Eshel about that.  If it's
unrelated, then please go ahead and install your changes in that
discussion.  In any case, perhaps you could help Eshell improve and
polish his additions, which AFAIU are supposed to provide an optional
behavior more similar to the previous one.

Thanks.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:15     ` Eli Zaretskii
@ 2023-12-08  7:31       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08  7:52         ` Eli Zaretskii
  0 siblings, 1 reply; 19+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-08  7:31 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Thierry Volpiatto, kun.liu, 67702

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Cc: Kun Liu <kun.liu@gmail.com>,  67702@debbugs.gnu.org
>> Date: Fri, 08 Dec 2023 07:04:53 +0000
>>
>> >> 1) Save some text in register a
>> >> 2) M-x query-replace
>> >> 3) in the minibuffer, type C-x r i
>> >> 4) got error: "byte-code: Command attempted to use minibuffer while in minibuffer"

Yes, this regression is one of the issues I pointed at with the recent
`register-read-with-preview` changes.

>> > I guess register-read-with-preview should temporarily bind
>> > enable-recursive-minibuffers to a non-nil value?
>>
>> Yes, do you want me to install this change?
>
> If you think that's the correct solution, sure.

FWIW, I think it's not the right solution.  As I wrote in bug#66394, I
think it's wrong to involve the minibuffer in reading registers in any
way.  `enable-recursive-minibuffers` would make this less broken, but
only slightly.

>> I have also a pending patch to apply for bug#66394 (see it there).
>
> You said you will install that if no one objects.  I'm not sure what
> was the conclusion of the discussion with Eshel about that.

It's up to you maintainers to decide, I think.  Following your request,
I've proposed a patch that reverts Thierry's changes, and implements the
parts I find useful in a clean and backward compatible way.

> If it's unrelated, then please go ahead and install your changes in
> that discussion.  In any case, perhaps you could help Eshel improve
> and polish his additions, which AFAIU are supposed to provide an
> optional behavior more similar to the previous one.

That'd be nice, thanks.


Best,

Eshel






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:31       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-08  7:52         ` Eli Zaretskii
  2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08  8:46           ` Thierry Volpiatto
  0 siblings, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2023-12-08  7:52 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: thievol, kun.liu, 67702

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: Thierry Volpiatto <thievol@posteo.net>,  kun.liu@gmail.com,
>   67702@debbugs.gnu.org
> Date: Fri, 08 Dec 2023 08:31:15 +0100
> 
> >> > I guess register-read-with-preview should temporarily bind
> >> > enable-recursive-minibuffers to a non-nil value?
> >>
> >> Yes, do you want me to install this change?
> >
> > If you think that's the correct solution, sure.
> 
> FWIW, I think it's not the right solution.  As I wrote in bug#66394, I
> think it's wrong to involve the minibuffer in reading registers in any
> way.  `enable-recursive-minibuffers` would make this less broken, but
> only slightly.

I'm not sure I understand: if we put aside the fundamental opposition
to using read-from-minibuffer, what problems will be left if we
temporarily enable recursive-minibuffers while prompting for the
register?

> It's up to you maintainers to decide, I think.  Following your request,
> I've proposed a patch that reverts Thierry's changes, and implements the
> parts I find useful in a clean and backward compatible way.

Thierry said your patch was incomplete.  And I wonder why we need to
completely revert his changes.  My suggestion was to allow both,
controlled by user option.  Then we could see what users prefer, and
make the decision: whether to keep both or just one, and which one.  I
very much prefer this path forward than continuing to argue now which
of the two ways is the only one that's correct.

> > If it's unrelated, then please go ahead and install your changes in
> > that discussion.  In any case, perhaps you could help Eshel improve
> > and polish his additions, which AFAIU are supposed to provide an
> > optional behavior more similar to the previous one.
> 
> That'd be nice, thanks.

Agreed.  But again: please consider reworking your patch such that it
allows using the minibuffer as optional behavior.  After all, using
the minibuffer has also advantages, not just disadvantages.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:52         ` Eli Zaretskii
@ 2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08  8:51             ` Eli Zaretskii
  2023-12-08 10:16             ` Thierry Volpiatto
  2023-12-08  8:46           ` Thierry Volpiatto
  1 sibling, 2 replies; 19+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-08  8:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: thievol, kun.liu, 67702

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>>
>> >> > I guess register-read-with-preview should temporarily bind
>> >> > enable-recursive-minibuffers to a non-nil value?
>> >>
>> >> Yes, do you want me to install this change?
>> >
>> > If you think that's the correct solution, sure.
>>
>> FWIW, I think it's not the right solution.  As I wrote in bug#66394, I
>> think it's wrong to involve the minibuffer in reading registers in any
>> way.  `enable-recursive-minibuffers` would make this less broken, but
>> only slightly.
>
> I'm not sure I understand: if we put aside the fundamental opposition
> to using read-from-minibuffer, what problems will be left if we
> temporarily enable recursive-minibuffers while prompting for the
> register?

Concretely this is still worse because starting a recursive minibuffer
hides the previous minibuffer.  So you no longer see what you're
operating on.  There are other problems that I mentioned in bug#66394,
and there's also the disadvantage that `read-from-minibuffer` switches
windows, which is redundant in this case.

>> It's up to you maintainers to decide, I think.  Following your request,
>> I've proposed a patch that reverts Thierry's changes, and implements the
>> parts I find useful in a clean and backward compatible way.
>
> Thierry said your patch was incomplete.

Well, I requested some elaboration on that comment.  Still waiting.

> And I wonder why we need to completely revert his changes.  My
> suggestion was to allow both, controlled by user option.

That's basically what my patch does, it even makes Thierry's preferred
behavior (confirmation before overwriting registers) the default, it
just doesn't use the minibuffer for that.  AFAIU the goal of Thierry's
patch wasn't to use the minibuffer, that's an implementation detail, one
with problematic consequences.

> Then we could see what users prefer, and make the decision: whether to
> keep both or just one, and which one.  I very much prefer this path
> forward than continuing to argue now which of the two ways is the only
> one that's correct.

>> > If it's unrelated, then please go ahead and install your changes in
>> > that discussion.  In any case, perhaps you could help Eshel improve
>> > and polish his additions, which AFAIU are supposed to provide an
>> > optional behavior more similar to the previous one.
>>
>> That'd be nice, thanks.
>
> Agreed.  But again: please consider reworking your patch such that it
> allows using the minibuffer as optional behavior.  After all, using
> the minibuffer has also advantages, not just disadvantages.

I don't see these advantages.  Again, I think this is just an
implementation detail, and my patch provides a better implementation.
If someone would spell out these advantages, I could look into it when I
have the time.


Eshel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  7:52         ` Eli Zaretskii
  2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-08  8:46           ` Thierry Volpiatto
  1 sibling, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08  8:46 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, Eshel Yaron, 67702

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

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure I understand: if we put aside the fundamental opposition
> to using read-from-minibuffer, what problems will be left if we
> temporarily enable recursive-minibuffers while prompting for the
> register?

The only complaint now (once my two missing patches will be applied) is
that we have to use C-d to save a key sequence register e.g. for saving
C-a we have to use C-q C-a.  For jumping to such a register we have to
insert in minibuffer C-q C-a or just use the navigation keys (C-n/p).

>
>> It's up to you maintainers to decide, I think.  Following your request,
>> I've proposed a patch that reverts Thierry's changes, and implements the
>> parts I find useful in a clean and backward compatible way.
>
> Thierry said your patch was incomplete.

It is, the use of read-key is a limitation in the features we provide,
the Eshel patch, as already said have no filtering, no possibility to
configure possible new future register commands, no navigation, no
defaults  etc...
Also it reintroduce a bad implementation of the usage of read-key which
lead to things like bug#27634 (even if this one was fixed, incorrectly IMHO). 

> And I wonder why we need to completely revert his changes.

Reverting those changes would be a regression, I have now a patch that
make register behave as before:

Here the docstring of register-use-preview:

    Maybe show register preview.

    When set to `t' show a preview buffer with navigation and highlighting.
    When nil show a basic preview buffer and exit minibuffer
    immediately after insertion in minibuffer.
    When set to 'never behave as above but with no preview buffer at
    all.

IOW (setq register-use-preview nil) will behave as before.

-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-08  8:51             ` Eli Zaretskii
  2023-12-08  9:07               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08 10:23               ` Thierry Volpiatto
  2023-12-08 10:16             ` Thierry Volpiatto
  1 sibling, 2 replies; 19+ messages in thread
From: Eli Zaretskii @ 2023-12-08  8:51 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: thievol, kun.liu, 67702

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: thievol@posteo.net,  kun.liu@gmail.com,  67702@debbugs.gnu.org
> Date: Fri, 08 Dec 2023 09:27:22 +0100
> 
> > Agreed.  But again: please consider reworking your patch such that it
> > allows using the minibuffer as optional behavior.  After all, using
> > the minibuffer has also advantages, not just disadvantages.
> 
> I don't see these advantages.

Well, Thierry does, so let's assume that some others will as well.

in general, assuming that others are definitely wrong is not always a
useful stance in such discussions.  It is better to let each one to
use the behavior they like, if that is feasible, and agree to disagree
about the rest.





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  8:51             ` Eli Zaretskii
@ 2023-12-08  9:07               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08 10:23               ` Thierry Volpiatto
  1 sibling, 0 replies; 19+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-08  9:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: thievol, kun.liu, 67702

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>

>> > Agreed.  But again: please consider reworking your patch such that it
>> > allows using the minibuffer as optional behavior.  After all, using
>> > the minibuffer has also advantages, not just disadvantages.
>>
>> I don't see these advantages.
>
> Well, Thierry does, so let's assume that some others will as well.

I'm not trying to be argumentative here, I hope it's clear I'm just
trying to help avoid a regression in Emacs 30.

Now, if Thierry would be kind enough to share those use cases, that
might allow for a more productive discussion.

> in general, assuming that others are definitely wrong is not always a
> useful stance in such discussions.

Of course, I don't assume that, I apologize if that was the impression
that came across.  Note that my very next sentence was:

  If someone would spell out these advantages, I could look into it when
  I have the time.

> It is better to let each one to use the behavior they like, if that is
> feasible, and agree to disagree about the rest.

Yes, that'd be great.  But currently on the master branch the previous
behavior is nowhere to be found.  What I (and others, I think) ask for
is restoring the previous behavior, in full, at least optionally.  If my
patch isn't good enough, and there's another solution at hand, that's
absolutely fine.


Thanks,

Eshel





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08  8:51             ` Eli Zaretskii
@ 2023-12-08 10:16             ` Thierry Volpiatto
  2023-12-08 10:40               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08 12:00               ` Eli Zaretskii
  1 sibling, 2 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 10:16 UTC (permalink / raw)
  To: Eshel Yaron; +Cc: kun.liu, Eli Zaretskii, 67702

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

Eshel Yaron <me@eshelyaron.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: Eshel Yaron <me@eshelyaron.com>
>>>
>>> >> > I guess register-read-with-preview should temporarily bind
>>> >> > enable-recursive-minibuffers to a non-nil value?
>>> >>
>>> >> Yes, do you want me to install this change?
>>> >
>>> > If you think that's the correct solution, sure.
>>>
>>> FWIW, I think it's not the right solution.  As I wrote in bug#66394, I
>>> think it's wrong to involve the minibuffer in reading registers in any
>>> way.  `enable-recursive-minibuffers` would make this less broken, but
>>> only slightly.
>>
>> I'm not sure I understand: if we put aside the fundamental opposition
>> to using read-from-minibuffer, what problems will be left if we
>> temporarily enable recursive-minibuffers while prompting for the
>> register?
>
> Concretely this is still worse because starting a recursive minibuffer
> hides the previous minibuffer.  So you no longer see what you're
> operating on.  There are other problems that I mentioned in bug#66394,
> and there's also the disadvantage that `read-from-minibuffer` switches
> windows, which is redundant in this case.
>
>>> It's up to you maintainers to decide, I think.  Following your request,
>>> I've proposed a patch that reverts Thierry's changes, and implements the
>>> parts I find useful in a clean and backward compatible way.
>>
>> Thierry said your patch was incomplete.
>
> Well, I requested some elaboration on that comment.  Still waiting.

I gave you twice the explanations, now please read the code to understand
what's going on, thanks.

>> And I wonder why we need to completely revert his changes.  My
>> suggestion was to allow both, controlled by user option.
>
> That's basically what my patch does, it even makes Thierry's preferred
> behavior (confirmation before overwriting registers) the default, it
> just doesn't use the minibuffer for that.

No, you didn't understand what the recent changes provide, "confirmation
before overwriting registers" is only one of the features provided.
Please read the code and try all the features provided, my guess is that
you even didn't try it. 

> AFAIU the goal of Thierry's patch wasn't to use the minibuffer, that's
> an implementation detail, one with problematic consequences.

There is no consequence apart one, as I already said, the usage of C-d.


-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08  8:51             ` Eli Zaretskii
  2023-12-08  9:07               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-08 10:23               ` Thierry Volpiatto
  1 sibling, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 10:23 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, Eshel Yaron, 67702

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>> Cc: thievol@posteo.net,  kun.liu@gmail.com,  67702@debbugs.gnu.org
>> Date: Fri, 08 Dec 2023 09:27:22 +0100
>> 
>> > Agreed.  But again: please consider reworking your patch such that it
>> > allows using the minibuffer as optional behavior.  After all, using
>> > the minibuffer has also advantages, not just disadvantages.
>> 
>> I don't see these advantages.
>
> Well, Thierry does, so let's assume that some others will as well.
>
> in general, assuming that others are definitely wrong is not always a
> useful stance in such discussions.  It is better to let each one to
> use the behavior they like, if that is feasible,

Eli, if you agree, I will push the last changes I made which provide the
same behavior as before (but with filtering).
Once this work will be finished, let see what people think and if the
new behavior is really not what people want we could revert completely
my changes.

Thanks.

-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 10:16             ` Thierry Volpiatto
@ 2023-12-08 10:40               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
  2023-12-08 12:00               ` Eli Zaretskii
  1 sibling, 0 replies; 19+ messages in thread
From: Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2023-12-08 10:40 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: kun.liu, Eli Zaretskii, 67702

Thierry Volpiatto <thievol@posteo.net> writes:

> Eshel Yaron <me@eshelyaron.com> writes:
>
>> Well, I requested some elaboration on that comment.  Still waiting.
>
> I gave you twice the explanations

Nope, you've listed some changes that I've (intentionally) omitted, but
I kindly requested that you explain the use case for those additional
changes, i.e. in which cases they provide a tangible benefit over the
current behavior.  That's what I don't see, and your code doesn't
explain that either.






^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 10:16             ` Thierry Volpiatto
  2023-12-08 10:40               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2023-12-08 12:00               ` Eli Zaretskii
  2023-12-08 12:11                 ` Thierry Volpiatto
                                   ` (2 more replies)
  1 sibling, 3 replies; 19+ messages in thread
From: Eli Zaretskii @ 2023-12-08 12:00 UTC (permalink / raw)
  To: Thierry Volpiatto; +Cc: kun.liu, me, 67702

> From: Thierry Volpiatto <thievol@posteo.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  kun.liu@gmail.com,  67702@debbugs.gnu.org
> Date: Fri, 08 Dec 2023 10:16:37 +0000
> 
> > AFAIU the goal of Thierry's patch wasn't to use the minibuffer, that's
> > an implementation detail, one with problematic consequences.
> 
> There is no consequence apart one, as I already said, the usage of C-d.

You mean C-q, I presume?





^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 12:00               ` Eli Zaretskii
@ 2023-12-08 12:11                 ` Thierry Volpiatto
  2023-12-08 12:39                 ` Thierry Volpiatto
  2023-12-08 12:40                 ` Thierry Volpiatto
  2 siblings, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 12:11 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, me, 67702

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Thierry Volpiatto <thievol@posteo.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  kun.liu@gmail.com,  67702@debbugs.gnu.org
>> Date: Fri, 08 Dec 2023 10:16:37 +0000
>> 
>> > AFAIU the goal of Thierry's patch wasn't to use the minibuffer, that's
>> > an implementation detail, one with problematic consequences.
>> 
>> There is no consequence apart one, as I already said, the usage of C-d.
>
> You mean C-q, I presume?

Yes, sorry.

-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 12:00               ` Eli Zaretskii
  2023-12-08 12:11                 ` Thierry Volpiatto
@ 2023-12-08 12:39                 ` Thierry Volpiatto
  2023-12-08 12:40                 ` Thierry Volpiatto
  2 siblings, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 12:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, me, 67702

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


So to summarize here the last patches not already merged, let me know if
I can merge them.

Thanks.

PS: I will update NEWS and perhaps info once the behavior will be fully
defined.

-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 12:00               ` Eli Zaretskii
  2023-12-08 12:11                 ` Thierry Volpiatto
  2023-12-08 12:39                 ` Thierry Volpiatto
@ 2023-12-08 12:40                 ` Thierry Volpiatto
  2023-12-08 16:37                   ` Thierry Volpiatto
  2 siblings, 1 reply; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 12:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, me, 67702


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


Here the patches, sorry.


-- 
Thierry

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

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

is non nil and .

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

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.3: 0002-Allow-inserting-registers-in-minibuffer-bug-67702.patch --]
[-- Type: text/x-diff, Size: 903 bytes --]

From 09c00dd82d377972bb9cc8b789156993f27c1026 Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Fri, 8 Dec 2023 08:09:38 +0100
Subject: [PATCH 2/3] Allow inserting registers in minibuffer (bug#67702)

* lisp/register.el (register-read-with-preview): Bind
`enable-recursive-minibuffers`.
---
 lisp/register.el | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lisp/register.el b/lisp/register.el
index a38b531dfc9..ba00f296af9 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -315,6 +315,7 @@ display such a window regardless."
                 (set-keymap-parent m minibuffer-local-map)
                 m))
          (data (register-command-info this-command))
+         (enable-recursive-minibuffers t)
          types msg result timer act win strs smatch)
     (if data
         (setq types  (register-preview-info-types data)
-- 
2.34.1


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.4: 0003-Add-more-options-to-register-use-preview.patch --]
[-- Type: text/x-diff, Size: 3625 bytes --]

From 633fa5245072a746d122eb6eba11751033749afc Mon Sep 17 00:00:00 2001
From: Thierry Volpiatto <thievol@posteo.net>
Date: Fri, 8 Dec 2023 11:34:08 +0100
Subject: [PATCH 3/3] Add more options to register-use-preview

This allow showing a basic preview buffer or no preview buffer at all.

* lisp/register.el (register-use-preview): Use choice with three
  options.
(register-read-with-preview): Use a basic buffer without navigation,
  highlighting etc... when register-use-preview is nil, and no buffer
  at all when set to 'never.
---
 lisp/register.el | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/lisp/register.el b/lisp/register.el
index ba00f296af9..ade65b5bdc2 100644
--- a/lisp/register.el
+++ b/lisp/register.el
@@ -107,8 +107,17 @@ If nil, do not show register previews, unless `help-char' (or a member of
   :type '(repeat string))
 
 (defcustom register-use-preview t
-  "Always show register preview when non nil."
-  :type 'boolean)
+  "Maybe show register preview.
+
+When set to `t' show a preview buffer with navigation and highlighting.
+When nil show a basic preview buffer and exit minibuffer
+immediately after insertion in minibuffer.
+When set to \\='never behave as above but with no preview buffer at
+all."
+  :type '(choice
+          (const :tag "Use preview" t)
+          (const :tag "Use quick preview" nil)
+          (const :tag "Never use preview" never)))
 
 (defun get-register (register)
   "Return contents of Emacs register named REGISTER, or nil if none."
@@ -310,6 +319,8 @@ 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*")
+         (buffer1 "*Register quick preview*")
+         (buf (if register-use-preview buffer buffer1))
          (pat "")
          (map (let ((m (make-sparse-keymap)))
                 (set-keymap-parent m minibuffer-local-map)
@@ -334,15 +345,16 @@ display such a window regardless."
       (define-key map
           (vector k) (lambda ()
                        (interactive)
-                       (unless (get-buffer-window buffer)
+                       ;; Do nothing when buffer1 is in use.
+                       (unless (get-buffer-window buf)
                          (with-selected-window (minibuffer-selected-window)
                            (register-preview buffer 'show-empty types))))))
     (define-key map (kbd "<down>") 'register-preview-next)
     (define-key map (kbd "<up>")   'register-preview-previous)
     (define-key map (kbd "C-n")    'register-preview-next)
     (define-key map (kbd "C-p")    'register-preview-previous)
-    (unless (or executing-kbd-macro (null register-use-preview))
-      (register-preview buffer nil types))
+    (unless (or executing-kbd-macro (eq register-use-preview 'never))
+      (register-preview buf nil types))
     (unwind-protect
          (progn
            (minibuffer-with-setup-hook
@@ -402,9 +414,9 @@ display such a window regardless."
                       nil "No register specified")
            (string-to-char result))
       (when timer (cancel-timer timer))
-      (let ((w (get-buffer-window buffer)))
+      (let ((w (get-buffer-window buf)))
         (and (window-live-p w) (delete-window w)))
-      (and (get-buffer buffer) (kill-buffer buffer)))))
+      (and (get-buffer buf) (kill-buffer buf)))))
 
 (defun point-to-register (register &optional arg)
   "Store current location of point in REGISTER.
-- 
2.34.1


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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* bug#67702: 30.0.50; insert-register can no longer be used in minibuffer
  2023-12-08 12:40                 ` Thierry Volpiatto
@ 2023-12-08 16:37                   ` Thierry Volpiatto
  0 siblings, 0 replies; 19+ messages in thread
From: Thierry Volpiatto @ 2023-12-08 16:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: kun.liu, me, 67702

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

Thierry Volpiatto <thievol@posteo.net> writes:

> Here the patches, sorry.

Now pushed to master, after I may not have the time (and network) to work on this.

So now to have the previous behavior:

    (setq register-use-preview nil)

If people are still not happy, please revert all these changes to what
we had.

Thanks.

-- 
Thierry

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2023-12-08 16:37 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-07 22:33 bug#67702: 30.0.50; insert-register can no longer be used in minibuffer Kun Liu
2023-12-08  6:32 ` Eli Zaretskii
2023-12-08  7:04   ` Thierry Volpiatto
2023-12-08  7:14     ` Thierry Volpiatto
2023-12-08  7:15     ` Eli Zaretskii
2023-12-08  7:31       ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-08  7:52         ` Eli Zaretskii
2023-12-08  8:27           ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-08  8:51             ` Eli Zaretskii
2023-12-08  9:07               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-08 10:23               ` Thierry Volpiatto
2023-12-08 10:16             ` Thierry Volpiatto
2023-12-08 10:40               ` Eshel Yaron via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-12-08 12:00               ` Eli Zaretskii
2023-12-08 12:11                 ` Thierry Volpiatto
2023-12-08 12:39                 ` Thierry Volpiatto
2023-12-08 12:40                 ` Thierry Volpiatto
2023-12-08 16:37                   ` Thierry Volpiatto
2023-12-08  8:46           ` Thierry Volpiatto

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).