bug#46583: 28.0.50; nested minibuffers

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

bug#46583: 28.0.50; nested minibuffers

Juri Linkov-2
>> Let-binding enable-recursive-minibuffers temporarily to t was necessary
>> to fix bug#17272/bug#19064.
>>
>> So when the minibuffer is already activated, and a minibuffer command
>> wants to ask a question, displaying another recursive minibuffer with
>> such question should override the value of enable-recursive-minibuffers.
>
> Those bugs are about an echo-area message overwriting the minibuffer
> prompt, are they not?  If so, doesn't the set-message-function feature
> we now have fixed those bugs indirectly, this removing the need for
> let-binding enable-recursive-minibuffers?

This problem is not fixed by set-message-function that only improves
messaging.  The problem was reported by João in the same bug report
https://debbugs.gnu.org/17272#114

Emacs -Q
M-x fido-mode
C-x b
C-k ;; to kill the Messages buffer

This affects yes-or-no-p that C-k needs to use from the minibuffer
in fido-mode.



Reply | Threaded
Open this post in threaded view
|

bug#46583: 28.0.50; nested minibuffers

Eli Zaretskii
> From: Juri Linkov <[hidden email]>
> Cc: [hidden email],  [hidden email]
> Date: Wed, 17 Feb 2021 22:20:01 +0200
>
> >> Let-binding enable-recursive-minibuffers temporarily to t was necessary
> >> to fix bug#17272/bug#19064.
> >>
> >> So when the minibuffer is already activated, and a minibuffer command
> >> wants to ask a question, displaying another recursive minibuffer with
> >> such question should override the value of enable-recursive-minibuffers.
> >
> > Those bugs are about an echo-area message overwriting the minibuffer
> > prompt, are they not?  If so, doesn't the set-message-function feature
> > we now have fixed those bugs indirectly, this removing the need for
> > let-binding enable-recursive-minibuffers?
>
> This problem is not fixed by set-message-function that only improves
> messaging.

It prevents messages from overwriting minibuffer prompts.  And both of
the bugs you mentioned seem to be about messages that overwrite such
prompts.  So what am I missing?

> The problem was reported by João in the same bug report
> https://debbugs.gnu.org/17272#114
>
> Emacs -Q
> M-x fido-mode
> C-x b
> C-k ;; to kill the Messages buffer
>
> This affects yes-or-no-p that C-k needs to use from the minibuffer
> in fido-mode.

Sorry, I don't understand: how is this related to what I asked about?



Reply | Threaded
Open this post in threaded view
|

bug#46583: 28.0.50; nested minibuffers

Juri Linkov-2
> It prevents messages from overwriting minibuffer prompts.  And both of
> the bugs you mentioned seem to be about messages that overwrite such
> prompts.  So what am I missing?

Sorry for mentioning old bugs that added confusion to this bug report.
What we have now is a new bug found by Richard:

  I save a file that is read-only, so I am asked

    File phones is write-protected; try to save anyway? (yes or no)

  Then I type C-x b and it tries to read a buffer name.

I could send a patch that fixes this bug.



Reply | Threaded
Open this post in threaded view
|

bug#46583: 28.0.50; nested minibuffers

Juri Linkov-2
>> I could send a patch that fixes this bug.
>
> Thanks.  I hoped first to understand why we bind
> enable-recursive-minibuffers in these cases, but if you prefer to
> begin with a patch, please do, and let's take it from there.

To understand the problem, it's better to start from the manual.
The node (info "(elisp) Recursive Mini") says:

     If a command name has a property ‘enable-recursive-minibuffers’ that
  is non-‘nil’, then the command can use the minibuffer to read arguments
  even if it is invoked from the minibuffer.  A command can also achieve
  this by binding ‘enable-recursive-minibuffers’ to ‘t’ in the interactive
  declaration (*note Using Interactive::).  The minibuffer command
  ‘next-matching-history-element’ (normally ‘M-s’ in the minibuffer) does
  the latter.

And indeed the command ‘next-matching-history-element’ (‘M-s’)
mentioned as an example in the documentation, exhibits the same
problem reported by Richard:

0. emacs -Q
1. M-x                  ;; execute-extended-command
2. M-s                  ;; next-matching-history-element
3. C-x b                ;; switch-to-buffer

And it tries to read a buffer name, ignoring the default nil value
of ‘enable-recursive-minibuffers’.

This is because the manual documents the official way by binding
‘enable-recursive-minibuffers’ to ‘t’ that next-matching-history-element
does:

  (defun next-matching-history-element (regexp n)
    (interactive
     (let* ((enable-recursive-minibuffers t)
            (regexp (read-from-minibuffer "Next element matching (regexp): "
  ...

One possible solution is to support an additional value ‘transient’
for the variable ‘enable-recursive-minibuffers’.  Then this value
could have its effect only for the next invocation of
read-from-minibuffer.  Afterwards it will be set to ‘nil’
for any further recursive calls of read-from-minibuffer.

PS: Such transient value is similar to the value ‘lambda’ of
‘transient-mark-mode’ described in (info "(elisp) The Mark").

Currently this patch fixes the reported problem only
for the commands yes-or-no-p and next-matching-history-element:


diff --git a/src/minibuf.c b/src/minibuf.c
index 4b1f4b1ff7..8a46693846 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -592,7 +592,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   if (!STRINGP (prompt))
     prompt = empty_unibyte_string;
 
-  if (!enable_recursive_minibuffers
+  if (NILP (Venable_recursive_minibuffers)
       && minibuf_level > 0)
     {
       Lisp_Object str
@@ -604,6 +604,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
  Fthrow (Qexit, str);
     }
 
+  if (EQ (Venable_recursive_minibuffers, Qtransient))
+    specbind (Qenable_recursive_minibuffers, Qnil);
+
   if ((noninteractive
        /* In case we are running as a daemon, only do this before
   detaching from the terminal.  */
@@ -2242,6 +2245,7 @@ syms_of_minibuf (void)
   DEFSYM (Qcase_fold_search, "case-fold-search");
   DEFSYM (Qmetadata, "metadata");
   DEFSYM (Qcycle_sort_function, "cycle-sort-function");
+  DEFSYM (Qtransient, "transient");
 
   /* A frame parameter.  */
   DEFSYM (Qminibuffer_exit, "minibuffer-exit");
@@ -2311,12 +2315,12 @@ syms_of_minibuf (void)
 controls the behavior, rather than this variable.  */);
   completion_ignore_case = 0;
 
-  DEFVAR_BOOL ("enable-recursive-minibuffers", enable_recursive_minibuffers,
+  DEFVAR_LISP ("enable-recursive-minibuffers", Venable_recursive_minibuffers,
        doc: /* Non-nil means to allow minibuffer commands while in the minibuffer.
 This variable makes a difference whenever the minibuffer window is active.
 Also see `minibuffer-depth-indicate-mode', which may be handy if this
 variable is non-nil. */);
-  enable_recursive_minibuffers = 0;
+  Venable_recursive_minibuffers = Qnil;
 
   DEFVAR_LISP ("minibuffer-completion-table", Vminibuffer_completion_table,
        doc: /* Alist or obarray used for completion in the minibuffer.
diff --git a/src/fns.c b/src/fns.c
index f51ef2781d..4744fddf27 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -2882,7 +2882,8 @@ DEFUN ("yes-or-no-p", Fyes_or_no_p, Syes_or_no_p, 1, 1, 0,
   prompt = CALLN (Fconcat, prompt, yes_or_no);
 
   ptrdiff_t count = SPECPDL_INDEX ();
-  specbind (Qenable_recursive_minibuffers, Qt);
+  if (NILP (Venable_recursive_minibuffers))
+    specbind (Qenable_recursive_minibuffers, Qtransient);
 
   while (1)
     {
diff --git a/lisp/simple.el b/lisp/simple.el
index 7eee65e204..99b959b2fc 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -2370,7 +2370,7 @@ next-matching-history-element
 `case-fold-search' is non-nil, but an uppercase letter in REGEXP
 makes the search case-sensitive."
   (interactive
-   (let* ((enable-recursive-minibuffers t)
+   (let* ((enable-recursive-minibuffers (or enable-recursive-minibuffers 'transient))
   (regexp (read-from-minibuffer "Next element matching (regexp): "
  nil
  minibuffer-local-map