unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Re: Stop frames stealing eachothers' minibuffers!
@ 2021-02-06 23:25 jakanakaevangeli
  2021-02-07 12:55 ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: jakanakaevangeli @ 2021-02-06 23:25 UTC (permalink / raw)
  To: acm; +Cc: emacs-devel

Thanks for your hard work.
Your patch fixes the mentioned problems, save for the following
regression, which isn't related to minibuffers: upon
entering two or more M-x recursive-edit, pressing C-]
(abort-recursive-edit) will take you to top-level rather than quit just
one recursive edit and produce an error "No catch for tag: exit, t".

> The handling you're proposing for exit-read-minibuffer is no less
> complicated than what's already there for exit.

Actually with function minibuf_c_loop_level, introduced in the patch,
you could probably do something similar to my proposed idea by adjusting
the catch for Qexit (in command_loop in src/keyboard.c) rather than
introducing a new catch for a new Qexit_read_minibuffer.

If you have the time and energy, you can read more about this below,
where I brainstorm two more ideas.

The way I see it, there is, in essence, one problem: we want to quit out
of multiple recursive-edit levels. And for this, there are three
solutions:

1) internal_catch shall detect if we are in a non-inner minibuffer and
   re-throw Qexit until necessary. This is the current solution.

   Pros:
   - The simplest solution and easiest to maintain
   Cons:
   - internal_catch special cases Qexit and is dependent on
     Fcurrent_buffer
   (Note that my experience in lisp programming is limited and my gut
   feeling that this "isn't clean" may be completely misplaced.)

2) command_loop (in src/keyboard.c) shall detect if we are in a
   non-inner minibuffer and re-throw Qexit until necessary. So
   basically, move the code from internal_catch to command_loop or
   recursive_edit_1.

   Cons:
   - A bit harder to implement and maintain

3) The catcher for Qexit in command_loop shall be generalized to accept
   an integer value N, which would mean to re-throw to Qexit with N-1
   (or quit if N<=1) and quit-minibuffers shall be adjusted to calculate
   how many minibuffers to quit with the help of your new
   minibuf_c_loop_level function.

   Cons:
   - The most complicated solution
   - The new (throw 'exit N) API will have to be documented and
     maintained
   - Will probably not be all that useful for users
   - In could even cause a lot more problems that its worth
   - Hard to deprecate (perhaps its use could be discouraged or the API
     considered not supported and only to be used by internal Emacs
     commands like quit-minibuffers)
   Pros:
   - does not rely on static variables
   - isolates multi-level quitting only to a single command
     (quit-minibuffers)
   - Users might get a new possibly useful API for quitting multiple
     levels



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-06 23:25 Stop frames stealing eachothers' minibuffers! jakanakaevangeli
@ 2021-02-07 12:55 ` Alan Mackenzie
  2021-02-07 16:44   ` jakanakaevangeli
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-02-07 12:55 UTC (permalink / raw)
  To: jakanakaevangeli; +Cc: emacs-devel

Hello again, jakanakaevangeli.

On Sun, Feb 07, 2021 at 00:25:56 +0100, jakanakaevangeli wrote:
> Thanks for your hard work.

Just as a matter of interest, your email host rejected my post yesterday,
saying that the interchange had to start with a STARTTLS command.

> Your patch fixes the mentioned problems, save for the following
> regression, which isn't related to minibuffers: upon entering two or
> more M-x recursive-edit, pressing C-] (abort-recursive-edit) will take
> you to top-level rather than quit just one recursive edit and produce
> an error "No catch for tag: exit, t".

Yes.  That happens even on C-] when there's just a single recursive edit
in progress.

> > The handling you're proposing for exit-read-minibuffer is no less
> > complicated than what's already there for exit.

> Actually with function minibuf_c_loop_level, introduced in the patch,
> you could probably do something similar to my proposed idea by adjusting
> the catch for Qexit (in command_loop in src/keyboard.c) rather than
> introducing a new catch for a new Qexit_read_minibuffer.

> If you have the time and energy, you can read more about this below,
> where I brainstorm two more ideas.

> The way I see it, there is, in essence, one problem: we want to quit out
> of multiple recursive-edit levels. And for this, there are three
> solutions:

[ .... ]

It turns out the tools in place were adequate to fix the problem without
too much trouble.  The critical thing is in internal_catch to distinguish
between minibuffer throws and recursive edit throws, and only throw again
for the minibuffer case.

Additionally, minibuf_c_loop_level should now be made a static function,
but I can do that later.

Could I ask you please to try out the following patch, which should be
applied on top of yesterday's patch:




--- src/eval.c~	2021-02-06 12:44:35.798096526 +0000
+++ src/eval.c	2021-02-07 12:27:09.927308860 +0000
@@ -1199,9 +1199,10 @@
 	{
           if (minibuffer_quit_level == -1)
             minibuffer_quit_level = this_minibuffer_depth (Qnil);
-          if (minibuf_level > minibuffer_quit_level
-              || command_loop_level
-              > minibuf_c_loop_level (minibuffer_quit_level))
+          /* MINIBUFFER_QUIT_LEVEL == 0 -> not in minibuffer. */
+	  if (minibuffer_quit_level
+	      && minibuf_level > minibuffer_quit_level
+	      && !NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
             Fthrow (Qexit, Qt);
 	  else
 	    minibuffer_quit_level = -1;
--- src/minibuf.c~	2021-02-06 15:04:58.878632379 +0000
+++ src/minibuf.c	2021-02-07 12:02:54.913389037 +0000
@@ -437,6 +437,8 @@
 
   if (!minibuf_depth)
     error ("Not in a minibuffer");
+  if (NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
+    error ("Not in most nested command loop");
   if (minibuf_depth < minibuf_level)
     {
       array[0] = fmt;


-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-07 12:55 ` Alan Mackenzie
@ 2021-02-07 16:44   ` jakanakaevangeli
  2021-02-07 20:26     ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: jakanakaevangeli @ 2021-02-07 16:44 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

> Just as a matter of interest, your email host rejected my post
> yesterday, saying that the interchange had to start with a STARTTLS
> command.

Thank you for reporting, I will try to get in touch with my email host.

> Could I ask you please to try out the following patch, which should be
> applied on top of yesterday's patch:

I tried it and everything works as expected, if we use C-g. The only
minor thing I noticed is with abort-recursive-edit (C-]). When executed
from a normal buffer, it does what its doc-string suggests. But when
executed from a non-innermost minibuffer, it exits multiple recursive
edits, just like abort-minibuffers. (This isn't surprising since it
calls Fthrow (Qexit, Qt) just like abort-minibuffers, only without
asking yes_or_no_p first.)

This might actually be intended behaviour, depending on how you
interpret "this minibuffer input" from abort-recursive-edit's
doc-string.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-07 16:44   ` jakanakaevangeli
@ 2021-02-07 20:26     ` Alan Mackenzie
  2021-02-07 23:48       ` [External] : " Drew Adams
  2021-02-08 12:53       ` jakanakaevangeli
  0 siblings, 2 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-02-07 20:26 UTC (permalink / raw)
  To: jakanakaevangeli; +Cc: emacs-devel

Hello jakanakaevangeli.

Thank you indeed for all the testing you are doing.  It is saving me a
lot of hassle in the future.

On Sun, Feb 07, 2021 at 17:44:17 +0100, jakanakaevangeli wrote:

[ .... ]

> > Could I ask you please to try out the following patch, which should be
> > applied on top of yesterday's patch:

> I tried it and everything works as expected, if we use C-g. The only
> minor thing I noticed is with abort-recursive-edit (C-]). When executed
> from a normal buffer, it does what its doc-string suggests. But when
> executed from a non-innermost minibuffer, it exits multiple recursive
> edits, just like abort-minibuffers. (This isn't surprising since it
> calls Fthrow (Qexit, Qt) just like abort-minibuffers, only without
> asking yes_or_no_p first.)

This is not good.

> This might actually be intended behaviour, depending on how you
> interpret "this minibuffer input" from abort-recursive-edit's
> doc-string.

I spent some time boring into the doc string and the description in the
Elisp manual.  The only conclusion I could come to was that C-] is meant
to abort EXACTLY ONE level.

So I have tweaked the C sources once more and come up with the following
patch.  It should apply to the Emacs state after yesterday's patch.
Please remove the patch from earlier today before you try to apply the
new patch.  Thanks!



--- src/eval.c~	2021-02-06 12:44:35.798096526 +0000
+++ src/eval.c	2021-02-07 20:07:13.793787769 +0000
@@ -1163,16 +1163,17 @@
    FUNC should return a Lisp_Object.
    This is how catches are done from within C code.  */
 
+/* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by
+   throwing t to tag `exit'.
+   Value -1 means there is no (throw 'exit t) in progress;
+   0 means the `throw' wasn't done from an active minibuffer;
+   N > 0 means the `throw' was done from the minibuffer at level N.  */
+EMACS_INT minibuffer_quit_level = -1;
+
 Lisp_Object
 internal_catch (Lisp_Object tag,
 		Lisp_Object (*func) (Lisp_Object), Lisp_Object arg)
 {
-  /* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by
-     throwing t to tag `exit'.
-     Value -1 means there is no (throw 'exit t) in progress;
-     0 means the `throw' wasn't done from an active minibuffer;
-     N > 0 means the `throw' was done from the minibuffer at level N.  */
-  static EMACS_INT minibuffer_quit_level = -1;
   /* This structure is made part of the chain `catchlist'.  */
   struct handler *c = push_handler (tag, CATCHER);
 
@@ -1192,16 +1193,14 @@
       Lisp_Object val = handlerlist->val;
       clobbered_eassert (handlerlist == c);
       handlerlist = handlerlist->next;
-      if (EQ (tag, Qexit) && EQ (val, Qt))
+      if (EQ (tag, Qexit) && EQ (val, Qt) && minibuffer_quit_level > 0)
 	/* If we've thrown t to tag `exit' from within a minibuffer, we
 	   exit all minibuffers more deeply nested than the current
 	   one.  */
 	{
-          if (minibuffer_quit_level == -1)
-            minibuffer_quit_level = this_minibuffer_depth (Qnil);
-          if (minibuf_level > minibuffer_quit_level
-              || command_loop_level
-              > minibuf_c_loop_level (minibuffer_quit_level))
+          /* MINIBUFFER_QUIT_LEVEL == 0 -> not in minibuffer. */
+	  if (minibuf_level > minibuffer_quit_level
+	      && !NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
             Fthrow (Qexit, Qt);
 	  else
 	    minibuffer_quit_level = -1;
--- src/minibuf.c~	2021-02-06 15:04:58.878632379 +0000
+++ src/minibuf.c	2021-02-07 20:06:34.249789948 +0000
@@ -437,12 +437,17 @@
 
   if (!minibuf_depth)
     error ("Not in a minibuffer");
+  if (NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
+    error ("Not in most nested command loop");
   if (minibuf_depth < minibuf_level)
     {
       array[0] = fmt;
       array[1] = make_fixnum (minibuf_level - minibuf_depth + 1);
       if (!NILP (Fyes_or_no_p (Fformat (2, array))))
-	Fthrow (Qexit, Qt);
+	{
+	  minibuffer_quit_level = minibuf_depth;
+	  Fthrow (Qexit, Qt);
+	}
     }
   else
     Fthrow (Qexit, Qt);
--- src/lisp.h~	2021-02-06 12:45:15.205094355 +0000
+++ src/lisp.h	2021-02-07 20:07:45.816786005 +0000
@@ -4090,6 +4090,7 @@
 }
 
 /* Defined in eval.c.  */
+extern EMACS_INT minibuffer_quit_level;
 extern Lisp_Object Vautoload_queue;
 extern Lisp_Object Vrun_hooks;
 extern Lisp_Object Vsignaling_function;


-- 
Alan Mackenzie (Nuremberg, Germany).



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

* RE: [External] : Re: Stop frames stealing eachothers' minibuffers!
  2021-02-07 20:26     ` Alan Mackenzie
@ 2021-02-07 23:48       ` Drew Adams
  2021-03-13 18:31         ` Alan Mackenzie
  2021-02-08 12:53       ` jakanakaevangeli
  1 sibling, 1 reply; 52+ messages in thread
From: Drew Adams @ 2021-02-07 23:48 UTC (permalink / raw)
  To: Alan Mackenzie, jakanakaevangeli; +Cc: emacs-devel@gnu.org

> I spent some time boring into the doc string and the description in the
> Elisp manual.  The only conclusion I could come to was that C-] is
> meant to abort EXACTLY ONE level.

Apologies for chiming in without following
the thread.

I just want to say that my use of recursive
minibuffers depends on being able to use a
key (preferably `C-]', as always previously)
to abort a single level (i.e., exactly one).
___

I also have a command, `icicle-top-level',
bound to `C-M-T' (aka `C-M-S-t'), which aborts
to the top-level whenever possible.  Doc string:

 Remove `*Completions*' window.  Return to top level.
 Try to throw to `icicle-top-level' (`catch' is for multi-commands).
 If that fails, call `top-level'.




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-07 20:26     ` Alan Mackenzie
  2021-02-07 23:48       ` [External] : " Drew Adams
@ 2021-02-08 12:53       ` jakanakaevangeli
  2021-02-11 11:44         ` Alan Mackenzie
  1 sibling, 1 reply; 52+ messages in thread
From: jakanakaevangeli @ 2021-02-08 12:53 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello jakanakaevangeli.

Hello

> I spent some time boring into the doc string and the description in the
> Elisp manual.  The only conclusion I could come to was that C-] is meant
> to abort EXACTLY ONE level.
>
> So I have tweaked the C sources once more and come up with the following
> patch.  It should apply to the Emacs state after yesterday's patch.
> Please remove the patch from earlier today before you try to apply the
> new patch.  Thanks!

During testing, I didn't encounter any problems related to your latest
patch so you hard work paid off.

Sadly, I did encounter 2 additional minibuffer issues which aren't
related to your latest two patches, that is, they are present regardless
if these patches are applied or not. I'm posting them here since they
are still possibly related to the new minibuffer frame-following
functionality.

1) With minibuffer-follows-selected-frame set to t (the default value):
- press M-x on frame A
- select frame B (the minibuffer will move to this frame)
- C-x o, to select the minibuffer
- C-g to quit it
Miniwindow stays selected and its buffer is *Minibuf-1* instead of
*Minibuf-0*. You can check this by evaluating (minibuffer-window).

2) With minibuffer-follows-selected-frame set to nil:
- (setq set enable-recursive-minibuffers t)
- (minibuffer-depth-indicate-mode 1)
- select frame A and press M-x
- select frame B and press M-x
- select frame A and close it
- select frame B and quit its minibuffer with C-g.
This doesn't quit the outer minibuffer, as expected, but this minibuffer
isn't shown anywhere and the only reasonable way to quit it is with C-],
which, I believe, a lot of users don't know about (at least I personally
didn't until quite recently). You can check this with
(minibuffer-depth).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-08 12:53       ` jakanakaevangeli
@ 2021-02-11 11:44         ` Alan Mackenzie
  2021-02-11 14:29           ` Stefan Monnier
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-02-11 11:44 UTC (permalink / raw)
  To: jakanakaevangeli; +Cc: emacs-devel

Hello, jakanakaevangeli.

On Mon, Feb 08, 2021 at 13:53:43 +0100, jakanakaevangeli wrote:
> Alan Mackenzie <acm@muc.de> writes:

> Hello

> > I spent some time boring into the doc string and the description in the
> > Elisp manual.  The only conclusion I could come to was that C-] is meant
> > to abort EXACTLY ONE level.

> > So I have tweaked the C sources once more and come up with the following
> > patch.  It should apply to the Emacs state after yesterday's patch.
> > Please remove the patch from earlier today before you try to apply the
> > new patch.  Thanks!

> During testing, I didn't encounter any problems related to your latest
> patch so you hard work paid off.

> Sadly, I did encounter 2 additional minibuffer issues which aren't
> related to your latest two patches, that is, they are present regardless
> if these patches are applied or not. I'm posting them here since they
> are still possibly related to the new minibuffer frame-following
> functionality.

Thanks!

> 1) With minibuffer-follows-selected-frame set to t (the default value):
> - press M-x on frame A
> - select frame B (the minibuffer will move to this frame)
> - C-x o, to select the minibuffer
> - C-g to quit it
> Miniwindow stays selected and its buffer is *Minibuf-1* instead of
> *Minibuf-0*. You can check this by evaluating (minibuffer-window).

This one was somewhat tricky to resolve, my fixes uncovering several
other inconsistencies in the minibuffer handling, which I hope I have
now fixed.

> 2) With minibuffer-follows-selected-frame set to nil:
> - (setq set enable-recursive-minibuffers t)
> - (minibuffer-depth-indicate-mode 1)
> - select frame A and press M-x
> - select frame B and press M-x
> - select frame A and close it
> - select frame B and quit its minibuffer with C-g.
> This doesn't quit the outer minibuffer, as expected, but this minibuffer
> isn't shown anywhere and the only reasonable way to quit it is with C-],
> which, I believe, a lot of users don't know about (at least I personally
> didn't until quite recently). You can check this with
> (minibuffer-depth).

This is more involved.  What do we want to happen when a frame with open
minibuffers is deleted?  I would say that these minibuffers should,
except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
together with any others in other frames whose nesting level makes this
necessary.  Then there're complications with recursive edits.  But I
haven't looked at implementing this, yet.  Alternatively, maybe open
minibuffers should be moved to the "previous" frame.  Perhaps I should
open another thread on emacs-devel for this problem.

Anyhow, I have a patch for bug 1.  It is a full diff from master, rather
than a difference from "yesterday".  I would be very grateful indeed if
you could try it out, and let me know again how well it's working.  I
would really like to commit this to master.  Thanks again!



diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index a899a943d4..aacb8ab00b 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2116,18 +2116,19 @@ minibuffer-hide-completions
 (defun exit-minibuffer ()
   "Terminate this minibuffer argument."
   (interactive)
-  (when (or
-         (innermost-minibuffer-p)
-         (not (minibufferp)))
+  (when (minibufferp)
+    (when (not (minibuffer-innermost-command-loop-p))
+      (error "%s" "Not in most nested command loop"))
+    (when (not (innermost-minibuffer-p))
+      (error "%s" "Not in most nested minibuffer")))
   ;; If the command that uses this has made modifications in the minibuffer,
   ;; we don't want them to cause deactivation of the mark in the original
   ;; buffer.
   ;; A better solution would be to make deactivate-mark buffer-local
   ;; (or to turn it into a list of buffers, ...), but in the mean time,
   ;; this should do the trick in most cases.
-    (setq deactivate-mark nil)
-    (throw 'exit nil))
-  (error "%s" "Not in most nested minibuffer"))
+  (setq deactivate-mark nil)
+  (throw 'exit nil))
 
 (defun self-insert-and-exit ()
   "Terminate minibuffer input."
diff --git a/src/eval.c b/src/eval.c
index 3aff3b56d5..91fc4e6837 100644
--- a/src/eval.c
+++ b/src/eval.c
@@ -1165,21 +1165,23 @@ usage: (catch TAG BODY...)  */)
    FUNC should return a Lisp_Object.
    This is how catches are done from within C code.  */
 
+/* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by
+   throwing t to tag `exit'.
+   0 means there is no (throw 'exit t) in progress, or it wasn't from
+     a minibuffer which isn't the most nested;
+   N > 0 means the `throw' was done from the minibuffer at level N which
+     wasn't the most nested.  */
+EMACS_INT minibuffer_quit_level = 0;
+
 Lisp_Object
 internal_catch (Lisp_Object tag,
 		Lisp_Object (*func) (Lisp_Object), Lisp_Object arg)
 {
-  /* MINIBUFFER_QUIT_LEVEL is to handle quitting from nested minibuffers by
-     throwing t to tag `exit'.
-     Value -1 means there is no (throw 'exit t) in progress;
-     0 means the `throw' wasn't done from an active minibuffer;
-     N > 0 means the `throw' was done from the minibuffer at level N.  */
-  static EMACS_INT minibuffer_quit_level = -1;
   /* This structure is made part of the chain `catchlist'.  */
   struct handler *c = push_handler (tag, CATCHER);
 
   if (EQ (tag, Qexit))
-    minibuffer_quit_level = -1;
+    minibuffer_quit_level = 0;
 
   /* Call FUNC.  */
   if (! sys_setjmp (c->jmp))
@@ -1194,22 +1196,16 @@ internal_catch (Lisp_Object tag,
       Lisp_Object val = handlerlist->val;
       clobbered_eassert (handlerlist == c);
       handlerlist = handlerlist->next;
-      if (EQ (tag, Qexit) && EQ (val, Qt))
+      if (EQ (tag, Qexit) && EQ (val, Qt) && minibuffer_quit_level > 0)
 	/* If we've thrown t to tag `exit' from within a minibuffer, we
 	   exit all minibuffers more deeply nested than the current
 	   one.  */
 	{
-	  EMACS_INT mini_depth = this_minibuffer_depth (Qnil);
-	  if (mini_depth && mini_depth != minibuffer_quit_level)
-	    {
-	      if (minibuffer_quit_level == -1)
-		minibuffer_quit_level = mini_depth;
-	      if (minibuffer_quit_level
-		  && (minibuf_level > minibuffer_quit_level))
-		Fthrow (Qexit, Qt);
-	    }
+	  if (minibuf_level > minibuffer_quit_level
+	      && !NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
+            Fthrow (Qexit, Qt);
 	  else
-	    minibuffer_quit_level = -1;
+	    minibuffer_quit_level = 0;
 	}
       return val;
     }
diff --git a/src/lisp.h b/src/lisp.h
index 409a1e7060..0847324d1f 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4091,6 +4091,7 @@ intern_c_string (const char *str)
 }
 
 /* Defined in eval.c.  */
+extern EMACS_INT minibuffer_quit_level;
 extern Lisp_Object Vautoload_queue;
 extern Lisp_Object Vrun_hooks;
 extern Lisp_Object Vsignaling_function;
@@ -4369,6 +4370,7 @@ extern void syms_of_casetab (void);
 
 /* Defined in keyboard.c.  */
 
+extern EMACS_INT command_loop_level;
 extern Lisp_Object echo_message_buffer;
 extern struct kboard *echo_kboard;
 extern void cancel_echoing (void);
diff --git a/src/minibuf.c b/src/minibuf.c
index 949c3d989d..4b1f4b1ff7 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -41,6 +41,7 @@ along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.  */
    minibuffer recursions are encountered.  */
 
 Lisp_Object Vminibuffer_list;
+Lisp_Object Vcommand_loop_level_list;
 
 /* Data to remember during recursive minibuffer invocations.  */
 
@@ -64,6 +65,8 @@ static Lisp_Object minibuf_prompt;
 static ptrdiff_t minibuf_prompt_width;
 
 static Lisp_Object nth_minibuffer (EMACS_INT depth);
+static EMACS_INT minibuf_c_loop_level (EMACS_INT depth);
+static void set_minibuffer_mode (Lisp_Object buf, EMACS_INT depth);
 
 \f
 /* Return TRUE when a frame switch causes a minibuffer on the old
@@ -181,7 +184,12 @@ void move_minibuffer_onto_frame (void)
 	set_window_buffer (sf->minibuffer_window, nth_minibuffer (i), 0, 0);
       minibuf_window = sf->minibuffer_window;
       if (of != sf)
-	set_window_buffer (of->minibuffer_window, get_minibuffer (0), 0, 0);
+	{
+	  Lisp_Object temp = get_minibuffer (0);
+
+	  set_window_buffer (of->minibuffer_window, temp, 0, 0);
+	  set_minibuffer_mode (temp, 0);
+	}
     }
 }
 
@@ -389,6 +397,21 @@ No argument or nil as argument means use the current buffer as BUFFER.  */)
     : Qnil;
 }
 
+DEFUN ("minibuffer-innermost-command-loop-p", Fminibuffer_innermost_command_loop_p,
+       Sminibuffer_innermost_command_loop_p, 0, 1, 0,
+       doc: /* Return t if BUFFER is a minibuffer at the current command loop level.
+No argument or nil as argument means use the current buffer as BUFFER.  */)
+  (Lisp_Object buffer)
+{
+  EMACS_INT depth;
+  if (NILP (buffer))
+    buffer = Fcurrent_buffer ();
+  depth = this_minibuffer_depth (buffer);
+  return depth && minibuf_c_loop_level (depth) == command_loop_level
+    ? Qt
+    : Qnil;
+}
+
 /* Return the nesting depth of the active minibuffer BUFFER, or 0 if
    BUFFER isn't such a thing.  If BUFFER is nil, this means use the current
    buffer.  */
@@ -420,12 +443,17 @@ confirm the aborting of the current minibuffer and all contained ones.  */)
 
   if (!minibuf_depth)
     error ("Not in a minibuffer");
+  if (NILP (Fminibuffer_innermost_command_loop_p (Qnil)))
+    error ("Not in most nested command loop");
   if (minibuf_depth < minibuf_level)
     {
       array[0] = fmt;
       array[1] = make_fixnum (minibuf_level - minibuf_depth + 1);
       if (!NILP (Fyes_or_no_p (Fformat (2, array))))
-	Fthrow (Qexit, Qt);
+	{
+	  minibuffer_quit_level = minibuf_depth;
+	  Fthrow (Qexit, Qt);
+	}
     }
   else
     Fthrow (Qexit, Qt);
@@ -508,6 +536,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   ptrdiff_t count = SPECPDL_INDEX ();
   Lisp_Object mini_frame, ambient_dir, minibuffer, input_method;
   Lisp_Object calling_frame = selected_frame;
+  Lisp_Object calling_window = selected_window;
   Lisp_Object enable_multibyte;
   EMACS_INT pos = 0;
   /* String to add to the history.  */
@@ -598,7 +627,8 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   if (minibuf_level > 1
       && minibuf_moves_frame_when_opened ()
-      && !minibuf_follows_frame ())
+      && (!minibuf_follows_frame ()
+	  || (!EQ (mini_frame, selected_frame))))
     {
       EMACS_INT i;
 
@@ -607,8 +637,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 	set_window_buffer (minibuf_window, nth_minibuffer (i), 0, 0);
     }
 
-  record_unwind_protect_void (choose_minibuf_frame);
-
   record_unwind_protect (restore_window_configuration,
                          Fcons (Qt, Fcurrent_window_configuration (Qnil)));
 
@@ -640,7 +668,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   minibuf_save_list
     = Fcons (Voverriding_local_map,
 	     Fcons (minibuf_window,
-		    minibuf_save_list));
+		    Fcons (calling_frame,
+			   Fcons (calling_window,
+				  minibuf_save_list))));
   minibuf_save_list
     = Fcons (minibuf_prompt,
 	     Fcons (make_fixnum (minibuf_prompt_width),
@@ -694,6 +724,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   /* Switch to the minibuffer.  */
 
   minibuffer = get_minibuffer (minibuf_level);
+  set_minibuffer_mode (minibuffer, minibuf_level);
   Fset_buffer (minibuffer);
 
   /* Defeat (setq-default truncate-lines t), since truncated lines do
@@ -738,6 +769,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
      where there is an active minibuffer.
      Set them to point to ` *Minibuf-0*', which is always empty.  */
   empty_minibuf = get_minibuffer (0);
+  set_minibuffer_mode (empty_minibuf, 0);
 
   FOR_EACH_FRAME (dummy, frame)
     {
@@ -837,20 +869,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
 
   recursive_edit_1 ();
 
-  /* We've exited the recursive edit without an error, so switch the
-     current window away from the expired minibuffer window.  */
-  {
-    Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
-    /* PREV can be on a different frame when we have a minibuffer only
-       frame, the other frame's minibuffer window is MINIBUF_WINDOW,
-       and its "focus window" is also MINIBUF_WINDOW.  */
-    while (!EQ (prev, minibuf_window)
-	   && !EQ (selected_frame, WINDOW_FRAME (XWINDOW (prev))))
-      prev = Fprevious_window (prev, Qnil, Qnil);
-    if (!EQ (prev, minibuf_window))
-      Fset_frame_selected_window (selected_frame, prev, Qnil);
-  }
-
   /* If cursor is on the minibuffer line,
      show the user we have exited by putting it in column 0.  */
   if (XWINDOW (minibuf_window)->cursor.vpos >= 0
@@ -959,11 +977,16 @@ Lisp_Object
 get_minibuffer (EMACS_INT depth)
 {
   Lisp_Object tail = Fnthcdr (make_fixnum (depth), Vminibuffer_list);
+  Lisp_Object cll_tail = Fnthcdr (make_fixnum (depth),
+				  Vcommand_loop_level_list);
   if (NILP (tail))
     {
       tail = list1 (Qnil);
       Vminibuffer_list = nconc2 (Vminibuffer_list, tail);
+      cll_tail = list1 (Qnil);
+      Vcommand_loop_level_list = nconc2 (Vcommand_loop_level_list, cll_tail);
     }
+  XSETCAR (cll_tail, make_fixnum (depth ? command_loop_level : 0));
   Lisp_Object buf = Fcar (tail);
   if (NILP (buf) || !BUFFER_LIVE_P (XBUFFER (buf)))
     {
@@ -973,7 +996,6 @@ get_minibuffer (EMACS_INT depth)
       buf = Fget_buffer_create (lname, Qnil);
       /* Do this before set_minibuffer_mode.  */
       XSETCAR (tail, buf);
-      set_minibuffer_mode (buf, depth);
       /* Although the buffer's name starts with a space, undo should be
 	 enabled in it.  */
       Fbuffer_enable_undo (buf);
@@ -985,12 +1007,19 @@ get_minibuffer (EMACS_INT depth)
 	 while the buffer doesn't know about them any more.  */
       delete_all_overlays (XBUFFER (buf));
       reset_buffer (XBUFFER (buf));
-      set_minibuffer_mode (buf, depth);
     }
 
   return buf;
 }
 
+static EMACS_INT minibuf_c_loop_level (EMACS_INT depth)
+{
+  Lisp_Object cll = Fnth (make_fixnum (depth), Vcommand_loop_level_list);
+  if (FIXNUMP (cll))
+    return XFIXNUM (cll);
+  return 0;
+}
+
 static void
 run_exit_minibuf_hook (void)
 {
@@ -1004,17 +1033,16 @@ static void
 read_minibuf_unwind (void)
 {
   Lisp_Object old_deactivate_mark;
-  Lisp_Object window;
+  Lisp_Object calling_frame;
+  Lisp_Object calling_window;
   Lisp_Object future_mini_window;
 
-  /* If this was a recursive minibuffer,
-     tie the minibuffer window back to the outer level minibuffer buffer.  */
-  minibuf_level--;
-
-  window = minibuf_window;
   /* To keep things predictable, in case it matters, let's be in the
-     minibuffer when we reset the relevant variables.  */
-  Fset_buffer (XWINDOW (window)->contents);
+     minibuffer when we reset the relevant variables.  Don't depend on
+     `minibuf_window' here.  This could by now be the mini-window of any
+     frame.  */
+  Fset_buffer (nth_minibuffer (minibuf_level));
+  minibuf_level--;
 
   /* Restore prompt, etc, from outer minibuffer level.  */
   Lisp_Object key_vec = Fcar (minibuf_save_list);
@@ -1042,6 +1070,10 @@ read_minibuf_unwind (void)
 #endif
   future_mini_window = Fcar (minibuf_save_list);
   minibuf_save_list = Fcdr (minibuf_save_list);
+  calling_frame = Fcar (minibuf_save_list);
+  minibuf_save_list = Fcdr (minibuf_save_list);
+  calling_window = Fcar (minibuf_save_list);
+  minibuf_save_list = Fcdr (minibuf_save_list);
 
   /* Erase the minibuffer we were using at this level.  */
   {
@@ -1059,7 +1091,7 @@ read_minibuf_unwind (void)
      mini-window back to its normal size.  */
   if (minibuf_level == 0
       || !EQ (selected_frame, WINDOW_FRAME (XWINDOW (future_mini_window))))
-    resize_mini_window (XWINDOW (window), 0);
+    resize_mini_window (XWINDOW (minibuf_window), 0);
 
   /* Deal with frames that should be removed when exiting the
      minibuffer.  */
@@ -1090,6 +1122,24 @@ read_minibuf_unwind (void)
      to make sure we don't leave around bindings and stuff which only
      made sense during the read_minibuf invocation.  */
   call0 (intern ("minibuffer-inactive-mode"));
+
+  /* We've exited the recursive edit, so switch the current windows
+     away from the expired minibuffer window, both in the current
+     minibuffer's frame and the original calling frame.  */
+  choose_minibuf_frame ();
+  if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
+  {
+    Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
+    /* PREV can be on a different frame when we have a minibuffer only
+       frame, the other frame's minibuffer window is MINIBUF_WINDOW,
+       and its "focus window" is also MINIBUF_WINDOW.  */
+    if (!EQ (prev, minibuf_window)
+	&& EQ (WINDOW_FRAME (XWINDOW (prev)),
+	       WINDOW_FRAME (XWINDOW (minibuf_window))))
+      Fset_frame_selected_window (selected_frame, prev, Qnil);
+  }
+  else
+    Fset_frame_selected_window (calling_frame, calling_window, Qnil);
 }
 \f
 
@@ -2137,6 +2187,7 @@ void
 init_minibuf_once (void)
 {
   staticpro (&Vminibuffer_list);
+  staticpro (&Vcommand_loop_level_list);
   pdumper_do_now_and_after_load (init_minibuf_once_for_pdumper);
 }
 
@@ -2150,6 +2201,7 @@ init_minibuf_once_for_pdumper (void)
      restore from a dump file.  pdumper doesn't try to preserve
      frames, windows, and so on, so reset everything related here.  */
   Vminibuffer_list = Qnil;
+  Vcommand_loop_level_list = Qnil;
   minibuf_level = 0;
   minibuf_prompt = Qnil;
   minibuf_save_list = Qnil;
@@ -2380,6 +2432,7 @@ instead. */);
 
   defsubr (&Sminibufferp);
   defsubr (&Sinnermost_minibuffer_p);
+  defsubr (&Sminibuffer_innermost_command_loop_p);
   defsubr (&Sabort_minibuffers);
   defsubr (&Sminibuffer_prompt_end);
   defsubr (&Sminibuffer_contents);
diff --git a/src/window.h b/src/window.h
index 79eb44e7a3..b6f88e8f55 100644
--- a/src/window.h
+++ b/src/window.h
@@ -1120,10 +1120,6 @@ void set_window_buffer (Lisp_Object window, Lisp_Object buffer,
 
 extern Lisp_Object echo_area_window;
 
-/* Depth in recursive edits.  */
-
-extern EMACS_INT command_loop_level;
-
 /* Non-zero if we should redraw the mode lines on the next redisplay.
    Usually set to a unique small integer so we can track the main causes of
    full redisplays in `redisplay--mode-lines-cause'.  */



-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-11 11:44         ` Alan Mackenzie
@ 2021-02-11 14:29           ` Stefan Monnier
  2021-02-12  9:48             ` Alan Mackenzie
  2021-03-13 18:23             ` Alan Mackenzie
  0 siblings, 2 replies; 52+ messages in thread
From: Stefan Monnier @ 2021-02-11 14:29 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

> This is more involved.  What do we want to happen when a frame with open
> minibuffers is deleted?  I would say that these minibuffers should,
> except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
> together with any others in other frames whose nesting level makes this
> necessary.

I vote for moving those minibuffers elsewhere (anywhere else is fine by
me, really).  I assume it's no more complicated code-wise, and it should
suffer less from the risk of losing information.


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-11 14:29           ` Stefan Monnier
@ 2021-02-12  9:48             ` Alan Mackenzie
  2021-03-13 18:23             ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-02-12  9:48 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: jakanakaevangeli, emacs-devel

Hello, Stefan.

Thanks for still reading this old thread!

On Thu, Feb 11, 2021 at 09:29:44 -0500, Stefan Monnier wrote:
> > This is more involved.  What do we want to happen when a frame with open
> > minibuffers is deleted?  I would say that these minibuffers should,
> > except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
> > together with any others in other frames whose nesting level makes this
> > necessary.

> I vote for moving those minibuffers elsewhere (anywhere else is fine by
> me, really).  I assume it's no more complicated code-wise, and it should
> suffer less from the risk of losing information.

Thinking about it, you are right.  Deleting frames doesn't abort any
other buffers, so why should minibuffers suffer?  The code is going to
be tedious and moderately difficult to write either way.  So I think on
deleting a frame, any minibuffers in it should get pushed onto the
previous frame.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-02-11 14:29           ` Stefan Monnier
  2021-02-12  9:48             ` Alan Mackenzie
@ 2021-03-13 18:23             ` Alan Mackenzie
  2021-03-13 19:39               ` Stefan Monnier
  2021-03-13 20:53               ` jakanakaevangeli
  1 sibling, 2 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-13 18:23 UTC (permalink / raw)
  To: Stefan Monnier, jakanakaevangeli; +Cc: emacs-devel

Hello, Stefan and jakanakaevangeli.

On Thu, Feb 11, 2021 at 09:29:44 -0500, Stefan Monnier wrote:
> > This is more involved.  What do we want to happen when a frame with open
> > minibuffers is deleted?  I would say that these minibuffers should,
> > except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
> > together with any others in other frames whose nesting level makes this
> > necessary.

> I vote for moving those minibuffers elsewhere (anywhere else is fine by
> me, really).  I assume it's no more complicated code-wise, and it should
> suffer less from the risk of losing information.

Just a quick recapitulation of the problem: when
minibuffer-follows-selected-frame is nil, and
enable-recursive-minibuffers t, it is possible to have several open
minibuffers on several frames.  If one of these frames is deleted, its
minibuffers continue to exist, but are wholly inaccessible - the only
thing to do with them is to abort them, e.g. with C-].  This is
suboptimal.

The patch below tries to solve this by, when such a frame gets deleted,
"zipping" its minibuffers into those of another frame.  The idea behind
the patch is to use the mini-window's w->prev_buffers component to hold
the list of its minibuffers.  This mini-window component was unused by
the rest of Emacs, apart from accidentally by
window--before-delete-windows, which I have now amended.

I would be grateful indeed if either of you (or indeed, anybody else),
would apply the patch and try it out, or indeed comment on it.  Thanks!



diff --git a/doc/emacs/mini.texi b/doc/emacs/mini.texi
index f81e64bdf9..7da0a48b7c 100644
--- a/doc/emacs/mini.texi
+++ b/doc/emacs/mini.texi
@@ -82,7 +82,9 @@ Basic Minibuffer
 (@pxref{Recursive Mini,,, elisp}).  This option is mainly to retain
 (approximately) the behavior prior to Emacs 28.1.  Note that the
 effect of the command, when you finally finish using the minibuffer,
-always takes place in the frame where you first opened it.
+always takes place in the frame where you first opened it.  The sole
+exception is that when that frame no longer exists, the action takes
+place in the currently selected frame.
 
 @node Minibuffer File
 @section Minibuffers for File Names
diff --git a/lisp/window.el b/lisp/window.el
index cfd9876ed0..fb2ea4a985 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4158,7 +4158,7 @@ window--before-delete-windows
 
 This function is called only if `switch-to-buffer-preserve-window-point'
 evaluates non-nil."
-  (dolist (win (window-list))
+  (dolist (win (window-list nil 'no-minibuf))
     (let* ((buf   (window-buffer (or window win)))
            (start (window-start win))
            (pos   (window-point win))
@@ -4416,7 +4416,8 @@ record-window-buffer
        window (assq-delete-all buffer (window-prev-buffers window))))
 
     ;; Don't record insignificant buffers.
-    (unless (eq (aref (buffer-name buffer) 0) ?\s)
+    (when (or (not (eq (aref (buffer-name buffer) 0) ?\s))
+              (string-match "^ \\*Minibuf" (buffer-name buffer)))
       ;; Add an entry for buffer to WINDOW's previous buffers.
       (with-current-buffer buffer
 	(let ((start (window-start window))
diff --git a/src/frame.c b/src/frame.c
index a62347c1fb..b9df5739dd 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1487,7 +1487,7 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
 #endif
     internal_last_event_frame = Qnil;
 
-  move_minibuffer_onto_frame ();
+  move_minibuffers_onto_frame (sf, for_deletion);
   return frame;
 }
 
diff --git a/src/lisp.h b/src/lisp.h
index b95f389b89..2f4e6377cb 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4348,7 +4348,7 @@ extern void clear_regexp_cache (void);
 
 extern Lisp_Object Vminibuffer_list;
 extern Lisp_Object last_minibuf_string;
-extern void move_minibuffer_onto_frame (void);
+extern void move_minibuffers_onto_frame (struct frame *, int);
 extern bool is_minibuffer (EMACS_INT, Lisp_Object);
 extern EMACS_INT this_minibuffer_depth (Lisp_Object);
 extern EMACS_INT minibuf_level;
diff --git a/src/minibuf.c b/src/minibuf.c
index 4b1f4b1ff7..53ed8d5ef8 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -67,6 +67,7 @@ static ptrdiff_t minibuf_prompt_width;
 static Lisp_Object nth_minibuffer (EMACS_INT depth);
 static EMACS_INT minibuf_c_loop_level (EMACS_INT depth);
 static void set_minibuffer_mode (Lisp_Object buf, EMACS_INT depth);
+static bool live_minibuffer_p (Lisp_Object);
 
 \f
 /* Return TRUE when a frame switch causes a minibuffer on the old
@@ -78,6 +79,7 @@ minibuf_follows_frame (void)
              Qt);
 }
 
+#if 0
 /* Return TRUE when a minibuffer always remains on the frame where it
    was first invoked. */
 static bool
@@ -85,6 +87,7 @@ minibuf_stays_put (void)
 {
   return NILP (Fdefault_toplevel_value (Qminibuffer_follows_selected_frame));
 }
+#endif
 
 /* Return TRUE when opening a (recursive) minibuffer causes
    minibuffers on other frames to move to the selected frame.  */
@@ -112,83 +115,137 @@ choose_minibuf_frame (void)
 	emacs_abort ();
 
       minibuf_window = sf->minibuffer_window;
-      /* If we've still got another minibuffer open, use its mini-window
-         instead.  */
-      if (minibuf_level > 1 && minibuf_stays_put ())
-        {
-          Lisp_Object buffer = get_minibuffer (minibuf_level);
-          Lisp_Object tail, frame;
-
-          FOR_EACH_FRAME (tail, frame)
-            if (EQ (XWINDOW (XFRAME (frame)->minibuffer_window)->contents,
-                    buffer))
-              {
-                minibuf_window = XFRAME (frame)->minibuffer_window;
-                break;
-              }
-        }
     }
+}
 
-  if (minibuf_moves_frame_when_opened ()
-      && FRAMEP (selected_frame)
-      && FRAME_LIVE_P (XFRAME (selected_frame)))
-    /* Make sure no other frame has a minibuffer as its selected window,
-       because the text would not be displayed in it, and that would be
-       confusing.  Only allow the selected frame to do this,
-       and that only if the minibuffer is active.  */
-  {
-    Lisp_Object tail, frame;
-    struct frame *of;
-
-    FOR_EACH_FRAME (tail, frame)
-      if (!EQ (frame, selected_frame)
-          && minibuf_level > 1
-	  /* The frame's minibuffer can be on a different frame.  */
-	  && ! EQ (XWINDOW ((of = XFRAME (frame))->minibuffer_window)->frame,
-		   selected_frame))
-        {
-          if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-            Fset_frame_selected_window (frame, Fframe_first_window (frame),
-                                        Qnil);
-
-          if (!EQ (XWINDOW (of->minibuffer_window)->contents,
-                   nth_minibuffer (0)))
-            set_window_buffer (of->minibuffer_window,
-                               nth_minibuffer (0), 0, 0);
-        }
-  }
+/* Get the next live buffer entry from a w->prev_buffer, setting the pertinent
+  variables of `zip_minibuffer_stacks'.  The parameter P is either "d" for
+  the destination structures, or "s" for the source structures.  When the
+  ->prev_buffers list is exhausted, set di/si to -1.  */
+#define NEXT_BUFFER_ENTRY(p)						\
+  do									\
+    {									\
+      if (NILP (p##_bufs))						\
+        {								\
+	  p##b = Qnil;							\
+	  p##_ent = Qnil;						\
+	}								\
+      else								\
+	{								\
+	  p##_ent = Fcar (p##_bufs);					\
+	  p##b = Fcar (p##_ent);					\
+	  p##_bufs = Fcdr (p##_bufs);					\
+	}								\
+      if (!NILP (p##b))							\
+	p##i = this_minibuffer_depth (p##b);				\
+      else								\
+	p##i = -1;							\
+    } while (p##i == 0)
+
+/* Move the ordered "stack" of minibuffers from SOURCE_WINDOW to
+   DEST_WINDOW, interleaving those minibuffers with any in DEST_WINDOW
+   to produce an ordered combination.  The ordering is by minibuffer
+   depth.  A stack of minibuffers consists of the minibuffer currently
+   in DEST/SOURCE_WINDOW together with any recorded in the
+   ->prev_buffers field of the struct window.  */
+static void
+zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
+{
+  struct window *dw = XWINDOW (dest_window);
+  struct window *sw = XWINDOW (source_window);
+  Lisp_Object d_bufs, s_bufs;	/* Lists of minibuffer entries */
+  Lisp_Object acc = Qnil;
+  Lisp_Object d_ent, s_ent;	/* Entries from dw/sw->prev_buffers */
+  Lisp_Object db, sb;		/* (Mini)buffers from the above */
+  EMACS_INT di, si;  /* Indices in the minibuffer list of db and sb */
+
+  if (!live_minibuffer_p (dw->contents)
+      && NILP (dw->prev_buffers))
+    {
+      set_window_buffer (dest_window, sw->contents, 0, 0);
+      Fset_window_start (dest_window, Fwindow_start (source_window), Qnil);
+      Fset_window_point (dest_window, Fwindow_point (source_window));
+      dw->prev_buffers = sw->prev_buffers;
+      set_window_buffer (source_window, get_minibuffer (0), 0, 0);
+      sw->prev_buffers = Qnil;
+      return;
+    }
+
+  if (live_minibuffer_p (dw->contents))
+    call1 (Qrecord_window_buffer, dest_window);
+  if (live_minibuffer_p (sw->contents))
+    call1 (Qrecord_window_buffer, source_window);
+
+  d_bufs = Fnreverse (dw->prev_buffers);
+  s_bufs = Fnreverse (sw->prev_buffers);
+
+  NEXT_BUFFER_ENTRY (d);
+  NEXT_BUFFER_ENTRY (s);
+
+  while (di != -1 && si != -1)
+    if (di < si)
+      {
+	acc = Fcons (d_ent, acc);
+	NEXT_BUFFER_ENTRY (d);
+      }
+    else
+      {
+	acc = Fcons (s_ent, acc);
+	NEXT_BUFFER_ENTRY (s);
+      }
+  while (di != -1)
+    {
+      acc = Fcons (d_ent, acc);
+      NEXT_BUFFER_ENTRY (d);
+    }
+  while (si != -1)
+    {
+      acc = Fcons (s_ent, acc);
+      NEXT_BUFFER_ENTRY (s);
+    }
+  if (!NILP (acc))
+    {
+      d_ent = Fcar (acc);
+      acc = Fcdr (acc);
+      set_window_buffer (dest_window, Fcar (d_ent), 0, 0);
+      Fset_window_start (dest_window, Fcar (Fcdr (d_ent)), Qnil);
+      Fset_window_point (dest_window, Fcar (Fcdr (Fcdr (d_ent))));
+    }
+  dw->prev_buffers = acc;
+  sw->prev_buffers = Qnil;
+  set_window_buffer (source_window, get_minibuffer (0), 0, 0);
 }
+#undef NEXT_BUFFER_ENTRY
 
-/* If `minibuffer_follows_selected_frame' is t and we have a
-   minibuffer, move it from its current frame to the selected frame.
-   This function is intended to be called from `do_switch_frame' in
-   frame.c.  */
-void move_minibuffer_onto_frame (void)
+/* If `minibuffer_follows_selected_frame' is t, or we're about to
+   delete a frame which potentially "contains" minibuffers, move them
+   from the old frame to the selected frame.  This function is
+   intended to be called from `do_switch_frame' in frame.c.  OF is the
+   old frame, FOR_DELETION is self explanatory.  */
+void
+move_minibuffers_onto_frame (struct frame *of, int for_deletion)
 {
-  if (!minibuf_level)
-    return;
-  if (!minibuf_follows_frame ())
+  if (!for_deletion && (!minibuf_level || !minibuf_follows_frame ()))
     return;
   if (FRAMEP (selected_frame)
       && FRAME_LIVE_P (XFRAME (selected_frame))
-      && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
+      && (for_deletion
+	  || !EQ (minibuf_window,
+		  XFRAME (selected_frame)->minibuffer_window)))
     {
-      EMACS_INT i;
       struct frame *sf = XFRAME (selected_frame);
-      Lisp_Object old_frame = XWINDOW (minibuf_window)->frame;
-      struct frame *of = XFRAME (old_frame);
-
-      /* Stack up all the (recursively) open minibuffers on the selected
-         mini_window.  */
-      for (i = 1; i <= minibuf_level; i++)
-	set_window_buffer (sf->minibuffer_window, nth_minibuffer (i), 0, 0);
-      minibuf_window = sf->minibuffer_window;
-      if (of != sf)
+      Lisp_Object mini_frame = XWINDOW (minibuf_window)->frame;
+      struct frame *mf = XFRAME (mini_frame);
+      if (minibuf_follows_frame () || for_deletion)
+	zip_minibuffer_stacks (sf->minibuffer_window,
+			       of->minibuffer_window);
+      if (minibuf_window != sf->minibuffer_window)
 	{
 	  Lisp_Object temp = get_minibuffer (0);
 
-	  set_window_buffer (of->minibuffer_window, temp, 0, 0);
+	  set_window_buffer (mf->minibuffer_window, temp, 0, 0);
 	  set_minibuffer_mode (temp, 0);
+	  minibuf_window = sf->minibuffer_window;
 	}
     }
 }
@@ -221,6 +278,7 @@ without invoking the usual minibuffer commands.  */)
 /* Actual minibuffer invocation.  */
 
 static void read_minibuf_unwind (void);
+static void minibuffer_unwind (void);
 static void run_exit_minibuf_hook (void);
 
 
@@ -544,7 +602,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
-  Lisp_Object dummy, frame;
+  Lisp_Object old_minibuf_window = minibuf_window;
 
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
@@ -626,17 +684,23 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
+      && (!EQ (minibuf_window, old_minibuf_window))
       && minibuf_moves_frame_when_opened ()
-      && (!minibuf_follows_frame ()
-	  || (!EQ (mini_frame, selected_frame))))
+      && (!minibuf_follows_frame ()))
     {
-      EMACS_INT i;
+      Lisp_Object old_frame = XWINDOW (old_minibuf_window)->frame;
+      struct frame *of = XFRAME (old_frame);
 
-      /* Stack up the existing minibuffers on the current mini-window */
-      for (i = 1; i < minibuf_level; i++)
-	set_window_buffer (minibuf_window, nth_minibuffer (i), 0, 0);
+      zip_minibuffer_stacks (minibuf_window, old_minibuf_window);
+      /* The frame's minibuffer can be on a different frame.  */
+      if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
+	Fset_frame_selected_window (old_frame,
+				    Fframe_first_window (old_frame), Qnil);
     }
+  if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
+    call1 (Qrecord_window_buffer, minibuf_window);
 
+  record_unwind_protect_void (minibuffer_unwind);
   record_unwind_protect (restore_window_configuration,
                          Fcons (Qt, Fcurrent_window_configuration (Qnil)));
 
@@ -771,23 +835,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   empty_minibuf = get_minibuffer (0);
   set_minibuffer_mode (empty_minibuf, 0);
 
-  FOR_EACH_FRAME (dummy, frame)
-    {
-      Lisp_Object root_window = Fframe_root_window (frame);
-      Lisp_Object mini_window = XWINDOW (root_window)->next;
-      Lisp_Object buffer;
-
-      if (!NILP (mini_window) && !EQ (mini_window, minibuf_window)
-          && !NILP (Fwindow_minibuffer_p (mini_window)))
-        {
-          buffer = XWINDOW (mini_window)->contents;
-          if (!live_minibuffer_p (buffer))
-            /* Use set_window_buffer instead of Fset_window_buffer (see
-               discussion of bug#11984, bug#12025, bug#12026).  */
-            set_window_buffer (mini_window, empty_minibuf, 0, 0);
-        }
-    }
-
   /* Display this minibuffer in the proper window.  */
   /* Use set_window_buffer instead of Fset_window_buffer (see
      discussion of bug#11984, bug#12025, bug#12026).  */
@@ -908,7 +955,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   unbind_to (count, Qnil);
 
   /* Switch the frame back to the calling frame.  */
-  if (!EQ (selected_frame, calling_frame)
+  if ((!EQ (selected_frame, calling_frame)
+       || !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)->frame,
+	       calling_frame))
       && FRAMEP (calling_frame)
       && FRAME_LIVE_P (XFRAME (calling_frame)))
     call2 (intern ("select-frame-set-input-focus"), calling_frame, Qnil);
@@ -1127,20 +1176,57 @@ read_minibuf_unwind (void)
      away from the expired minibuffer window, both in the current
      minibuffer's frame and the original calling frame.  */
   choose_minibuf_frame ();
-  if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
-  {
-    Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
-    /* PREV can be on a different frame when we have a minibuffer only
-       frame, the other frame's minibuffer window is MINIBUF_WINDOW,
-       and its "focus window" is also MINIBUF_WINDOW.  */
-    if (!EQ (prev, minibuf_window)
-	&& EQ (WINDOW_FRAME (XWINDOW (prev)),
-	       WINDOW_FRAME (XWINDOW (minibuf_window))))
-      Fset_frame_selected_window (selected_frame, prev, Qnil);
-  }
-  else
-    Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+  if (NILP (XWINDOW (minibuf_window)->prev_buffers))
+    {
+      if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
+	{
+	  Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
+	  /* PREV can be on a different frame when we have a minibuffer only
+	     frame, the other frame's minibuffer window is MINIBUF_WINDOW,
+	     and its "focus window" is also MINIBUF_WINDOW.  */
+	  if (!EQ (prev, minibuf_window)
+	      && EQ (WINDOW_FRAME (XWINDOW (prev)),
+		     WINDOW_FRAME (XWINDOW (minibuf_window))))
+	    Fset_frame_selected_window (selected_frame, prev, Qnil);
+	}
+      else
+	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+    }
 }
+
+/* Replace the expired minibuffer in the selected frame with the next
+   less nested minibuffer, if any, stored in the minibuffer window.
+   Otherwise, replace it with the null minibuffer.  MINIBUF_WINDOW
+   gets set to the selected frame's minibuffer.  */
+static void
+minibuffer_unwind (void)
+{
+  struct frame *sf = XFRAME (selected_frame);
+  struct window *w;
+  Lisp_Object entry;
+
+  if (FRAMEP (selected_frame)
+      && FRAME_LIVE_P (sf))
+    {
+      minibuf_window = sf->minibuffer_window;
+      w = XWINDOW (minibuf_window);
+      if (!NILP (w->prev_buffers))
+	{
+	  entry = Fcar (w->prev_buffers);
+	  w->prev_buffers = Fcdr (w->prev_buffers);
+	  set_window_buffer (minibuf_window, Fcar (entry), 0, 0);
+	  Fset_window_start (minibuf_window, Fcar (Fcdr (entry)), Qnil);
+	  Fset_window_point (minibuf_window, Fcar (Fcdr (Fcdr (entry))));
+	  /* set-window-configuration may/will have unselected the
+	     mini-window as the selected window.  Restore it. */
+	  if (EQ (w->frame, selected_frame))
+	    Fset_frame_selected_window (selected_frame, minibuf_window, Qnil);
+	}
+      else
+	set_window_buffer (minibuf_window, nth_minibuffer (0), 0, 0);
+    }
+}
+
 \f
 
 void
diff --git a/src/window.c b/src/window.c
index eb16e2a433..cde53e8059 100644
--- a/src/window.c
+++ b/src/window.c
@@ -6958,7 +6960,8 @@ the return value is nil.  Otherwise the value is t.  */)
 
 	  if (BUFFERP (w->contents)
 	      && !EQ (w->contents, p->buffer)
-	      && BUFFER_LIVE_P (XBUFFER (p->buffer)))
+	      && BUFFER_LIVE_P (XBUFFER (p->buffer))
+	      && (NILP (Fminibufferp (p->buffer, Qnil))))
 	    /* If a window we restore gets another buffer, record the
 	       window's old buffer.  */
 	    call1 (Qrecord_window_buffer, window);
diff --git a/src/xdisp.c b/src/xdisp.c
index cc0a689ba3..a405d51f80 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12650,9 +12650,8 @@ gui_consider_frame_title (Lisp_Object frame)
 	 mode_line_noprop_buf; then display the title.  */
       record_unwind_protect (unwind_format_mode_line,
 			     format_mode_line_unwind_data
-			       (f, current_buffer, selected_window, false));
+			     (NULL, current_buffer, Qnil, false));
 
-      Fselect_window (f->selected_window, Qt);
       set_buffer_internal_1
 	(XBUFFER (XWINDOW (f->selected_window)->contents));
       fmt = FRAME_ICONIFIED_P (f) ? Vicon_title_format : Vframe_title_format;


>         Stefan


-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: [External] : Re: Stop frames stealing eachothers' minibuffers!
  2021-02-07 23:48       ` [External] : " Drew Adams
@ 2021-03-13 18:31         ` Alan Mackenzie
  0 siblings, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-13 18:31 UTC (permalink / raw)
  To: Drew Adams; +Cc: jakanakaevangeli, emacs-devel@gnu.org

Hello, Drew.

On Sun, Feb 07, 2021 at 23:48:31 +0000, Drew Adams wrote:
> > I spent some time boring into the doc string and the description in the
> > Elisp manual.  The only conclusion I could come to was that C-] is
> > meant to abort EXACTLY ONE level.

> Apologies for chiming in without following
> the thread.

> I just want to say that my use of recursive
> minibuffers depends on being able to use a
> key (preferably `C-]', as always previously)
> to abort a single level (i.e., exactly one).
> ___

> I also have a command, `icicle-top-level',
> bound to `C-M-T' (aka `C-M-S-t'), which aborts
> to the top-level whenever possible.  Doc string:

>  Remove `*Completions*' window.  Return to top level.
>  Try to throw to `icicle-top-level' (`catch' is for multi-commands).
>  If that fails, call `top-level'.

I've just posted another patch in this area.

There's no intention that the functionality of C-] should be changed.  I
see no reason why your C-M-T should not carry on working.  However, if
you feel like testing these things, I'd be grateful!  Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 18:23             ` Alan Mackenzie
@ 2021-03-13 19:39               ` Stefan Monnier
  2021-03-13 20:24                 ` Alan Mackenzie
  2021-03-13 20:53               ` jakanakaevangeli
  1 sibling, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2021-03-13 19:39 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

Hi Alan,

> I would be grateful indeed if either of you (or indeed, anybody else),
> would apply the patch and try it out, or indeed comment on it.  Thanks!

I'm not very familiar with this part of Emacs's code, but here are some
things I noticed:

> @@ -4416,7 +4416,8 @@ record-window-buffer
>         window (assq-delete-all buffer (window-prev-buffers window))))
>  
>      ;; Don't record insignificant buffers.
> -    (unless (eq (aref (buffer-name buffer) 0) ?\s)
> +    (when (or (not (eq (aref (buffer-name buffer) 0) ?\s))
> +              (string-match "^ \\*Minibuf" (buffer-name buffer)))

I think you want to use `minibufferp` here (and if not, then please add
a comment explaining why).

> @@ -4348,7 +4348,7 @@ extern void clear_regexp_cache (void);
>  
>  extern Lisp_Object Vminibuffer_list;
>  extern Lisp_Object last_minibuf_string;
> -extern void move_minibuffer_onto_frame (void);
> +extern void move_minibuffers_onto_frame (struct frame *, int);

Please use the `bool` type for booleans.

> +/* Get the next live buffer entry from a w->prev_buffer, setting the pertinent
> +  variables of `zip_minibuffer_stacks'.  The parameter P is either "d" for
> +  the destination structures, or "s" for the source structures.  When the
> +  ->prev_buffers list is exhausted, set di/si to -1.  */
> +#define NEXT_BUFFER_ENTRY(p)						\
> +  do									\
> +    {									\
> +      if (NILP (p##_bufs))						\
> +        {								\
> +	  p##b = Qnil;							\
> +	  p##_ent = Qnil;						\
> +	}								\
> +      else								\
> +	{								\
> +	  p##_ent = Fcar (p##_bufs);					\
> +	  p##b = Fcar (p##_ent);					\
> +	  p##_bufs = Fcdr (p##_bufs);					\
> +	}								\
> +      if (!NILP (p##b))							\
> +	p##i = this_minibuffer_depth (p##b);				\
> +      else								\
> +	p##i = -1;							\
> +    } while (p##i == 0)

Any chance we could make that into a function taking an argument `p`
that's a reference to a

   struct { Lisp_Object bufs; Lisp_Object b; Lisp_Object ent; int depth; }

?  I know we already use such longish macros in various places, but I'd
rather we use fewer of them rather than adding more of them, because
they're a pain to debug (and I find they make the code harder to read;
e.g. here we see a call to `NEXT_BUFFER_ENTRY` we can't know which
variables will be accessed/modified without looking at the definition of
the macro).

> +/* Move the ordered "stack" of minibuffers from SOURCE_WINDOW to
> +   DEST_WINDOW, interleaving those minibuffers with any in DEST_WINDOW
> +   to produce an ordered combination.  The ordering is by minibuffer
> +   depth.  A stack of minibuffers consists of the minibuffer currently
> +   in DEST/SOURCE_WINDOW together with any recorded in the
> +   ->prev_buffers field of the struct window.  */

This is actually the "merge" part of a merge sort, so we could use
`sort_list` or even just the `merge` function used by `sort_list`.

[ Tho maybe it is simpler and more robust (in case ELisp code can access
  (and hence modify) those lists) to leave those lists not sorted, and
  instead look for the deepest buffer in `minibuffer_unwind` rather than
  just picking the next.  ]

> +/* If `minibuffer_follows_selected_frame' is t, or we're about to
> +   delete a frame which potentially "contains" minibuffers, move them
> +   from the old frame to the selected frame.  This function is
> +   intended to be called from `do_switch_frame' in frame.c.  OF is the
> +   old frame, FOR_DELETION is self explanatory.  */

Actually, I suspect that the "self explanatory" part will not be true
when my daughter ends up having to fix that code 10 years from now.
[ Tho, it currently looks quite unlikely :-(  ]


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 19:39               ` Stefan Monnier
@ 2021-03-13 20:24                 ` Alan Mackenzie
  2021-03-13 20:52                   ` Stefan Monnier
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-13 20:24 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: jakanakaevangeli, emacs-devel

Hello, Stefan.

Thanks for such a rapid and useful reply.

I'll put most of your suggestions (actually, probably all of them) into
my code this evening.

I didn't actually know there was a `sort_list' function, or a merge part
of it, so that should save quite a lot of code if I can use it (which I
probably can).

On Sat, Mar 13, 2021 at 14:39:28 -0500, Stefan Monnier wrote:
> Hi Alan,

> > I would be grateful indeed if either of you (or indeed, anybody else),
> > would apply the patch and try it out, or indeed comment on it.  Thanks!

> I'm not very familiar with this part of Emacs's code, but here are some
> things I noticed:

[ .... ]

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 20:24                 ` Alan Mackenzie
@ 2021-03-13 20:52                   ` Stefan Monnier
  2021-03-14 18:26                     ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Stefan Monnier @ 2021-03-13 20:52 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

> I didn't actually know there was a `sort_list' function, or a merge part
> of it, so that should save quite a lot of code if I can use it (which I
> probably can).

I didn't either, really, I found them by following the code from
`Fsort`.  Of course, this is meant for ELisp so the predicate has to be
an ELisp value rather than a C function, so it will probably force you
to define the predicate with DEFUN, which is not completely
satisfactory (tho not terribly harmful either, I guess).


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 18:23             ` Alan Mackenzie
  2021-03-13 19:39               ` Stefan Monnier
@ 2021-03-13 20:53               ` jakanakaevangeli
  2021-03-14 19:17                 ` Alan Mackenzie
  1 sibling, 1 reply; 52+ messages in thread
From: jakanakaevangeli @ 2021-03-13 20:53 UTC (permalink / raw)
  To: Alan Mackenzie, Stefan Monnier; +Cc: emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello, Stefan and jakanakaevangeli.
>
> On Thu, Feb 11, 2021 at 09:29:44 -0500, Stefan Monnier wrote:
>> > This is more involved.  What do we want to happen when a frame with open
>> > minibuffers is deleted?  I would say that these minibuffers should,
>> > except in the (eq minibuffer-follows-selected-frame t) case, be aborted,
>> > together with any others in other frames whose nesting level makes this
>> > necessary.
>
>> I vote for moving those minibuffers elsewhere (anywhere else is fine by
>> me, really).  I assume it's no more complicated code-wise, and it should
>> suffer less from the risk of losing information.
>
> Just a quick recapitulation of the problem: when
> minibuffer-follows-selected-frame is nil, and
> enable-recursive-minibuffers t, it is possible to have several open
> minibuffers on several frames.  If one of these frames is deleted, its
> minibuffers continue to exist, but are wholly inaccessible - the only
> thing to do with them is to abort them, e.g. with C-].  This is
> suboptimal.
>
> The patch below tries to solve this by, when such a frame gets deleted,
> "zipping" its minibuffers into those of another frame.  The idea behind
> the patch is to use the mini-window's w->prev_buffers component to hold
> the list of its minibuffers.  This mini-window component was unused by
> the rest of Emacs, apart from accidentally by
> window--before-delete-windows, which I have now amended.
>
> I would be grateful indeed if either of you (or indeed, anybody else),
> would apply the patch and try it out, or indeed comment on it.  Thanks!

Great! I tested it and there still seem to be a few ways one can end up
with invisible active minibuffers, all of them with
minibuffer-follows-selected-frame set to nil:

1)
C-x C-f on frame A
C-x C-f on frame B
select frame A and press C-] (abort-recursive-edit)

This does quit only the inner minibuffer as expected but it hides the
outer one for some reason. This also seems to be a regression of this
patch, the minibuffer isn't hidden without this patch applied.

2) (Might depend on your window manager, I'm not sure)
Open frames A, B, and C
C-x C-f on frame A
C-x b on frame B
Close frame A

Both of the minibuffers are now moved to frame C and the outer C-x C-f
is shown in the mini-window, which is kind of annoying. If we now select
frame C and press C-], the same problem as in 1) occurs.

3)
In emacs daemon, evaluate
(run-at-time 5 nil #'make-frame '((window-system . x)))
and then open a minibuffer and close the last frame. When a new frame
appears, the same problem as in 1) occurs.

Hope this helps.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 20:52                   ` Stefan Monnier
@ 2021-03-14 18:26                     ` Alan Mackenzie
  2021-03-14 18:48                       ` Eli Zaretskii
  2021-03-14 20:32                       ` Stefan Monnier
  0 siblings, 2 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-14 18:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: jakanakaevangeli, emacs-devel

Hello, Stefan.

On Sat, Mar 13, 2021 at 15:52:51 -0500, Stefan Monnier wrote:
> > I didn't actually know there was a `sort_list' function, or a merge part
> > of it, so that should save quite a lot of code if I can use it (which I
> > probably can).

> I didn't either, really, I found them by following the code from
> `Fsort`.  Of course, this is meant for ELisp so the predicate has to be
> an ELisp value rather than a C function, so it will probably force you
> to define the predicate with DEFUN, which is not completely
> satisfactory (tho not terribly harmful either, I guess).

No, not completely satisfactory.  So, after spending an hour or so
trying to work out how to define a Lisp function in C without it being
visible in the Lisp world, I hit on a simpler solution - copy `merge' to
a new function `merge_c', replacing the Lisp predicate with a boolean C
function.  This works fine, at the small cost of a little code
duplication.

I think I've followed all your other suggestions from yesterday, now.

Anyhow, I think I've fixed the bugs that jakanakaevangeli pointed out
yesterday, so I'll post my updated patch in my reply to her/him.

>         Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-14 18:26                     ` Alan Mackenzie
@ 2021-03-14 18:48                       ` Eli Zaretskii
  2021-03-14 20:32                       ` Stefan Monnier
  1 sibling, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-14 18:48 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: monnier, jakanakaevangeli, emacs-devel

> Date: Sun, 14 Mar 2021 18:26:18 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: jakanakaevangeli <jakanakaevangeli@chiru.no>, emacs-devel@gnu.org
> 
> after spending an hour or so trying to work out how to define a Lisp
> function in C without it being visible in the Lisp world

Isn't defsubr sup[posed to do that?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-13 20:53               ` jakanakaevangeli
@ 2021-03-14 19:17                 ` Alan Mackenzie
  2021-03-14 21:23                   ` Miha Rihtaršič
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-14 19:17 UTC (permalink / raw)
  To: jakanakaevangeli; +Cc: Stefan Monnier, emacs-devel

Hello, Jakanakaevangeli,

On Sat, Mar 13, 2021 at 21:53:05 +0100, jakanakaevangeli wrote:
> Alan Mackenzie <acm@muc.de> writes:

[ .... ]

> > I would be grateful indeed if either of you (or indeed, anybody else),
> > would apply the patch and try it out, or indeed comment on it.  Thanks!

> Great! I tested it and there still seem to be a few ways one can end up
> with invisible active minibuffers, all of them with
> minibuffer-follows-selected-frame set to nil:

> 1)
> C-x C-f on frame A
> C-x C-f on frame B
> select frame A and press C-] (abort-recursive-edit)

> This does quit only the inner minibuffer as expected but it hides the
> outer one for some reason. This also seems to be a regression of this
> patch, the minibuffer isn't hidden without this patch applied.

I think I've fixed that, now.  There was a call to
set-window-configuration which was replacing the wanted minibuffer with
a different one.  That's now fixed.

> 2) (Might depend on your window manager, I'm not sure)
> Open frames A, B, and C
> C-x C-f on frame A
> C-x b on frame B
> Close frame A

> Both of the minibuffers are now moved to frame C and the outer C-x C-f
> is shown in the mini-window, which is kind of annoying. If we now select
> frame C and press C-], the same problem as in 1) occurs.

This was a bit of dangling code from an earlier version which should
have been removed.  It was, again, overwriting "the" minibuffer (which
was still on frame B) with the null minibuffer, thus causing the C-x b
to be lost.

> 3)
> In emacs daemon, evaluate
> (run-at-time 5 nil #'make-frame '((window-system . x)))
> and then open a minibuffer and close the last frame. When a new frame
> appears, the same problem as in 1) occurs.

I think one of the two previous changes fixed this.

> Hope this helps.

It's been an enormous help, thanks!  Testing this is difficult because
there are so many different things which can interact with eachother in
so many unwanted ways.

Anyway, here's an updated patch, which should apply to master,
incorporating Stefan's suggestions from last night, and fixes to the
above bugs:




diff --git a/doc/emacs/mini.texi b/doc/emacs/mini.texi
index f81e64bdf9..7da0a48b7c 100644
--- a/doc/emacs/mini.texi
+++ b/doc/emacs/mini.texi
@@ -82,7 +82,9 @@ Basic Minibuffer
 (@pxref{Recursive Mini,,, elisp}).  This option is mainly to retain
 (approximately) the behavior prior to Emacs 28.1.  Note that the
 effect of the command, when you finally finish using the minibuffer,
-always takes place in the frame where you first opened it.
+always takes place in the frame where you first opened it.  The sole
+exception is that when that frame no longer exists, the action takes
+place in the currently selected frame.
 
 @node Minibuffer File
 @section Minibuffers for File Names
diff --git a/lisp/window.el b/lisp/window.el
index cfd9876ed0..f27631bb86 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -4158,7 +4158,7 @@ window--before-delete-windows
 
 This function is called only if `switch-to-buffer-preserve-window-point'
 evaluates non-nil."
-  (dolist (win (window-list))
+  (dolist (win (window-list nil 'no-minibuf))
     (let* ((buf   (window-buffer (or window win)))
            (start (window-start win))
            (pos   (window-point win))
@@ -4416,7 +4416,8 @@ record-window-buffer
        window (assq-delete-all buffer (window-prev-buffers window))))
 
     ;; Don't record insignificant buffers.
-    (unless (eq (aref (buffer-name buffer) 0) ?\s)
+    (when (or (not (eq (aref (buffer-name buffer) 0) ?\s))
+              (minibufferp buffer))
       ;; Add an entry for buffer to WINDOW's previous buffers.
       (with-current-buffer buffer
 	(let ((start (window-start window))
diff --git a/src/fns.c b/src/fns.c
index 7914bd4779..f9bd3913bb 100644
--- a/src/fns.c
+++ b/src/fns.c
@@ -2227,6 +2227,52 @@ merge (Lisp_Object org_l1, Lisp_Object org_l2, Lisp_Object pred)
     }
 }
 
+Lisp_Object
+merge_c (Lisp_Object org_l1, Lisp_Object org_l2, bool (*less) (Lisp_Object, Lisp_Object))
+{
+  Lisp_Object l1 = org_l1;
+  Lisp_Object l2 = org_l2;
+  Lisp_Object tail = Qnil;
+  Lisp_Object value = Qnil;
+
+  while (1)
+    {
+      if (NILP (l1))
+	{
+	  if (NILP (tail))
+	    return l2;
+	  Fsetcdr (tail, l2);
+	  return value;
+	}
+      if (NILP (l2))
+	{
+	  if (NILP (tail))
+	    return l1;
+	  Fsetcdr (tail, l1);
+	  return value;
+	}
+
+      Lisp_Object tem;
+      if (less (Fcar (l1), Fcar (l2)))
+	{
+	  tem = l1;
+	  l1 = Fcdr (l1);
+	  org_l1 = l1;
+	}
+      else
+	{
+	  tem = l2;
+	  l2 = Fcdr (l2);
+	  org_l2 = l2;
+	}
+      if (NILP (tail))
+	value = tem;
+      else
+	Fsetcdr (tail, tem);
+      tail = tem;
+    }
+}
+
 \f
 /* This does not check for quits.  That is safe since it must terminate.  */
 
diff --git a/src/frame.c b/src/frame.c
index a62347c1fb..b9df5739dd 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1487,7 +1487,7 @@ do_switch_frame (Lisp_Object frame, int track, int for_deletion, Lisp_Object nor
 #endif
     internal_last_event_frame = Qnil;
 
-  move_minibuffer_onto_frame ();
+  move_minibuffers_onto_frame (sf, for_deletion);
   return frame;
 }
 
diff --git a/src/lisp.h b/src/lisp.h
index b95f389b89..c67c8b0857 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -3610,6 +3610,7 @@ extern void validate_subarray (Lisp_Object, Lisp_Object, Lisp_Object,
 extern Lisp_Object substring_both (Lisp_Object, ptrdiff_t, ptrdiff_t,
 				   ptrdiff_t, ptrdiff_t);
 extern Lisp_Object merge (Lisp_Object, Lisp_Object, Lisp_Object);
+extern Lisp_Object merge_c (Lisp_Object, Lisp_Object, bool (*) (Lisp_Object, Lisp_Object));
 extern Lisp_Object do_yes_or_no_p (Lisp_Object);
 extern int string_version_cmp (Lisp_Object, Lisp_Object);
 extern Lisp_Object concat2 (Lisp_Object, Lisp_Object);
@@ -4348,7 +4349,7 @@ extern void clear_regexp_cache (void);
 
 extern Lisp_Object Vminibuffer_list;
 extern Lisp_Object last_minibuf_string;
-extern void move_minibuffer_onto_frame (void);
+extern void move_minibuffers_onto_frame (struct frame *, bool);
 extern bool is_minibuffer (EMACS_INT, Lisp_Object);
 extern EMACS_INT this_minibuffer_depth (Lisp_Object);
 extern EMACS_INT minibuf_level;
diff --git a/src/minibuf.c b/src/minibuf.c
index 4b1f4b1ff7..279dc67c33 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -67,6 +67,7 @@ static ptrdiff_t minibuf_prompt_width;
 static Lisp_Object nth_minibuffer (EMACS_INT depth);
 static EMACS_INT minibuf_c_loop_level (EMACS_INT depth);
 static void set_minibuffer_mode (Lisp_Object buf, EMACS_INT depth);
+static bool live_minibuffer_p (Lisp_Object);
 
 \f
 /* Return TRUE when a frame switch causes a minibuffer on the old
@@ -78,6 +79,7 @@ minibuf_follows_frame (void)
              Qt);
 }
 
+#if 0
 /* Return TRUE when a minibuffer always remains on the frame where it
    was first invoked. */
 static bool
@@ -85,6 +87,7 @@ minibuf_stays_put (void)
 {
   return NILP (Fdefault_toplevel_value (Qminibuffer_follows_selected_frame));
 }
+#endif
 
 /* Return TRUE when opening a (recursive) minibuffer causes
    minibuffers on other frames to move to the selected frame.  */
@@ -112,84 +115,86 @@ choose_minibuf_frame (void)
 	emacs_abort ();
 
       minibuf_window = sf->minibuffer_window;
-      /* If we've still got another minibuffer open, use its mini-window
-         instead.  */
-      if (minibuf_level > 1 && minibuf_stays_put ())
-        {
-          Lisp_Object buffer = get_minibuffer (minibuf_level);
-          Lisp_Object tail, frame;
-
-          FOR_EACH_FRAME (tail, frame)
-            if (EQ (XWINDOW (XFRAME (frame)->minibuffer_window)->contents,
-                    buffer))
-              {
-                minibuf_window = XFRAME (frame)->minibuffer_window;
-                break;
-              }
-        }
     }
+}
 
-  if (minibuf_moves_frame_when_opened ()
-      && FRAMEP (selected_frame)
-      && FRAME_LIVE_P (XFRAME (selected_frame)))
-    /* Make sure no other frame has a minibuffer as its selected window,
-       because the text would not be displayed in it, and that would be
-       confusing.  Only allow the selected frame to do this,
-       and that only if the minibuffer is active.  */
-  {
-    Lisp_Object tail, frame;
-    struct frame *of;
-
-    FOR_EACH_FRAME (tail, frame)
-      if (!EQ (frame, selected_frame)
-          && minibuf_level > 1
-	  /* The frame's minibuffer can be on a different frame.  */
-	  && ! EQ (XWINDOW ((of = XFRAME (frame))->minibuffer_window)->frame,
-		   selected_frame))
-        {
-          if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-            Fset_frame_selected_window (frame, Fframe_first_window (frame),
-                                        Qnil);
-
-          if (!EQ (XWINDOW (of->minibuffer_window)->contents,
-                   nth_minibuffer (0)))
-            set_window_buffer (of->minibuffer_window,
-                               nth_minibuffer (0), 0, 0);
-        }
-  }
+/* If ENT1 has a higher minibuffer index than ENT2, return true.  More
+precisely, compare the buffer components of each window->prev_buffers
+entry.  */
+static bool
+minibuffer_ent_greater (Lisp_Object ent1, Lisp_Object ent2)
+{
+  return this_minibuffer_depth (Fcar (ent1))
+    > this_minibuffer_depth (Fcar (ent2)) ;
 }
 
-/* If `minibuffer_follows_selected_frame' is t and we have a
-   minibuffer, move it from its current frame to the selected frame.
-   This function is intended to be called from `do_switch_frame' in
-   frame.c.  */
-void move_minibuffer_onto_frame (void)
+/* Move the ordered "stack" of minibuffers from SOURCE_WINDOW to
+   DEST_WINDOW, interleaving those minibuffers with any in DEST_WINDOW
+   to produce an ordered combination.  The ordering is by minibuffer
+   depth.  A stack of minibuffers consists of the minibuffer currently
+   in DEST/SOURCE_WINDOW together with any recorded in the
+   ->prev_buffers field of the struct window.  */
+static void
+zip_minibuffer_stacks (Lisp_Object dest_window, Lisp_Object source_window)
 {
-  if (!minibuf_level)
-    return;
-  if (!minibuf_follows_frame ())
+  struct window *dw = XWINDOW (dest_window);
+  struct window *sw = XWINDOW (source_window);
+  Lisp_Object acc;
+  Lisp_Object d_ent;	/* Entry from dw->prev_buffers */
+
+  if (!live_minibuffer_p (dw->contents)
+      && NILP (dw->prev_buffers))
+    {
+      set_window_buffer (dest_window, sw->contents, 0, 0);
+      Fset_window_start (dest_window, Fwindow_start (source_window), Qnil);
+      Fset_window_point (dest_window, Fwindow_point (source_window));
+      dw->prev_buffers = sw->prev_buffers;
+      set_window_buffer (source_window, get_minibuffer (0), 0, 0);
+      sw->prev_buffers = Qnil;
+      return;
+    }
+
+  if (live_minibuffer_p (dw->contents))
+    call1 (Qrecord_window_buffer, dest_window);
+  if (live_minibuffer_p (sw->contents))
+    call1 (Qrecord_window_buffer, source_window);
+
+  acc = merge_c (dw->prev_buffers, sw->prev_buffers, minibuffer_ent_greater);
+
+  if (!NILP (acc))
+    {
+      d_ent = Fcar (acc);
+      acc = Fcdr (acc);
+      set_window_buffer (dest_window, Fcar (d_ent), 0, 0);
+      Fset_window_start (dest_window, Fcar (Fcdr (d_ent)), Qnil);
+      Fset_window_point (dest_window, Fcar (Fcdr (Fcdr (d_ent))));
+    }
+  dw->prev_buffers = acc;
+  sw->prev_buffers = Qnil;
+  set_window_buffer (source_window, get_minibuffer (0), 0, 0);
+}
+
+/* If `minibuffer_follows_selected_frame' is t, or we're about to
+   delete a frame which potentially "contains" minibuffers, move them
+   from the old frame to the selected frame.  This function is
+   intended to be called from `do_switch_frame' in frame.c.  OF is the
+   old frame, FOR_DELETION is true if OF is about to be deleted.  */
+void
+move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
+{
+  if (!for_deletion && (!minibuf_level || !minibuf_follows_frame ()))
     return;
   if (FRAMEP (selected_frame)
       && FRAME_LIVE_P (XFRAME (selected_frame))
-      && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
+      && (for_deletion
+	  || !EQ (minibuf_window,
+		  XFRAME (selected_frame)->minibuffer_window)))
     {
-      EMACS_INT i;
       struct frame *sf = XFRAME (selected_frame);
-      Lisp_Object old_frame = XWINDOW (minibuf_window)->frame;
-      struct frame *of = XFRAME (old_frame);
 
-      /* Stack up all the (recursively) open minibuffers on the selected
-         mini_window.  */
-      for (i = 1; i <= minibuf_level; i++)
-	set_window_buffer (sf->minibuffer_window, nth_minibuffer (i), 0, 0);
-      minibuf_window = sf->minibuffer_window;
-      if (of != sf)
-	{
-	  Lisp_Object temp = get_minibuffer (0);
-
-	  set_window_buffer (of->minibuffer_window, temp, 0, 0);
-	  set_minibuffer_mode (temp, 0);
-	}
+      if (minibuf_follows_frame () || for_deletion)
+	zip_minibuffer_stacks (sf->minibuffer_window,
+			       of->minibuffer_window);
     }
 }
 
@@ -221,6 +226,7 @@ without invoking the usual minibuffer commands.  */)
 /* Actual minibuffer invocation.  */
 
 static void read_minibuf_unwind (void);
+static void minibuffer_unwind (void);
 static void run_exit_minibuf_hook (void);
 
 
@@ -544,7 +550,7 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
-  Lisp_Object dummy, frame;
+  Lisp_Object old_minibuf_window = minibuf_window;
 
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
@@ -626,17 +632,23 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
+      && (!EQ (minibuf_window, old_minibuf_window))
       && minibuf_moves_frame_when_opened ()
-      && (!minibuf_follows_frame ()
-	  || (!EQ (mini_frame, selected_frame))))
+      && (!minibuf_follows_frame ()))
     {
-      EMACS_INT i;
+      Lisp_Object old_frame = XWINDOW (old_minibuf_window)->frame;
+      struct frame *of = XFRAME (old_frame);
 
-      /* Stack up the existing minibuffers on the current mini-window */
-      for (i = 1; i < minibuf_level; i++)
-	set_window_buffer (minibuf_window, nth_minibuffer (i), 0, 0);
+      zip_minibuffer_stacks (minibuf_window, old_minibuf_window);
+      /* The frame's minibuffer can be on a different frame.  */
+      if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
+	Fset_frame_selected_window (old_frame,
+				    Fframe_first_window (old_frame), Qnil);
     }
+  if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
+    call1 (Qrecord_window_buffer, minibuf_window);
 
+  record_unwind_protect_void (minibuffer_unwind);
   record_unwind_protect (restore_window_configuration,
                          Fcons (Qt, Fcurrent_window_configuration (Qnil)));
 
@@ -771,23 +783,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   empty_minibuf = get_minibuffer (0);
   set_minibuffer_mode (empty_minibuf, 0);
 
-  FOR_EACH_FRAME (dummy, frame)
-    {
-      Lisp_Object root_window = Fframe_root_window (frame);
-      Lisp_Object mini_window = XWINDOW (root_window)->next;
-      Lisp_Object buffer;
-
-      if (!NILP (mini_window) && !EQ (mini_window, minibuf_window)
-          && !NILP (Fwindow_minibuffer_p (mini_window)))
-        {
-          buffer = XWINDOW (mini_window)->contents;
-          if (!live_minibuffer_p (buffer))
-            /* Use set_window_buffer instead of Fset_window_buffer (see
-               discussion of bug#11984, bug#12025, bug#12026).  */
-            set_window_buffer (mini_window, empty_minibuf, 0, 0);
-        }
-    }
-
   /* Display this minibuffer in the proper window.  */
   /* Use set_window_buffer instead of Fset_window_buffer (see
      discussion of bug#11984, bug#12025, bug#12026).  */
@@ -908,7 +903,9 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, Lisp_Object prompt,
   unbind_to (count, Qnil);
 
   /* Switch the frame back to the calling frame.  */
-  if (!EQ (selected_frame, calling_frame)
+  if ((!EQ (selected_frame, calling_frame)
+       || !EQ (XWINDOW (XFRAME (calling_frame)->minibuffer_window)->frame,
+	       calling_frame))
       && FRAMEP (calling_frame)
       && FRAME_LIVE_P (XFRAME (calling_frame)))
     call2 (intern ("select-frame-set-input-focus"), calling_frame, Qnil);
@@ -1026,6 +1023,11 @@ run_exit_minibuf_hook (void)
   safe_run_hooks (Qminibuffer_exit_hook);
 }
 
+/* This variable preserves the minibuffer in the selected frame across
+   the call of restore_window_configuration.  It should be used only
+   by `read_minibuf_unwind' and `minibuffer_unwind'.  */
+static Lisp_Object selected_frame_MB;
+
 /* This function is called on exiting minibuffer, whether normally or
    not, and it restores the current window, buffer, etc.  */
 
@@ -1127,20 +1129,72 @@ read_minibuf_unwind (void)
      away from the expired minibuffer window, both in the current
      minibuffer's frame and the original calling frame.  */
   choose_minibuf_frame ();
-  if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
-  {
-    Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
-    /* PREV can be on a different frame when we have a minibuffer only
-       frame, the other frame's minibuffer window is MINIBUF_WINDOW,
-       and its "focus window" is also MINIBUF_WINDOW.  */
-    if (!EQ (prev, minibuf_window)
-	&& EQ (WINDOW_FRAME (XWINDOW (prev)),
-	       WINDOW_FRAME (XWINDOW (minibuf_window))))
-      Fset_frame_selected_window (selected_frame, prev, Qnil);
-  }
-  else
-    Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+  selected_frame_MB = XWINDOW (minibuf_window)->contents;
+  if (NILP (XWINDOW (minibuf_window)->prev_buffers))
+    {
+      if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
+	{
+	  Lisp_Object prev = Fprevious_window (minibuf_window, Qnil, Qnil);
+	  /* PREV can be on a different frame when we have a minibuffer only
+	     frame, the other frame's minibuffer window is MINIBUF_WINDOW,
+	     and its "focus window" is also MINIBUF_WINDOW.  */
+	  if (!EQ (prev, minibuf_window)
+	      && EQ (WINDOW_FRAME (XWINDOW (prev)),
+		     WINDOW_FRAME (XWINDOW (minibuf_window))))
+	    Fset_frame_selected_window (selected_frame, prev, Qnil);
+	}
+      else
+	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
+    }
 }
+
+/* Replace the expired minibuffer in whatever frame it is now in with
+   the next less nested minibuffer in that frame, if any.  Otherwise,
+   replace it with the null minibuffer.  MINIBUF_WINDOW is not
+   changed.  */
+static void
+minibuffer_unwind (void)
+{
+  struct frame *f;
+  struct window *w;
+  Lisp_Object window, frame, frames;
+  Lisp_Object entry;
+
+  /* Locate the expired minibuffer.  */
+  FOR_EACH_FRAME (frames, frame)
+    {
+      f = XFRAME (frame);
+      window = f->minibuffer_window;
+      w = XWINDOW (window);
+      if (EQ (w->frame, frame)
+	  && EQ (EQ (frame, selected_frame)
+		 ? selected_frame_MB
+		 : w->contents,
+		 nth_minibuffer (minibuf_level + 1)))
+	goto found;
+    }
+  return; 			/* expired minibuffer not found */
+
+ found:
+  if (FRAME_LIVE_P (f))
+    {
+      /* minibuf_window = sf->minibuffer_window; */
+      if (!NILP (w->prev_buffers))
+	{
+	  entry = Fcar (w->prev_buffers);
+	  w->prev_buffers = Fcdr (w->prev_buffers);
+	  set_window_buffer (window, Fcar (entry), 0, 0);
+	  Fset_window_start (window, Fcar (Fcdr (entry)), Qnil);
+	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
+	  /* set-window-configuration may/will have unselected the
+	     mini-window as the selected window.  Restore it. */
+	  Fset_frame_selected_window (frame, window, Qnil);
+	}
+      else
+	set_window_buffer (window, nth_minibuffer (0), 0, 0);
+    }
+}
+
 \f
 
 void
@@ -2213,6 +2267,7 @@ syms_of_minibuf (void)
 {
   staticpro (&minibuf_prompt);
   staticpro (&minibuf_save_list);
+  staticpro (&selected_frame_MB);
 
   DEFSYM (Qminibuffer_follows_selected_frame,
           "minibuffer-follows-selected-frame");
diff --git a/src/window.c b/src/window.c
index eb16e2a433..cde53e8059 100644
--- a/src/window.c
+++ b/src/window.c
@@ -6958,7 +6960,8 @@ the return value is nil.  Otherwise the value is t.  */)
 
 	  if (BUFFERP (w->contents)
 	      && !EQ (w->contents, p->buffer)
-	      && BUFFER_LIVE_P (XBUFFER (p->buffer)))
+	      && BUFFER_LIVE_P (XBUFFER (p->buffer))
+	      && (NILP (Fminibufferp (p->buffer, Qnil))))
 	    /* If a window we restore gets another buffer, record the
 	       window's old buffer.  */
 	    call1 (Qrecord_window_buffer, window);
diff --git a/src/xdisp.c b/src/xdisp.c
index cc0a689ba3..a405d51f80 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -12650,9 +12650,8 @@ gui_consider_frame_title (Lisp_Object frame)
 	 mode_line_noprop_buf; then display the title.  */
       record_unwind_protect (unwind_format_mode_line,
 			     format_mode_line_unwind_data
-			       (f, current_buffer, selected_window, false));
+			     (NULL, current_buffer, Qnil, false));
 
-      Fselect_window (f->selected_window, Qt);
       set_buffer_internal_1
 	(XBUFFER (XWINDOW (f->selected_window)->contents));
       fmt = FRAME_ICONIFIED_P (f) ? Vicon_title_format : Vframe_title_format;


-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-14 18:26                     ` Alan Mackenzie
  2021-03-14 18:48                       ` Eli Zaretskii
@ 2021-03-14 20:32                       ` Stefan Monnier
  1 sibling, 0 replies; 52+ messages in thread
From: Stefan Monnier @ 2021-03-14 20:32 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

> No, not completely satisfactory.  So, after spending an hour or so
> trying to work out how to define a Lisp function in C without it being
> visible in the Lisp world,

Oh, I wasn't even thinking of trying to hide it from ELisp (AFAICT it's
not dangerous to expose it to ELisp).


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-14 19:17                 ` Alan Mackenzie
@ 2021-03-14 21:23                   ` Miha Rihtaršič
  2021-03-17 19:32                     ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Miha Rihtaršič @ 2021-03-14 21:23 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Stefan Monnier, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello, Jakanakaevangeli,
>
> On Sat, Mar 13, 2021 at 21:53:05 +0100, jakanakaevangeli wrote:
>
> [ .... ]
>
>> 3)
>> In emacs daemon, evaluate
>> (run-at-time 5 nil #'make-frame '((window-system . x)))
>> and then open a minibuffer and close the last frame. When a new frame
>> appears, the same problem as in 1) occurs.
>
> I think one of the two previous changes fixed this.

Sorry, this still doesn't seem to be fixed. Just in case I wasn't clear
enough, I'll try to reiterate.

When a frame is closed, it's active minibuffer should now be moved to
another frame. What I wanted to test then was, what happens if we have
only a single frame with a minibuffer and we decide to close it, which
is possible with emacs daemon. Immediately, there will be no frames left
for the minibuffer to take refuge in and this seems to cause some
problems.

And two new regressions. These require minibuffer-follows-selected-frame
to be set to t.

1)
C-x C-f on frame A
select frame B
select frame A
Minibuffer is moved to B, but not back to A.

2)
Have two frames open
open a minibuffer on a frame
close this frame
The other frame does have the miniwindow selected, but the
minibuffer isn't shown in it.

Best.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-14 21:23                   ` Miha Rihtaršič
@ 2021-03-17 19:32                     ` Alan Mackenzie
  2021-03-17 19:55                       ` Eli Zaretskii
  2021-03-21 15:44                       ` Miha Rihtaršič
  0 siblings, 2 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-17 19:32 UTC (permalink / raw)
  To: Miha Rihtaršič; +Cc: Stefan Monnier, emacs-devel

Hello, Miha.

On Sun, Mar 14, 2021 at 22:23:02 +0100, Miha Rihtaršič wrote:
> Alan Mackenzie <acm@muc.de> writes:

> > Hello, Jakanakaevangeli,

> > On Sat, Mar 13, 2021 at 21:53:05 +0100, jakanakaevangeli wrote:

> > [ .... ]

> >> 3)
> >> In emacs daemon, evaluate
> >> (run-at-time 5 nil #'make-frame '((window-system . x)))
> >> and then open a minibuffer and close the last frame. When a new frame
> >> appears, the same problem as in 1) occurs.

> > I think one of the two previous changes fixed this.

> Sorry, this still doesn't seem to be fixed. Just in case I wasn't clear
> enough, I'll try to reiterate.

> When a frame is closed, it's active minibuffer should now be moved to
> another frame. What I wanted to test then was, what happens if we have
> only a single frame with a minibuffer and we decide to close it, which
> is possible with emacs daemon. Immediately, there will be no frames left
> for the minibuffer to take refuge in and this seems to cause some
> problems.

My apologies.  I hadn't understood what the bug is, and can confirm it's
still there.  My patch below doesn't address it.

This could be difficult to fix.  I don't think that clicking on the last
frame's close button goes through `delete-frame' - it just closes the
program, whether that's emacs or emacsclient.  Maybe there's some "close
program" hook that could be used to save any stack of open minibuffers.
I just don't know the code in this area.  At least, not yet.  ;-)

> And two new regressions. These require minibuffer-follows-selected-frame
> to be set to t.

Yes.  Sorry about these.  This time around, I've tried to be more
careful and done more testing myself - hence me taking a few days longer
than I have up till now.  I've found and corrected another ~two bugs
myself.

> 1)
> C-x C-f on frame A
> select frame B
> select frame A
> Minibuffer is moved to B, but not back to A.

> 2)
> Have two frames open
> open a minibuffer on a frame
> close this frame
> The other frame does have the miniwindow selected, but the
> minibuffer isn't shown in it.

I think I've corrected these two, now.

Here's another patch, this time to be applied on top of the last patch.



--- minibuf.20210315.see	2021-03-16 10:36:40.058109894 +0000
+++ minibuf.c	2021-03-17 19:15:33.586176135 +0000
@@ -59,6 +59,12 @@
 
 static Lisp_Object minibuf_prompt;
 
+/* The frame containinug the most recently opened Minibuffer.  This is
+   used only when `minibuffer-follows-selected-frame' is neither nil
+   nor t.  */
+
+static Lisp_Object MB_frame;
+
 /* Width of current mini-buffer prompt.  Only set after display_line
    of the line that contains the prompt.  */
 
@@ -182,19 +188,17 @@
 void
 move_minibuffers_onto_frame (struct frame *of, bool for_deletion)
 {
+  struct frame *f = XFRAME (selected_frame);
+
+  minibuf_window = f->minibuffer_window;
   if (!for_deletion && (!minibuf_level || !minibuf_follows_frame ()))
     return;
-  if (FRAMEP (selected_frame)
-      && FRAME_LIVE_P (XFRAME (selected_frame))
-      && (for_deletion
-	  || !EQ (minibuf_window,
-		  XFRAME (selected_frame)->minibuffer_window)))
+  if (FRAME_LIVE_P (f)
+      && !EQ (f->minibuffer_window, of->minibuffer_window))
     {
-      struct frame *sf = XFRAME (selected_frame);
-
-      if (minibuf_follows_frame () || for_deletion)
-	zip_minibuffer_stacks (sf->minibuffer_window,
-			       of->minibuffer_window);
+      zip_minibuffer_stacks (f->minibuffer_window, of->minibuffer_window);
+      if (for_deletion && EQ (XFRAME (MB_frame), of))
+	MB_frame = selected_frame;
     }
 }
 
@@ -550,7 +554,6 @@
   Lisp_Object histval;
 
   Lisp_Object empty_minibuf;
-  Lisp_Object old_minibuf_window = minibuf_window;
 
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
@@ -632,19 +635,20 @@
   mini_frame = WINDOW_FRAME (XWINDOW (minibuf_window));
 
   if (minibuf_level > 1
-      && (!EQ (minibuf_window, old_minibuf_window))
+      && !EQ (XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame,
+	      MB_frame)
       && minibuf_moves_frame_when_opened ()
       && (!minibuf_follows_frame ()))
     {
-      Lisp_Object old_frame = XWINDOW (old_minibuf_window)->frame;
-      struct frame *of = XFRAME (old_frame);
+      struct frame *of = XFRAME (MB_frame);
 
-      zip_minibuffer_stacks (minibuf_window, old_minibuf_window);
-      /* The frame's minibuffer can be on a different frame.  */
+      zip_minibuffer_stacks (minibuf_window, of->minibuffer_window);
+      /* MB_frame's minibuffer can be on a different frame.  */
       if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (of))))
-	Fset_frame_selected_window (old_frame,
-				    Fframe_first_window (old_frame), Qnil);
+	Fset_frame_selected_window (MB_frame,
+				    Fframe_first_window (MB_frame), Qnil);
     }
+  MB_frame = XWINDOW (XFRAME (selected_frame)->minibuffer_window)->frame;
   if (live_minibuffer_p (XWINDOW (minibuf_window)->contents))
     call1 (Qrecord_window_buffer, minibuf_window);
 
@@ -1023,10 +1027,13 @@
   safe_run_hooks (Qminibuffer_exit_hook);
 }
 
-/* This variable preserves the minibuffer in the selected frame across
-   the call of restore_window_configuration.  It should be used only
-   by `read_minibuf_unwind' and `minibuffer_unwind'.  */
-static Lisp_Object selected_frame_MB;
+/* This variable records the expired minibuffer's frame between the
+   calls of `read_minibuf_unwind' and `minibuffer_unwind'.  It should
+   be used only by these two functions.  Note that the same search
+   method for the MB's frame won't always work in `minibuffer_unwind'
+   because the intervening `restore-window-configuration' will have
+   changed the buffer in the mini-window.  */
+static Lisp_Object exp_MB_frame;
 
 /* This function is called on exiting minibuffer, whether normally or
    not, and it restores the current window, buffer, etc.  */
@@ -1038,6 +1045,28 @@
   Lisp_Object calling_frame;
   Lisp_Object calling_window;
   Lisp_Object future_mini_window;
+  Lisp_Object saved_selected_frame = selected_frame;
+  Lisp_Object window, frames;
+  struct window *w;
+  struct frame *f;
+
+  /* Locate the expired minibuffer.  */
+  FOR_EACH_FRAME (frames, exp_MB_frame)
+    {
+      f = XFRAME (exp_MB_frame);
+      window = f->minibuffer_window;
+      w = XWINDOW (window);
+      if (EQ (w->frame, exp_MB_frame)
+	  && EQ (w->contents, nth_minibuffer (minibuf_level)))
+	goto found;
+    }
+  return; /* expired minibuffer not found.  Maybe we should output an
+	     error, here. */
+
+ found:
+  if (!EQ (exp_MB_frame, saved_selected_frame))
+    do_switch_frame (exp_MB_frame, 0, 0, Qt); /* This also sets
+					     minibuff_window */
 
   /* To keep things predictable, in case it matters, let's be in the
      minibuffer when we reset the relevant variables.  Don't depend on
@@ -1129,7 +1158,6 @@
      away from the expired minibuffer window, both in the current
      minibuffer's frame and the original calling frame.  */
   choose_minibuf_frame ();
-  selected_frame_MB = XWINDOW (minibuf_window)->contents;
   if (NILP (XWINDOW (minibuf_window)->prev_buffers))
     {
       if (!EQ (WINDOW_FRAME (XWINDOW (minibuf_window)), calling_frame))
@@ -1146,36 +1174,26 @@
       else
 	Fset_frame_selected_window (calling_frame, calling_window, Qnil);
     }
+
+  /* Restore the selected frame. */
+  if (!EQ (exp_MB_frame, saved_selected_frame))
+    do_switch_frame (saved_selected_frame, 0, 0, Qt);
 }
 
-/* Replace the expired minibuffer in whatever frame it is now in with
-   the next less nested minibuffer in that frame, if any.  Otherwise,
-   replace it with the null minibuffer.  MINIBUF_WINDOW is not
-   changed.  */
+/* Replace the expired minibuffer in frame exp_MB_frame with the next less
+   nested minibuffer in that frame, if any.  Otherwise, replace it
+   with the null minibuffer.  MINIBUF_WINDOW is not changed.  */
 static void
 minibuffer_unwind (void)
 {
   struct frame *f;
   struct window *w;
-  Lisp_Object window, frame, frames;
+  Lisp_Object window;
   Lisp_Object entry;
 
-  /* Locate the expired minibuffer.  */
-  FOR_EACH_FRAME (frames, frame)
-    {
-      f = XFRAME (frame);
-      window = f->minibuffer_window;
-      w = XWINDOW (window);
-      if (EQ (w->frame, frame)
-	  && EQ (EQ (frame, selected_frame)
-		 ? selected_frame_MB
-		 : w->contents,
-		 nth_minibuffer (minibuf_level + 1)))
-	goto found;
-    }
-  return; 			/* expired minibuffer not found */
-
- found:
+  f = XFRAME (exp_MB_frame);
+  window = f->minibuffer_window;
+  w = XWINDOW (window);
   if (FRAME_LIVE_P (f))
     {
       /* minibuf_window = sf->minibuffer_window; */
@@ -1188,7 +1206,7 @@
 	  Fset_window_point (window, Fcar (Fcdr (Fcdr (entry))));
 	  /* set-window-configuration may/will have unselected the
 	     mini-window as the selected window.  Restore it. */
-	  Fset_frame_selected_window (frame, window, Qnil);
+	  Fset_frame_selected_window (exp_MB_frame, window, Qnil);
 	}
       else
 	set_window_buffer (window, nth_minibuffer (0), 0, 0);
@@ -2267,7 +2285,9 @@
 {
   staticpro (&minibuf_prompt);
   staticpro (&minibuf_save_list);
-  staticpro (&selected_frame_MB);
+  staticpro (&MB_frame);
+  MB_frame = Qnil;
+  staticpro (&exp_MB_frame);
 
   DEFSYM (Qminibuffer_follows_selected_frame,
           "minibuffer-follows-selected-frame");


> Best.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-17 19:32                     ` Alan Mackenzie
@ 2021-03-17 19:55                       ` Eli Zaretskii
  2021-03-17 20:19                         ` Eli Zaretskii
  2021-03-21 15:44                       ` Miha Rihtaršič
  1 sibling, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-17 19:55 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: monnier, jakanakaevangeli, emacs-devel

> Date: Wed, 17 Mar 2021 19:32:37 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: Stefan Monnier <monnier@iro.umontreal.ca>, emacs-devel@gnu.org
> 
> > When a frame is closed, it's active minibuffer should now be moved to
> > another frame. What I wanted to test then was, what happens if we have
> > only a single frame with a minibuffer and we decide to close it, which
> > is possible with emacs daemon. Immediately, there will be no frames left
> > for the minibuffer to take refuge in and this seems to cause some
> > problems.
> 
> My apologies.  I hadn't understood what the bug is, and can confirm it's
> still there.  My patch below doesn't address it.
> 
> This could be difficult to fix.  I don't think that clicking on the last
> frame's close button goes through `delete-frame' - it just closes the
> program, whether that's emacs or emacsclient.

I'm not sure I understand completely what you say here, but clicking
on the frame's close button does invoke delete-frame, see the handling
of DELETE_FRAME_EVENT in keyboard.c.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-17 19:55                       ` Eli Zaretskii
@ 2021-03-17 20:19                         ` Eli Zaretskii
  2021-03-18 11:27                           ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-17 20:19 UTC (permalink / raw)
  To: acm; +Cc: monnier, jakanakaevangeli, emacs-devel

> Date: Wed, 17 Mar 2021 21:55:38 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> 
> > This could be difficult to fix.  I don't think that clicking on the last
> > frame's close button goes through `delete-frame' - it just closes the
> > program, whether that's emacs or emacsclient.
> 
> I'm not sure I understand completely what you say here, but clicking
> on the frame's close button does invoke delete-frame, see the handling
> of DELETE_FRAME_EVENT in keyboard.c.

And if you are talking about the last live frame of an Emacs session,
then look at handle-delete-frame, which is called when you click that
close button: it will call save-buffers-kill-emacs (which has a hook)
and then kill-emacs (which also has a hook).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-17 20:19                         ` Eli Zaretskii
@ 2021-03-18 11:27                           ` Alan Mackenzie
  2021-03-18 11:46                             ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-18 11:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Wed, Mar 17, 2021 at 22:19:10 +0200, Eli Zaretskii wrote:
> > Date: Wed, 17 Mar 2021 21:55:38 +0200
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no, emacs-devel@gnu.org

> > > This could be difficult to fix.  I don't think that clicking on the last
> > > frame's close button goes through `delete-frame' - it just closes the
> > > program, whether that's emacs or emacsclient.

> > I'm not sure I understand completely what you say here, but clicking
> > on the frame's close button does invoke delete-frame, see the handling
> > of DELETE_FRAME_EVENT in keyboard.c.

> And if you are talking about the last live frame of an Emacs session,
> then look at handle-delete-frame, which is called when you click that
> close button: it will call save-buffers-kill-emacs (which has a hook)
> and then kill-emacs (which also has a hook).

Thank you, that is very helpful.  I was indeed talking about the last
live frame, specifically in an emacsclient session when the Emacs daemon
is running.

The problem that Miha highlighted is that when there is an open
minibuffer when that last frame is deleted, that open minibuffer remains
in existence, fouling up the next emacsclient session.

Maybe such open minibuffers should just be aborted (along with any other
recursive edits) when the last frame gets deleted.  This would be
simpler to code than preserving those minibuffers somewhere, and
restoring them at the next emacsclient session.  Aborting them also
seems more natural, since their contents are unlikely to have any
relevance to the next emacsclient session.

Incidentally, there doesn't seem to be anything in the Elisp manual
about the handling of Emacs daemon sessions.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-18 11:27                           ` Alan Mackenzie
@ 2021-03-18 11:46                             ` Eli Zaretskii
  2021-03-18 15:51                               ` martin rudalics
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-18 11:46 UTC (permalink / raw)
  To: Alan Mackenzie, martin rudalics; +Cc: monnier, jakanakaevangeli, emacs-devel

> Date: Thu, 18 Mar 2021 11:27:18 +0000
> Cc: monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> The problem that Miha highlighted is that when there is an open
> minibuffer when that last frame is deleted, that open minibuffer remains
> in existence, fouling up the next emacsclient session.
> 
> Maybe such open minibuffers should just be aborted (along with any other
> recursive edits) when the last frame gets deleted.  This would be
> simpler to code than preserving those minibuffers somewhere, and
> restoring them at the next emacsclient session.  Aborting them also
> seems more natural, since their contents are unlikely to have any
> relevance to the next emacsclient session.

Martin, can you comment on this, please?

> Incidentally, there doesn't seem to be anything in the Elisp manual
> about the handling of Emacs daemon sessions.

Please be more specific: which aspects of the daemon sessions would
you like to be described that aren't already described?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-18 11:46                             ` Eli Zaretskii
@ 2021-03-18 15:51                               ` martin rudalics
  2021-03-18 16:58                                 ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: martin rudalics @ 2021-03-18 15:51 UTC (permalink / raw)
  To: Eli Zaretskii, Alan Mackenzie; +Cc: monnier, jakanakaevangeli, emacs-devel

 >> The problem that Miha highlighted is that when there is an open
 >> minibuffer when that last frame is deleted, that open minibuffer remains
 >> in existence, fouling up the next emacsclient session.
 >>
 >> Maybe such open minibuffers should just be aborted (along with any other
 >> recursive edits) when the last frame gets deleted.  This would be
 >> simpler to code than preserving those minibuffers somewhere, and
 >> restoring them at the next emacsclient session.  Aborting them also
 >> seems more natural, since their contents are unlikely to have any
 >> relevance to the next emacsclient session.
 >
 > Martin, can you comment on this, please?

What about answering questions about unsaved buffers, running processes
...  in such a situation?  I never use emacsclient so I have no idea how
this should behave in practice.  But any non-crucial dialog should be
aborted IMHO.

martin





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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-18 15:51                               ` martin rudalics
@ 2021-03-18 16:58                                 ` Alan Mackenzie
  2021-03-18 18:44                                   ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-18 16:58 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, monnier, jakanakaevangeli, emacs-devel

Hello, Martin and Eli.

On Thu, Mar 18, 2021 at 16:51:32 +0100, martin rudalics wrote:
>  >> The problem that Miha highlighted is that when there is an open
>  >> minibuffer when that last frame is deleted, that open minibuffer remains
>  >> in existence, fouling up the next emacsclient session.
>  >>
>  >> Maybe such open minibuffers should just be aborted (along with any other
>  >> recursive edits) when the last frame gets deleted.  This would be
>  >> simpler to code than preserving those minibuffers somewhere, and
>  >> restoring them at the next emacsclient session.  Aborting them also
>  >> seems more natural, since their contents are unlikely to have any
>  >> relevance to the next emacsclient session.
>  >
>  > Martin, can you comment on this, please?

> What about answering questions about unsaved buffers, running processes
> ...  in such a situation?  I never use emacsclient so I have no idea how
> this should behave in practice.

I don't use emacsclient either.  But questions about unsaved buffers
seem to prevent Emacs terminating until they get answered.  The same for
running processes (at least, for gdb).

> But any non-crucial dialog should be aborted IMHO.

Are the any uses of the minibuffer which count as crucial, in this
sense?

> martin

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-18 16:58                                 ` Alan Mackenzie
@ 2021-03-18 18:44                                   ` Eli Zaretskii
  2021-03-19 11:40                                     ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-18 18:44 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

> Date: Thu, 18 Mar 2021 16:58:25 +0000
> Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca,
>   jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> >  >> Maybe such open minibuffers should just be aborted (along with any other
> >  >> recursive edits) when the last frame gets deleted.  This would be
> >  >> simpler to code than preserving those minibuffers somewhere, and
> >  >> restoring them at the next emacsclient session.  Aborting them also
> >  >> seems more natural, since their contents are unlikely to have any
> >  >> relevance to the next emacsclient session.
> >  >
> >  > Martin, can you comment on this, please?
> 
> > What about answering questions about unsaved buffers, running processes
> > ...  in such a situation?  I never use emacsclient so I have no idea how
> > this should behave in practice.
> 
> I don't use emacsclient either.  But questions about unsaved buffers
> seem to prevent Emacs terminating until they get answered.  The same for
> running processes (at least, for gdb).

Are we shutting down Emacs, or are we returning to a headless daemon
state?  If the former, we need to ask all those questions before we
shut down Emacs.  Why is it a problem to ask them?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-18 18:44                                   ` Eli Zaretskii
@ 2021-03-19 11:40                                     ` Alan Mackenzie
  2021-03-19 12:33                                       ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-19 11:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Thu, Mar 18, 2021 at 20:44:12 +0200, Eli Zaretskii wrote:
> > Date: Thu, 18 Mar 2021 16:58:25 +0000
> > Cc: Eli Zaretskii <eliz@gnu.org>, monnier@iro.umontreal.ca,
> >   jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > >  >> Maybe such open minibuffers should just be aborted (along with any other
> > >  >> recursive edits) when the last frame gets deleted.  This would be
> > >  >> simpler to code than preserving those minibuffers somewhere, and
> > >  >> restoring them at the next emacsclient session.  Aborting them also
> > >  >> seems more natural, since their contents are unlikely to have any
> > >  >> relevance to the next emacsclient session.

> > >  > Martin, can you comment on this, please?

> > > What about answering questions about unsaved buffers, running processes
> > > ...  in such a situation?  I never use emacsclient so I have no idea how
> > > this should behave in practice.

> > I don't use emacsclient either.  But questions about unsaved buffers
> > seem to prevent Emacs terminating until they get answered.  The same for
> > running processes (at least, for gdb).

> Are we shutting down Emacs, or are we returning to a headless daemon
> state?

Returning to a headless daemon state.

> If the former, we need to ask all those questions before we shut down
> Emacs.  Why is it a problem to ask them?

Sorry, I didn't mean to imply these things were problems; it was just an
observation, comparing the question about unsaved buffers with the lack
of a question on open minibuffers.

In the following, I'm using my not yet committed version of minibuf.c,
etc.:

When I now try Miha's recipe:
   (In last frame of emacsclient:)
   M-: (run-at-time 10 nil #'make-frame '((window-system . x)))
   C-x C-f foo
   Close the frame by clicking on its close button
   <Wait the remainder of the 10 seconds>

, the new frame comes up _with_ the minibuffer visible.  I don't
understand how the mini window survived with its contents, and it
worries me.  The minibuffers are stored in the frames'
->minibuffer_window components, as well as existing as buffers in their
own right.  When the last frame is deleted, I would expect the MB to
survive, but NOT to be put back into the mini window of the newly
created frame.

Could it be that the last frame isn't actually deleted from the Emacs
daemon when clicking on its close button?  Merely that the emacsclient
program is closed, leaving its window inactive/invisible/whatever?  In
fact running M-: (frame-list) shows two frames, one of which is called "
*Minibuf-1*", though it isn't displayed in X-Windows.

This is the sort of thing I would like to be able to read about in the
Elisp manual, along with basic questions like "How can one test whether
one is in an emacsclient session rather than an ordinary Emacs?".

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-19 11:40                                     ` Alan Mackenzie
@ 2021-03-19 12:33                                       ` Eli Zaretskii
  2021-03-19 15:35                                         ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-19 12:33 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

> Date: Fri, 19 Mar 2021 11:40:52 +0000
> Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
>   emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > > What about answering questions about unsaved buffers, running processes
> > > > ...  in such a situation?  I never use emacsclient so I have no idea how
> > > > this should behave in practice.
> 
> > > I don't use emacsclient either.  But questions about unsaved buffers
> > > seem to prevent Emacs terminating until they get answered.  The same for
> > > running processes (at least, for gdb).
> 
> > Are we shutting down Emacs, or are we returning to a headless daemon
> > state?
> 
> Returning to a headless daemon state.

Then I think there's no need to ask those questions at all.  Emacs is
not going away, so it's okay to leave unsaved edits in the session
that continues to run.

> When I now try Miha's recipe:
>    (In last frame of emacsclient:)
>    M-: (run-at-time 10 nil #'make-frame '((window-system . x)))
>    C-x C-f foo
>    Close the frame by clicking on its close button
>    <Wait the remainder of the 10 seconds>
> 
> , the new frame comes up _with_ the minibuffer visible.  I don't
> understand how the mini window survived with its contents, and it
> worries me.

Why does it worry you?  You've asked Emacs to create a frame, and a
frame by default has a mini-window.  Right?

> Could it be that the last frame isn't actually deleted from the Emacs
> daemon when clicking on its close button?

I think indeed that's what happens, because otherwise clicking there
would have shut down Emacs -- which it doesn't in the daemon case.

> This is the sort of thing I would like to be able to read about in the
> Elisp manual

You expect the ELisp manual to describe the internals to this detail?
That's stuff for comments in the code (adding which will be very
welcome), not for the ELisp reference.

> along with basic questions like "How can one test whether one is in
> an emacsclient session rather than an ordinary Emacs?".

This seems to be a matter of improving indexing: see the variable
mode-line-client and how it is computed.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-19 12:33                                       ` Eli Zaretskii
@ 2021-03-19 15:35                                         ` Alan Mackenzie
  2021-03-19 15:59                                           ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-19 15:35 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Fri, Mar 19, 2021 at 14:33:20 +0200, Eli Zaretskii wrote:
> > Date: Fri, 19 Mar 2021 11:40:52 +0000
> > Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
> >   emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > > What about answering questions about unsaved buffers, running processes
> > > > > ...  in such a situation?  I never use emacsclient so I have no idea how
> > > > > this should behave in practice.

> > > > I don't use emacsclient either.  But questions about unsaved buffers
> > > > seem to prevent Emacs terminating until they get answered.  The same for
> > > > running processes (at least, for gdb).

> > > Are we shutting down Emacs, or are we returning to a headless daemon
> > > state?

> > Returning to a headless daemon state.

> Then I think there's no need to ask those questions at all.  Emacs is
> not going away, so it's okay to leave unsaved edits in the session
> that continues to run.

Yes, I think that's right.

> > When I now try Miha's recipe:
> >    (In last frame of emacsclient:)
> >    M-: (run-at-time 10 nil #'make-frame '((window-system . x)))
> >    C-x C-f foo
> >    Close the frame by clicking on its close button
> >    <Wait the remainder of the 10 seconds>

> > , the new frame comes up _with_ the minibuffer visible.  I don't
> > understand how the mini window survived with its contents, and it
> > worries me.

> Why does it worry you?  You've asked Emacs to create a frame, and a
> frame by default has a mini-window.  Right?

I understand it a lot better now.  The new mini-window gets the
minibuffers only because I had minibuffer-follows-selected-frame set to
t.  With either of the other values, the minibuffers remain on the old
inaccessible frame.  This is not good.

> > Could it be that the last frame isn't actually deleted from the Emacs
> > daemon when clicking on its close button?

> I think indeed that's what happens, because otherwise clicking there
> would have shut down Emacs -- which it doesn't in the daemon case.

Yes, this is the case.  I would dearly like to find a way of determining
if this last frame is still visible, or iconified, or whatever, as
opposed to inaccessible.  The old frame in the above recipe was a -nw
frame, effectively a TTY, and `frame-visible-p' returns t for it, even
though it is not even displayable (it's containing shell has gone).

> > This is the sort of thing I would like to be able to read about in the
> > Elisp manual

> You expect the ELisp manual to describe the internals to this detail?
> That's stuff for comments in the code (adding which will be very
> welcome), not for the ELisp reference.

Hmm.  There is effectively nothing about the effect of Emacs as a daemon
in Elisp.

> > along with basic questions like "How can one test whether one is in
> > an emacsclient session rather than an ordinary Emacs?".

> This seems to be a matter of improving indexing: see the variable
> mode-line-client and how it is computed.

OK, thanks.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-19 15:35                                         ` Alan Mackenzie
@ 2021-03-19 15:59                                           ` Eli Zaretskii
  2021-03-20 10:28                                             ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-19 15:59 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

> Date: Fri, 19 Mar 2021 15:35:13 +0000
> Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
>   emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > > , the new frame comes up _with_ the minibuffer visible.  I don't
> > > understand how the mini window survived with its contents, and it
> > > worries me.
> 
> > Why does it worry you?  You've asked Emacs to create a frame, and a
> > frame by default has a mini-window.  Right?
> 
> I understand it a lot better now.  The new mini-window gets the
> minibuffers only because I had minibuffer-follows-selected-frame set to
> t.  With either of the other values, the minibuffers remain on the old
> inaccessible frame.  This is not good.

Can't you move them from those inaccessible frames to this one?

> > > Could it be that the last frame isn't actually deleted from the Emacs
> > > daemon when clicking on its close button?
> 
> > I think indeed that's what happens, because otherwise clicking there
> > would have shut down Emacs -- which it doesn't in the daemon case.
> 
> Yes, this is the case.  I would dearly like to find a way of determining
> if this last frame is still visible, or iconified, or whatever, as
> opposed to inaccessible.

I think this is the initial frame that exists in every Emacs session
when it starts, except that in the daemon we don't delete it.  It's a
non-GUI frame which exists just to keep code that expects some frame
to exist happy.

> The old frame in the above recipe was a -nw frame, effectively a
> TTY, and `frame-visible-p' returns t for it, even though it is not
> even displayable (it's containing shell has gone).

We don't have a means of knowing whether a TTY frame is displayed, it
conceptually always is.

> Hmm.  There is effectively nothing about the effect of Emacs as a daemon
> in Elisp.

There's nothing special about that, just some implementation details.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-19 15:59                                           ` Eli Zaretskii
@ 2021-03-20 10:28                                             ` Alan Mackenzie
  2021-03-20 10:49                                               ` Eli Zaretskii
  0 siblings, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-20 10:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Fri, Mar 19, 2021 at 17:59:45 +0200, Eli Zaretskii wrote:
> > Date: Fri, 19 Mar 2021 15:35:13 +0000
> > Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
> >   emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > > , the new frame comes up _with_ the minibuffer visible.  I don't
> > > > understand how the mini window survived with its contents, and it
> > > > worries me.

> > > Why does it worry you?  You've asked Emacs to create a frame, and a
> > > frame by default has a mini-window.  Right?

> > I understand it a lot better now.  The new mini-window gets the
> > minibuffers only because I had minibuffer-follows-selected-frame set to
> > t.  With either of the other values, the minibuffers remain on the old
> > inaccessible frame.  This is not good.

> Can't you move them from those inaccessible frames to this one?

The actual moving of the minibuffers isn't the problem ....

> > > > Could it be that the last frame isn't actually deleted from the Emacs
> > > > daemon when clicking on its close button?

> > > I think indeed that's what happens, because otherwise clicking there
> > > would have shut down Emacs -- which it doesn't in the daemon case.

> > Yes, this is the case.  I would dearly like to find a way of determining
> > if this last frame is still visible, or iconified, or whatever, as
> > opposed to inaccessible.

> I think this is the initial frame that exists in every Emacs session
> when it starts, except that in the daemon we don't delete it.  It's a
> non-GUI frame which exists just to keep code that expects some frame
> to exist happy.

This initial frame gets used as a normal frame when one calls

    $ emacsclient foo

for the first time.  It stays in use until one clicks on its close
button (I think this is actually the close button of the containing
xterm).  When a new GUI frame is brought up by

    M-: (run-at-time 10 nil #'make-frame '((window-system . x)))

, the initial frame may or may not have been "closed" by clicking its
close button.  I think you're telling me that it's not possible to
distinguish these two cases.  If so, that's surely a defect in Emacs.

> > The old frame in the above recipe was a -nw frame, effectively a
> > TTY, and `frame-visible-p' returns t for it, even though it is not
> > even displayable (it's containing shell has gone).

> We don't have a means of knowing whether a TTY frame is displayed, it
> conceptually always is.

Perhaps we should create some means of knowing this?  Maybe in
handle-delete-frame, where Emacs has determined there is only one frame
left, before calling save-buffers-kill-emacs we could mark the last
frame's f->visible to "not visible".  Or maybe set up a special non-zero
dummy value for f->terminal.  Or something like that.

> > Hmm.  There is effectively nothing about the effect of Emacs as a daemon
> > in Elisp.

> There's nothing special about that, just some implementation details.

OK.  I'm not sure I agree, but it's not really a big point.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 10:28                                             ` Alan Mackenzie
@ 2021-03-20 10:49                                               ` Eli Zaretskii
  2021-03-20 12:24                                                 ` Alan Mackenzie
  0 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-20 10:49 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

> Date: Sat, 20 Mar 2021 10:28:26 +0000
> Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
>   emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > I think this is the initial frame that exists in every Emacs session
> > when it starts, except that in the daemon we don't delete it.  It's a
> > non-GUI frame which exists just to keep code that expects some frame
> > to exist happy.
> 
> This initial frame gets used as a normal frame when one calls
> 
>     $ emacsclient foo
> 
> for the first time.

Are you sure?  ISTR that we create a new frame on the terminal from
which emacsclient was invoked.  The initial frame is never used except
at startup and when the daemon should do something while it has no
clients.

> It stays in use until one clicks on its close
> button (I think this is actually the close button of the containing
> xterm).  When a new GUI frame is brought up by
> 
>     M-: (run-at-time 10 nil #'make-frame '((window-system . x)))
> 
> , the initial frame may or may not have been "closed" by clicking its
> close button.  I think you're telling me that it's not possible to
> distinguish these two cases.  If so, that's surely a defect in Emacs.

No, I'm saying that the initial frame is never deleted.  At least
that's my recollection.

> > We don't have a means of knowing whether a TTY frame is displayed, it
> > conceptually always is.
> 
> Perhaps we should create some means of knowing this?  Maybe in
> handle-delete-frame, where Emacs has determined there is only one frame
> left, before calling save-buffers-kill-emacs we could mark the last
> frame's f->visible to "not visible".

You mean, for a frame that is about to be deleted anyway? what purpose
would that serve?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 10:49                                               ` Eli Zaretskii
@ 2021-03-20 12:24                                                 ` Alan Mackenzie
  2021-03-20 12:49                                                   ` Miha Rihtaršič
                                                                     ` (2 more replies)
  0 siblings, 3 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-20 12:24 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Sat, Mar 20, 2021 at 12:49:05 +0200, Eli Zaretskii wrote:
> > Date: Sat, 20 Mar 2021 10:28:26 +0000
> > Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
> >   emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > I think this is the initial frame that exists in every Emacs session
> > > when it starts, except that in the daemon we don't delete it.  It's a
> > > non-GUI frame which exists just to keep code that expects some frame
> > > to exist happy.

> > This initial frame gets used as a normal frame when one calls

> >     $ emacsclient foo

> > for the first time.

> Are you sure?  ISTR that we create a new frame on the terminal from
> which emacsclient was invoked.  The initial frame is never used except
> at startup and when the daemon should do something while it has no
> clients.

Apologies, I was wrong.  On the first emacsclient call, a second frame
gets created.  The initial frame is marked in some way so that, e.g., C-x
5 o doesn't see it.

> > It stays in use until one clicks on its close
> > button (I think this is actually the close button of the containing
> > xterm).  When a new GUI frame is brought up by

> >     M-: (run-at-time 10 nil #'make-frame '((window-system . x)))

> > , the initial frame may or may not have been "closed" by clicking its
> > close button.  I think you're telling me that it's not possible to
> > distinguish these two cases.  If so, that's surely a defect in Emacs.

> No, I'm saying that the initial frame is never deleted.  At least
> that's my recollection.

So, when a new frame is created (possibly with emacsclient), and there
are now exactly two frames, we want to copy any minibuffers from the
other frame to the new one when that other frame is the initial frame.

I still can't find a way of identifying the initial frame for sure - it
lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
lacks a 'client frame parameter, but so do ordinary frames when Emacs is
started normally and M-x server-start invoked.

Would it, perhaps, be useful to add another frame parameter
'initial-frame to identify this frame?

[ .... ]

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:24                                                 ` Alan Mackenzie
@ 2021-03-20 12:49                                                   ` Miha Rihtaršič
  2021-03-20 13:59                                                     ` Stefan Monnier
  2021-03-21 10:30                                                     ` Alan Mackenzie
  2021-03-20 12:50                                                   ` Eli Zaretskii
  2021-03-20 13:55                                                   ` Stefan Monnier
  2 siblings, 2 replies; 52+ messages in thread
From: Miha Rihtaršič @ 2021-03-20 12:49 UTC (permalink / raw)
  To: Alan Mackenzie, Eli Zaretskii; +Cc: emacs-devel

Alan Mackenzie <acm@muc.de> writes:

>> No, I'm saying that the initial frame is never deleted.  At least
>> that's my recollection.
>
> So, when a new frame is created (possibly with emacsclient), and there
> are now exactly two frames, we want to copy any minibuffers from the
> other frame to the new one when that other frame is the initial frame.

Just giving a heads up that trouble may arise when moving a minibuffer
from one terminal to another (from tty to X for example).
This is judging from the following comment that I stumbled upon when
reading function `server-goto-toplevel':

;; We're inside a minibuffer already, so if the emacs-client is trying
;; to open a frame on a new display, we might end up with an unusable
;; frame because input from that display will be blocked (until exiting
;; the minibuffer).  Better exit this minibuffer right away.

`emacsclient ~/foo' causes a throw to top-level in most cases before
spawning a new frame to avoid some trouble, but I'm not sure if this
trouble applies for our case.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:24                                                 ` Alan Mackenzie
  2021-03-20 12:49                                                   ` Miha Rihtaršič
@ 2021-03-20 12:50                                                   ` Eli Zaretskii
  2021-03-20 13:51                                                     ` Alan Mackenzie
  2021-03-20 13:55                                                   ` Stefan Monnier
  2 siblings, 1 reply; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-20 12:50 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

> Date: Sat, 20 Mar 2021 12:24:55 +0000
> Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
>   emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> So, when a new frame is created (possibly with emacsclient), and there
> are now exactly two frames, we want to copy any minibuffers from the
> other frame to the new one when that other frame is the initial frame.
> 
> I still can't find a way of identifying the initial frame for sure - it
> lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
> lacks a 'client frame parameter, but so do ordinary frames when Emacs is
> started normally and M-x server-start invoked.
> 
> Would it, perhaps, be useful to add another frame parameter
> 'initial-frame to identify this frame?

Can you tell how you are trying to "identify this frame"?  I'm afraid
I don't see what is your difficulties in this regard.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:50                                                   ` Eli Zaretskii
@ 2021-03-20 13:51                                                     ` Alan Mackenzie
  0 siblings, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-20 13:51 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: rudalics, monnier, jakanakaevangeli, emacs-devel

Hello, Eli.

On Sat, Mar 20, 2021 at 14:50:19 +0200, Eli Zaretskii wrote:
> > Date: Sat, 20 Mar 2021 12:24:55 +0000
> > Cc: rudalics@gmx.at, monnier@iro.umontreal.ca, jakanakaevangeli@chiru.no,
> >   emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > So, when a new frame is created (possibly with emacsclient), and there
> > are now exactly two frames, we want to copy any minibuffers from the
> > other frame to the new one when that other frame is the initial frame.

> > I still can't find a way of identifying the initial frame for sure - it
> > lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
> > lacks a 'client frame parameter, but so do ordinary frames when Emacs is
> > started normally and M-x server-start invoked.

> > Would it, perhaps, be useful to add another frame parameter
> > 'initial-frame to identify this frame?

> Can you tell how you are trying to "identify this frame"?  I'm afraid
> I don't see what is your difficulties in this regard.

The problem is that I can't come up with any way to identify the initial
frame reliably.  I can't see how to write

    bool initial_frame_p (struct frame *f) { .... }

That's why I suggested a new frame property.

Do you know of any means to identify this frame?  If so, please tell me
about it!  Thanks!

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:24                                                 ` Alan Mackenzie
  2021-03-20 12:49                                                   ` Miha Rihtaršič
  2021-03-20 12:50                                                   ` Eli Zaretskii
@ 2021-03-20 13:55                                                   ` Stefan Monnier
  2021-03-20 14:01                                                     ` Eli Zaretskii
  2021-03-20 14:12                                                     ` Alan Mackenzie
  2 siblings, 2 replies; 52+ messages in thread
From: Stefan Monnier @ 2021-03-20 13:55 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: rudalics, Eli Zaretskii, jakanakaevangeli, emacs-devel

> I still can't find a way of identifying the initial frame for sure - it
> lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
> lacks a 'client frame parameter, but so do ordinary frames when Emacs is
> started normally and M-x server-start invoked.

xdisp.c uses

    FRAME_INITIAL_P (SELECTED_FRAME ())


-- Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:49                                                   ` Miha Rihtaršič
@ 2021-03-20 13:59                                                     ` Stefan Monnier
  2021-03-21 10:30                                                     ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Stefan Monnier @ 2021-03-20 13:59 UTC (permalink / raw)
  To: Miha Rihtaršič; +Cc: Alan Mackenzie, Eli Zaretskii, emacs-devel

> Just giving a heads up that trouble may arise when moving a minibuffer
> from one terminal to another (from tty to X for example).

Actually that comment from server.el is not about the difficulty of
moving minibuffers but about the problem caused by the lack of minibuffer
movement: if a minibuffer is active in terminal 1, then Emacs is in
a special mode where input from other terminals is ignored, so an
emacsclient opening a frame on a new terminal will display a "dead"
frame (in the sense that it doesn't respond to user input until the
minibuffer in terminal 1 is exited).

Alan's recent changes could actually be used to improve that behavior of
server.el by moving the active minibuffer(s) to the new frame instead of
aborting those minibuffers.


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 13:55                                                   ` Stefan Monnier
@ 2021-03-20 14:01                                                     ` Eli Zaretskii
  2021-03-20 14:12                                                     ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-20 14:01 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: acm, emacs-devel, jakanakaevangeli, rudalics

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Sat, 20 Mar 2021 09:55:02 -0400
> Cc: rudalics@gmx.at, Eli Zaretskii <eliz@gnu.org>, jakanakaevangeli@chiru.no,
>  emacs-devel@gnu.org
> 
> > I still can't find a way of identifying the initial frame for sure - it
> > lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
> > lacks a 'client frame parameter, but so do ordinary frames when Emacs is
> > started normally and M-x server-start invoked.
> 
> xdisp.c uses
> 
>     FRAME_INITIAL_P (SELECTED_FRAME ())

Right.  Just use a frame pointer argument instead of SELECTED_FRAME(),
i.e.

   struct frame *f;
   ...
   if (FRAME_INITIAL_P (f))
     ...



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 13:55                                                   ` Stefan Monnier
  2021-03-20 14:01                                                     ` Eli Zaretskii
@ 2021-03-20 14:12                                                     ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-20 14:12 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: rudalics, Eli Zaretskii, jakanakaevangeli, emacs-devel

Hello, Stefan.

On Sat, Mar 20, 2021 at 09:55:02 -0400, Stefan Monnier wrote:
> > I still can't find a way of identifying the initial frame for sure - it
> > lacks a 'display frame parameter, but so do ordinary frames on a tty.  It
> > lacks a 'client frame parameter, but so do ordinary frames when Emacs is
> > started normally and M-x server-start invoked.

> xdisp.c uses

>     FRAME_INITIAL_P (SELECTED_FRAME ())

Thanks, that's exactly what I need.

> -- Stefan

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-20 12:49                                                   ` Miha Rihtaršič
  2021-03-20 13:59                                                     ` Stefan Monnier
@ 2021-03-21 10:30                                                     ` Alan Mackenzie
  2021-03-21 10:38                                                       ` Eli Zaretskii
  1 sibling, 1 reply; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-21 10:30 UTC (permalink / raw)
  To: Miha Rihtaršič; +Cc: Eli Zaretskii, emacs-devel

Hello, Miha.

On Sat, Mar 20, 2021 at 13:49:19 +0100, Miha Rihtaršič wrote:
> Alan Mackenzie <acm@muc.de> writes:

> >> No, I'm saying that the initial frame is never deleted.  At least
> >> that's my recollection.

> > So, when a new frame is created (possibly with emacsclient), and there
> > are now exactly two frames, we want to copy any minibuffers from the
> > other frame to the new one when that other frame is the initial frame.

> Just giving a heads up that trouble may arise when moving a minibuffer
> from one terminal to another (from tty to X for example).
> This is judging from the following comment that I stumbled upon when
> reading function `server-goto-toplevel':

Thanks.

> ;; We're inside a minibuffer already, so if the emacs-client is trying
> ;; to open a frame on a new display, we might end up with an unusable
> ;; frame because input from that display will be blocked (until exiting
> ;; the minibuffer).  Better exit this minibuffer right away.

That looks like a bug which ought to be fixed, but I've no idea where to
start looking for the problem.

> `emacsclient ~/foo' causes a throw to top-level in most cases before
> spawning a new frame to avoid some trouble, but I'm not sure if this
> trouble applies for our case.

When starting emacsclient, I've seen minibuffers being preserved, but I
think I've also seen them being aborted.  By trouble, I think you mean
that described in the comment above.

On a slightly different note, do you see any reason not to commit the
latest state of src/minibuf.c etc., as amended by my patch from
Wednesday?

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 10:30                                                     ` Alan Mackenzie
@ 2021-03-21 10:38                                                       ` Eli Zaretskii
  2021-03-21 10:40                                                         ` Eli Zaretskii
  2021-03-21 14:49                                                         ` Alan Mackenzie
  0 siblings, 2 replies; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-21 10:38 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

> Date: Sun, 21 Mar 2021 10:30:39 +0000
> From: Alan Mackenzie <acm@muc.de>
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> > ;; We're inside a minibuffer already, so if the emacs-client is trying
> > ;; to open a frame on a new display, we might end up with an unusable
> > ;; frame because input from that display will be blocked (until exiting
> > ;; the minibuffer).  Better exit this minibuffer right away.
> 
> That looks like a bug which ought to be fixed, but I've no idea where to
> start looking for the problem.

It's a "feature" we only allow a single frame to be in the "input"
state at any given time.

> > `emacsclient ~/foo' causes a throw to top-level in most cases before
> > spawning a new frame to avoid some trouble, but I'm not sure if this
> > trouble applies for our case.
> 
> When starting emacsclient, I've seen minibuffers being preserved, but I
> think I've also seen them being aborted.  By trouble, I think you mean
> that described in the comment above.

I confess that I don't have a clear idea of what is the problem you
are trying to solve in the context of this discussion.  Why do you
want to "move" the minibuffer when emacsclient starts a new frame (or
is it when it closes a frame?), and what does "move" mean in this
context?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 10:38                                                       ` Eli Zaretskii
@ 2021-03-21 10:40                                                         ` Eli Zaretskii
  2021-03-21 14:49                                                         ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-21 10:40 UTC (permalink / raw)
  To: acm; +Cc: jakanakaevangeli, emacs-devel

> Date: Sun, 21 Mar 2021 12:38:34 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> 
> > Date: Sun, 21 Mar 2021 10:30:39 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> > 
> > > ;; We're inside a minibuffer already, so if the emacs-client is trying
> > > ;; to open a frame on a new display, we might end up with an unusable
> > > ;; frame because input from that display will be blocked (until exiting
> > > ;; the minibuffer).  Better exit this minibuffer right away.
> > 
> > That looks like a bug which ought to be fixed, but I've no idea where to
> > start looking for the problem.
> 
> It's a "feature" we only allow a single frame to be in the "input"
> state at any given time.

Sorry, my sloppy wording made this misleading.  Instead of "frame"
please read "display" or "terminal".



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 10:38                                                       ` Eli Zaretskii
  2021-03-21 10:40                                                         ` Eli Zaretskii
@ 2021-03-21 14:49                                                         ` Alan Mackenzie
  2021-03-21 15:00                                                           ` Stefan Monnier
  2021-03-21 15:43                                                           ` Eli Zaretskii
  1 sibling, 2 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-21 14:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jakanakaevangeli, emacs-devel

[ "frame" replaced by "terminal" where appropriate ]

Hello, Eli.

On Sun, Mar 21, 2021 at 12:38:34 +0200, Eli Zaretskii wrote:
> > Date: Sun, 21 Mar 2021 10:30:39 +0000
> > From: Alan Mackenzie <acm@muc.de>
> > Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org

> > > ;; We're inside a minibuffer already, so if the emacs-client is trying
> > > ;; to open a frame on a new display, we might end up with an unusable
> > > ;; frame because input from that display will be blocked (until exiting
> > > ;; the minibuffer).  Better exit this minibuffer right away.

> > That looks like a bug which ought to be fixed, but I've no idea where to
> > start looking for the problem.

> It's a "feature" we only allow a single terminal to be in the "input"
> state at any given time.

Er, OK.  There surely must be good reasons for this.

> > > `emacsclient ~/foo' causes a throw to top-level in most cases before
> > > spawning a new frame to avoid some trouble, but I'm not sure if this
> > > trouble applies for our case.

> > When starting emacsclient, I've seen minibuffers being preserved, but I
> > think I've also seen them being aborted.  By trouble, I think you mean
> > that described in the comment above.

> I confess that I don't have a clear idea of what is the problem you
> are trying to solve in the context of this discussion.  Why do you
> want to "move" the minibuffer when emacsclient starts a new frame (or
> is it when it closes a frame?), and what does "move" mean in this
> context?

The following description includes things which aren't yet committed.

"Move" means to move the minibuffer from frame F1 to frame F2, as for
example, C-x 5 o requires when minibuffer-follows-selected-frame is t
(the default).  When enable-recursive-minibuffers is t, there can be a
"stack" of minibuffers on a frame.  The entire stack gets moved.  The top
minibuffer in the stack is at f->minibuffer_window, any lower elements
are stored in w->prev_buffers, where w is XWINDOW (f->minibuffer_window).

It is also required to move minibuffers when the frame containing them
gets deleted.  Otherwise the minibuffers would continue to be active, yet
be inaccessible from any frame - this would be undesirable.

When the (second) last frame in a --daemon Emacs gets deleted, any
minibuffers it contains get moved to the initial frame.  When calling
emacsclient opens a new frame, these minibuffers need to be moved onto
that new frame (or else be aborted, somehow).

The problem I'm trying to solve here is to understand what happens when
emacsclient opens a frame on a different terminal from where emacs
--daemon was started, when there are active minibuffers on the original
terminal.  What would be nice would be for these minibuffers to be moved
onto the new frame (when minibuffer-follows-selected-frame is t) or left
on the other non-initial frame (otherwise).  It appears, from Miha's
observation yesterday, that any active minibuffers would get aborted in
this case, to prevent the old frame blocking the new one.

It may well be that further work in this area isn't worthwhile - just how
often is somebody going to create a frame in a different terminal whilst
a minibuffer is active, anyway?

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 14:49                                                         ` Alan Mackenzie
@ 2021-03-21 15:00                                                           ` Stefan Monnier
  2021-03-21 15:43                                                           ` Eli Zaretskii
  1 sibling, 0 replies; 52+ messages in thread
From: Stefan Monnier @ 2021-03-21 15:00 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Eli Zaretskii, jakanakaevangeli, emacs-devel

> The problem I'm trying to solve here is to understand what happens when
> emacsclient opens a frame on a different terminal from where emacs
> --daemon was started, when there are active minibuffers on the original
> terminal.  What would be nice would be for these minibuffers to be moved
> onto the new frame (when minibuffer-follows-selected-frame is t) or left
> on the other non-initial frame (otherwise).

Leaving them on the other non-initial frame is OK if the new frame is on
the same display.  If it is on another display it can be a problem
because that other display may be inaccessible to the user at
that moment.

That's why I added `server-goto-toplevel`.

> It appears, from Miha's observation yesterday, that any active
> minibuffers would get aborted in this case, to prevent the old frame
> blocking the new one.

Of course, if you could move the active minibuffers to the new frame
(when minibuffer-follows-selected-frame is t), then we could change
`server-goto-toplevel` so as not to abort them.

> It may well be that further work in this area isn't worthwhile - just how
> often is somebody going to create a frame in a different terminal whilst
> a minibuffer is active, anyway?

I implemented `server-goto-toplevel` because it happened to me quite
a few times ;-)


        Stefan




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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 14:49                                                         ` Alan Mackenzie
  2021-03-21 15:00                                                           ` Stefan Monnier
@ 2021-03-21 15:43                                                           ` Eli Zaretskii
  2021-03-21 16:17                                                             ` Michael Welsh Duggan
  2021-03-21 16:37                                                             ` Alan Mackenzie
  1 sibling, 2 replies; 52+ messages in thread
From: Eli Zaretskii @ 2021-03-21 15:43 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: jakanakaevangeli, emacs-devel

> Date: Sun, 21 Mar 2021 14:49:56 +0000
> Cc: jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> From: Alan Mackenzie <acm@muc.de>
> 
> > It's a "feature" we only allow a single terminal to be in the "input"
> > state at any given time.
> 
> Er, OK.  There surely must be good reasons for this.

The "good reasons" are that we are unable to handle input from two or
more keyboards: they will mix up into a single garbled stream of input
events.  That's because there's only one channel through which Emacs
reads input -- the single event queue we have in keyboard.c.

> The following description includes things which aren't yet committed.
> 
> "Move" means to move the minibuffer from frame F1 to frame F2, as for
> example, C-x 5 o requires when minibuffer-follows-selected-frame is t
> (the default).  When enable-recursive-minibuffers is t, there can be a
> "stack" of minibuffers on a frame.  The entire stack gets moved.  The top
> minibuffer in the stack is at f->minibuffer_window, any lower elements
> are stored in w->prev_buffers, where w is XWINDOW (f->minibuffer_window).
> 
> It is also required to move minibuffers when the frame containing them
> gets deleted.  Otherwise the minibuffers would continue to be active, yet
> be inaccessible from any frame - this would be undesirable.
> 
> When the (second) last frame in a --daemon Emacs gets deleted, any
> minibuffers it contains get moved to the initial frame.  When calling
> emacsclient opens a new frame, these minibuffers need to be moved onto
> that new frame (or else be aborted, somehow).

AFAIU, this just means copying the data that describes the stack of
the minibuffers, is that right?  If so, what is left on the frame that
is the source of that: a single empty minibuffer? or the entire
original stack? or something else?

> The problem I'm trying to solve here is to understand what happens when
> emacsclient opens a frame on a different terminal from where emacs
> --daemon was started, when there are active minibuffers on the original
> terminal.  What would be nice would be for these minibuffers to be moved
> onto the new frame (when minibuffer-follows-selected-frame is t) or left
> on the other non-initial frame (otherwise).  It appears, from Miha's
> observation yesterday, that any active minibuffers would get aborted in
> this case, to prevent the old frame blocking the new one.

And what's wrong with aborting the active minibuffer in this case?
isn't that what the user wants?



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-17 19:32                     ` Alan Mackenzie
  2021-03-17 19:55                       ` Eli Zaretskii
@ 2021-03-21 15:44                       ` Miha Rihtaršič
  2021-03-21 17:03                         ` Alan Mackenzie
  1 sibling, 1 reply; 52+ messages in thread
From: Miha Rihtaršič @ 2021-03-21 15:44 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Stefan Monnier, emacs-devel

Alan Mackenzie <acm@muc.de> writes:

> Hello, Miha.

Hello

> Yes.  Sorry about these.  This time around, I've tried to be more
> careful and done more testing myself - hence me taking a few days longer
> than I have up till now.  I've found and corrected another ~two bugs
> myself.
>
>> 1)
>> C-x C-f on frame A
>> select frame B
>> select frame A
>> Minibuffer is moved to B, but not back to A.
>
>> 2)
>> Have two frames open
>> open a minibuffer on a frame
>> close this frame
>> The other frame does have the miniwindow selected, but the
>> minibuffer isn't shown in it.
>
> I think I've corrected these two, now.
>
> Here's another patch, this time to be applied on top of the last patch.
> [...]
> do you see any reason not to commit this?

Sorry for the late reply. During my testing I didn't encounter any new
problems. Thanks.



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 15:43                                                           ` Eli Zaretskii
@ 2021-03-21 16:17                                                             ` Michael Welsh Duggan
  2021-03-21 16:37                                                             ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Michael Welsh Duggan @ 2021-03-21 16:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Alan Mackenzie, jakanakaevangeli, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> The problem I'm trying to solve here is to understand what happens when
>> emacsclient opens a frame on a different terminal from where emacs
>> --daemon was started, when there are active minibuffers on the original
>> terminal.  What would be nice would be for these minibuffers to be moved
>> onto the new frame (when minibuffer-follows-selected-frame is t) or left
>> on the other non-initial frame (otherwise).  It appears, from Miha's
>> observation yesterday, that any active minibuffers would get aborted in
>> this case, to prevent the old frame blocking the new one.
>
> And what's wrong with aborting the active minibuffer in this case?
> isn't that what the user wants?

I see two use cases here.  

1) I've purposefully killed that last active frame.  In this case any
   remaining open minibuffers are probably a mistake.

2) My connection to a remote machine went down, taking my last frame
   with it.  I reconnect and I might want my minibuffer state back.

I'm personally of the opinion that just aborting the minibuffers is good
enough.  Minibuffers should not generally be long-lived with state
anyway.  Under almost all circumstances, I believe any lost minibuffer
state should be recoverable manually.

-- 
Michael Welsh Duggan
(md5i@md5i.com)



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 15:43                                                           ` Eli Zaretskii
  2021-03-21 16:17                                                             ` Michael Welsh Duggan
@ 2021-03-21 16:37                                                             ` Alan Mackenzie
  1 sibling, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-21 16:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: jakanakaevangeli, emacs-devel

Hello, Eli.

On Sun, Mar 21, 2021 at 17:43:02 +0200, Eli Zaretskii wrote:
> > Date: Sun, 21 Mar 2021 14:49:56 +0000
> > Cc: jakanakaevangeli@chiru.no, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > It's a "feature" we only allow a single terminal to be in the "input"
> > > state at any given time.

> > Er, OK.  There surely must be good reasons for this.

> The "good reasons" are that we are unable to handle input from two or
> more keyboards: they will mix up into a single garbled stream of input
> events.  That's because there's only one channel through which Emacs
> reads input -- the single event queue we have in keyboard.c.

Thanks, that's very clear.

> > The following description includes things which aren't yet committed.

> > "Move" means to move the minibuffer from frame F1 to frame F2, as for
> > example, C-x 5 o requires when minibuffer-follows-selected-frame is t
> > (the default).  When enable-recursive-minibuffers is t, there can be a
> > "stack" of minibuffers on a frame.  The entire stack gets moved.  The top
> > minibuffer in the stack is at f->minibuffer_window, any lower elements
> > are stored in w->prev_buffers, where w is XWINDOW (f->minibuffer_window).

> > It is also required to move minibuffers when the frame containing them
> > gets deleted.  Otherwise the minibuffers would continue to be active, yet
> > be inaccessible from any frame - this would be undesirable.

> > When the (second) last frame in a --daemon Emacs gets deleted, any
> > minibuffers it contains get moved to the initial frame.  When calling
> > emacsclient opens a new frame, these minibuffers need to be moved onto
> > that new frame (or else be aborted, somehow).

> AFAIU, this just means copying the data that describes the stack of
> the minibuffers, is that right?

Not quite: if there're also minibuffers in the destination frame (with
minibuffer-follows-selected-frame set to nil), the two minibuffer stacks
get combined in the destination.

> If so, what is left on the frame that is the source of that: a single
> empty minibuffer? or the entire original stack? or something else?

What is left on the source frame is a single minibuffer, " *Minibuf-0*",
which functions as the null minibuffer.

> > The problem I'm trying to solve here is to understand what happens when
> > emacsclient opens a frame on a different terminal from where emacs
> > --daemon was started, when there are active minibuffers on the original
> > terminal.  What would be nice would be for these minibuffers to be moved
> > onto the new frame (when minibuffer-follows-selected-frame is t) or left
> > on the other non-initial frame (otherwise).  It appears, from Miha's
> > observation yesterday, that any active minibuffers would get aborted in
> > this case, to prevent the old frame blocking the new one.

> And what's wrong with aborting the active minibuffer in this case?

I don't think there's all that much wrong with aborting it.  It is a
little inconsistent with creating a new emacsclient frame on the same
terminal - here, any existing minibuffers survive.

> isn't that what the user wants?

I honestly don't know.  But as long as the code ends up doing something
consistent and reasonable (and this aborting the minibuffers _is_
reasonable), I don't think it's worth spending too much time on.  Given
how there's just one keyboard input stream, it would be a LOT of work to
make that one input stream per terminal.  We surely don't really need
this.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

* Re: Stop frames stealing eachothers' minibuffers!
  2021-03-21 15:44                       ` Miha Rihtaršič
@ 2021-03-21 17:03                         ` Alan Mackenzie
  0 siblings, 0 replies; 52+ messages in thread
From: Alan Mackenzie @ 2021-03-21 17:03 UTC (permalink / raw)
  To: Miha Rihtaršič; +Cc: Stefan Monnier, emacs-devel

Hello, Miha.

On Sun, Mar 21, 2021 at 16:44:53 +0100, Miha Rihtaršič wrote:
> Alan Mackenzie <acm@muc.de> writes:

[ .... ]

> > Here's another patch, this time to be applied on top of the last patch.
> > [...]
> > do you see any reason not to commit this?

> Sorry for the late reply. During my testing I didn't encounter any new
> problems. Thanks.

Many thanks for the testing!  I've now committed the patches to savannah.

-- 
Alan Mackenzie (Nuremberg, Germany).



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

end of thread, other threads:[~2021-03-21 17:03 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-06 23:25 Stop frames stealing eachothers' minibuffers! jakanakaevangeli
2021-02-07 12:55 ` Alan Mackenzie
2021-02-07 16:44   ` jakanakaevangeli
2021-02-07 20:26     ` Alan Mackenzie
2021-02-07 23:48       ` [External] : " Drew Adams
2021-03-13 18:31         ` Alan Mackenzie
2021-02-08 12:53       ` jakanakaevangeli
2021-02-11 11:44         ` Alan Mackenzie
2021-02-11 14:29           ` Stefan Monnier
2021-02-12  9:48             ` Alan Mackenzie
2021-03-13 18:23             ` Alan Mackenzie
2021-03-13 19:39               ` Stefan Monnier
2021-03-13 20:24                 ` Alan Mackenzie
2021-03-13 20:52                   ` Stefan Monnier
2021-03-14 18:26                     ` Alan Mackenzie
2021-03-14 18:48                       ` Eli Zaretskii
2021-03-14 20:32                       ` Stefan Monnier
2021-03-13 20:53               ` jakanakaevangeli
2021-03-14 19:17                 ` Alan Mackenzie
2021-03-14 21:23                   ` Miha Rihtaršič
2021-03-17 19:32                     ` Alan Mackenzie
2021-03-17 19:55                       ` Eli Zaretskii
2021-03-17 20:19                         ` Eli Zaretskii
2021-03-18 11:27                           ` Alan Mackenzie
2021-03-18 11:46                             ` Eli Zaretskii
2021-03-18 15:51                               ` martin rudalics
2021-03-18 16:58                                 ` Alan Mackenzie
2021-03-18 18:44                                   ` Eli Zaretskii
2021-03-19 11:40                                     ` Alan Mackenzie
2021-03-19 12:33                                       ` Eli Zaretskii
2021-03-19 15:35                                         ` Alan Mackenzie
2021-03-19 15:59                                           ` Eli Zaretskii
2021-03-20 10:28                                             ` Alan Mackenzie
2021-03-20 10:49                                               ` Eli Zaretskii
2021-03-20 12:24                                                 ` Alan Mackenzie
2021-03-20 12:49                                                   ` Miha Rihtaršič
2021-03-20 13:59                                                     ` Stefan Monnier
2021-03-21 10:30                                                     ` Alan Mackenzie
2021-03-21 10:38                                                       ` Eli Zaretskii
2021-03-21 10:40                                                         ` Eli Zaretskii
2021-03-21 14:49                                                         ` Alan Mackenzie
2021-03-21 15:00                                                           ` Stefan Monnier
2021-03-21 15:43                                                           ` Eli Zaretskii
2021-03-21 16:17                                                             ` Michael Welsh Duggan
2021-03-21 16:37                                                             ` Alan Mackenzie
2021-03-20 12:50                                                   ` Eli Zaretskii
2021-03-20 13:51                                                     ` Alan Mackenzie
2021-03-20 13:55                                                   ` Stefan Monnier
2021-03-20 14:01                                                     ` Eli Zaretskii
2021-03-20 14:12                                                     ` Alan Mackenzie
2021-03-21 15:44                       ` Miha Rihtaršič
2021-03-21 17:03                         ` Alan Mackenzie

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