Re: Stop frames stealing eachothers' minibuffers!

[Alan Mackenzie]

Hello, Eli.

On Thu, Oct 15, 2020 at 21:18:10 +0300, Eli Zaretskii wrote:
> > Date: Thu, 15 Oct 2020 18:01:43 +0000
> > Cc: ghe@sdf.org, emacs-devel@gnu.org
> > From: Alan Mackenzie <acm@muc.de>

> > > The latter is correct, except for the last step: the active minibuffer
> > > should have switched to F2, which is now the selected frame.

> > OK, but I disagree.  We seem to have different mental models of the
> > minibuffer.  For you, the MB, I think, is what demands immediate
> > attention, therefore should be on the selected frame.  For me, the MB is
> > an integral part of the frame on which it was opened.

> > Is there any chance we could implement an option for this (which has been
> > mentioned already)?

> I'm okay with such an option.  I think I already said that.

I too think you did.  There's a proposed patch below.

[ .... ]

> > Anyhow, back to practicalities.  I think we agreed last night (talking
> > about the "hybrid" option) that the current way of doing things isn't
> > very good.  I think, but I'm not sure, that you're saying the MB, if
> > open, should always be present on the currently selected frame and
> > nowhere else.  If I'm wrong here, could you possibly give a precise
> > description of when you say the MB should be moved to a different frame.

> IMO, the minibuffer should _always_ be on the selected frame, unless
> it has its own minibuffer-only frame (in which case it is always
> there).

OK.  I've implemented a new variable `minibuffer-follows-frame' which is
t by default.  For safety's sake, only the default-toplevel-value of this
variable is consulted, to stop funny things happening should it be
dynamically bound or if there might be a buffer local value.

For minibuffer-follows-frame nil, the error "Command attempted to use
minibuffer while in minibuffer" no longer aborts an enclosing minibuffer
command.  There doesn't seem any need or benefit - the enclosing command
can carry on later.

Here's my patch (including amendments to the Emacs manual and a NEWS
entry).  Comments would be most appreciated.



diff --git a/doc/emacs/mini.texi b/doc/emacs/mini.texi
index 54f046a7e0..0e6d86f3b8 100644
--- a/doc/emacs/mini.texi
+++ b/doc/emacs/mini.texi
@@ -69,6 +69,16 @@ Basic Minibuffer
 the minibuffer comes back.  While the minibuffer is in use, Emacs does
 not echo keystrokes.
 
+  Whilst using the minibuffer, you can switch to a different frame,
+perhaps to note text you need to enter (@pxref{Frame Commands}).  By
+default, the minibuffer moves to this new frame.  If you set the user
+option @code{minibuffer-follows-frame} to @code{nil}, then the
+minibuffer stays in the frame where it was opened, and you must switch
+back to that frame in order to complete (or abort) the current
+command.  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.
+
 @node Minibuffer File
 @section Minibuffers for File Names
 
diff --git a/etc/NEWS b/etc/NEWS
index f3e3d9a1b6..95b5d7ffd0 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -86,6 +86,19 @@ useful on systems such as FreeBSD which ships only with 
"etc/termcap".
 * Changes in Emacs 28.1
 
 +++
+** Switching frames when a minibuffer is active has been rationalized.
+By default, the minibuffer is moved to the newly selected frame.  When
+the current command is continued (by completing the minibuffer
+action), it takes effect in the frame the minibuffer was first opened
+in.  An alternative behavior is available by customizing
+'minibuffer-follows-frame' to nil; here, the minibuffer stays on the
+frame it was first opened on, and you must switch back to this frame
+to continue or abort the current command.  The old (pre 28.1),
+somewhat chaotic behavior is no longer available.
+
++++
+*** A new system for displaying documentation for groups of function is added.
+
 ** New system for displaying documentation for groups of function.
 This can either be used by saying 'M-x shortdoc-display-group' and
 choosing a group, or clicking a button in the *Help* buffers when
diff --git a/lisp/cus-start.el b/lisp/cus-start.el
index 3fd6ac031c..2120f5a627 100644
--- a/lisp/cus-start.el
+++ b/lisp/cus-start.el
@@ -394,6 +394,7 @@ minibuffer-prompt-properties--setter
             ;;                         (directory :format "%v"))))
             (load-prefer-newer lisp boolean "24.4")
             ;; minibuf.c
+             (minibuffer-follows-frame minibuffer boolean "28.1")
             (enable-recursive-minibuffers minibuffer boolean)
             (history-length minibuffer
                             (choice (const :tag "Infinite" t) integer)
diff --git a/src/frame.c b/src/frame.c
index 0b707c2af8..361a7119c5 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -1482,6 +1482,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 ();
   return frame;
 }
 
diff --git a/src/lisp.h b/src/lisp.h
index 45353fbef3..5b00252f24 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -4336,6 +4336,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 Lisp_Object get_minibuffer (EMACS_INT);
 extern void init_minibuf_once (void);
 extern void syms_of_minibuf (void);
diff --git a/src/minibuf.c b/src/minibuf.c
index f957b2ae17..61863af538 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -64,6 +64,12 @@ static Lisp_Object minibuf_prompt;
 static ptrdiff_t minibuf_prompt_width;
 
 
+static bool
+minibuf_follows_frame (void)
+{
+  return !NILP (Fdefault_toplevel_value (Qminibuffer_follows_frame));
+}
+
 /* Put minibuf on currently selected frame's minibuffer.
    We do this whenever the user starts a new minibuffer
    or when a minibuffer exits.  */
@@ -76,37 +82,74 @@ choose_minibuf_frame (void)
       && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
     {
       struct frame *sf = XFRAME (selected_frame);
-      Lisp_Object buffer;
-
       /* I don't think that any frames may validly have a null minibuffer
         window anymore.  */
       if (NILP (sf->minibuffer_window))
        emacs_abort ();
 
-      /* Under X, we come here with minibuf_window being the
-        minibuffer window of the unused termcap window created in
-        init_window_once.  That window doesn't have a buffer.  */
-      buffer = XWINDOW (minibuf_window)->contents;
-      if (BUFFERP (buffer))
-       /* Use set_window_buffer instead of Fset_window_buffer (see
-          discussion of bug#11984, bug#12025, bug#12026).  */
-       set_window_buffer (sf->minibuffer_window, buffer, 0, 0);
-      minibuf_window = sf->minibuffer_window;
+      if (minibuf_follows_frame ())
+        minibuf_window = sf->minibuffer_window;
+      else if (minibuf_level)
+        {
+          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;
+                  goto after_set;
+                }
+              minibuf_window = sf->minibuffer_window;
+            after_set: ;
+            }
+        }
+      else
+        minibuf_window = Qnil;
     }
 
-  /* 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;
+  if (minibuf_follows_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;
 
-    FOR_EACH_FRAME (tail, frame)
-      if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (XFRAME (frame))))
-         && !(EQ (frame, selected_frame)
-              && minibuf_level > 0))
-       Fset_frame_selected_window (frame, Fframe_first_window (frame), Qnil);
-  }
+      FOR_EACH_FRAME (tail, frame)
+        if (MINI_WINDOW_P (XWINDOW (FRAME_SELECTED_WINDOW (XFRAME (frame))))
+            && !(EQ (frame, selected_frame)
+                 && minibuf_level > 0))
+          Fset_frame_selected_window (frame, Fframe_first_window (frame), 
Qnil);
+    }
+}
+
+/* If `minibuffer_follows_frame' 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 (!minibuf_level)
+    return;
+  if (!minibuf_follows_frame ())
+    return;
+  if (FRAMEP (selected_frame)
+        && FRAME_LIVE_P (XFRAME (selected_frame))
+        && !EQ (minibuf_window, XFRAME (selected_frame)->minibuffer_window))
+    {
+      struct frame *sf = XFRAME (selected_frame);
+      Lisp_Object old_frame = XWINDOW (minibuf_window)->frame;
+      struct frame *of = XFRAME (old_frame);
+      Lisp_Object buffer = XWINDOW (minibuf_window)->contents;
+
+      set_window_buffer (sf->minibuffer_window, buffer, 0, 0);
+      minibuf_window = sf->minibuffer_window;
+      Fset_frame_selected_window (selected_frame, sf->minibuffer_window,
+                                  Qnil);
+      set_window_buffer (of->minibuffer_window, get_minibuffer (0), 0, 0);
+    }
 }
 
 DEFUN ("active-minibuffer-window", Factive_minibuffer_window,
@@ -362,9 +405,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, 
Lisp_Object prompt,
   Lisp_Object histstring;
   Lisp_Object histval;
 
-  Lisp_Object empty_minibuf;
-  Lisp_Object dummy, frame;
-
   specbind (Qminibuffer_default, defalt);
   specbind (Qinhibit_read_only, Qnil);
 
@@ -416,11 +456,15 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, 
Lisp_Object prompt,
     {
       Lisp_Object str
        = build_string ("Command attempted to use minibuffer while in 
minibuffer");
-      if (EQ (selected_window, minibuf_window))
-       Fsignal (Quser_error, (list1 (str)));
+      /* OLD STOUGH, 2020-10-21 */
+      if (!minibuf_follows_frame ()
+          || EQ (selected_window, minibuf_window))
+        Fsignal (Quser_error, (list1 (str)));
       else
-       /* If we're in another window, cancel the minibuffer that's active.  */
-       Fthrow (Qexit, str);
+        /* If we're in another window, cancel the minibuffer that's active.  */
+        Fthrow (Qexit, str);
+      /* END OF OLD STOUGH */
+      Fsignal (Quser_error, (list1 (str)));
     }
 
   if ((noninteractive
@@ -433,6 +477,8 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, 
Lisp_Object prompt,
       return unbind_to (count, val);
     }
 
+  minibuf_level++;         /* Before calling choose_minibuf_frame.  */
+
   /* Choose the minibuffer window and frame, and take action on them.  */
 
   /* Prepare for restoring the current buffer since choose_minibuf_frame
@@ -484,7 +530,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, 
Lisp_Object prompt,
     = Fcons (Fthis_command_keys_vector (), minibuf_save_list);
 
   record_unwind_protect_void (read_minibuf_unwind);
-  minibuf_level++;
   /* We are exiting the minibuffer one way or the other, so run the hook.
      It should be run before unwinding the minibuf settings.  Do it
      separately from read_minibuf_unwind because we need to make sure that
@@ -566,23 +611,6 @@ read_minibuf (Lisp_Object map, Lisp_Object initial, 
Lisp_Object prompt,
   if (minibuf_level == 1 || !EQ (minibuf_window, selected_window))
     minibuf_selected_window = selected_window;
 
-  /* Empty out the minibuffers of all frames other than the one
-     where we are going to display one now.
-     Set them to point to ` *Minibuf-0*', which is always empty.  */
-  empty_minibuf = get_minibuffer (0);
-
-  FOR_EACH_FRAME (dummy, frame)
-    {
-      Lisp_Object root_window = Fframe_root_window (frame);
-      Lisp_Object mini_window = XWINDOW (root_window)->next;
-
-      if (! NILP (mini_window) && ! EQ (mini_window, minibuf_window)
-         && !NILP (Fwindow_minibuffer_p (mini_window)))
-       /* 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).  */
@@ -1911,6 +1939,8 @@ syms_of_minibuf (void)
   staticpro (&minibuf_prompt);
   staticpro (&minibuf_save_list);
 
+  DEFSYM (Qminibuffer_follows_frame,
+          "minibuffer-follows-frame");
   DEFSYM (Qcompletion_ignore_case, "completion-ignore-case");
   DEFSYM (Qminibuffer_default, "minibuffer-default");
   Fset (Qminibuffer_default, Qnil);
@@ -1954,6 +1984,14 @@ For example, `eval-expression' uses this.  */);
 The function is called with the arguments passed to `read-buffer'.  */);
   Vread_buffer_function = Qnil;
 
+  DEFVAR_BOOL ("minibuffer-follows-frame", minibuffer_follows_frame,
+               doc: /* Non-nil means an open minibuffer will move to a newly 
selected frame.
+Nil means that a minibuffer will appear only in the frame which created it.
+
+Any buffer local or dynamic binding of this variable is ignored.  Only the
+default top level value is used.  */);
+  minibuffer_follows_frame = 1;
+
   DEFVAR_BOOL ("read-buffer-completion-ignore-case",
               read_buffer_completion_ignore_case,
               doc: /* Non-nil means completion ignores case when reading a 
buffer name.  */);



-- 
Alan Mackenzie (Nuremberg, Germany).