Re: About the 'minibuffer' frame parameter

[martin rudalics <rudalics@gmx.at>]

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

 >> But then we can't discriminate minibuffer-less from normal frames by
 >> looking at the parameter value only.
 >
 > We can look at what window-frame returns for that window, can't we?

Tediously so, yes.

 >> This goes both ways.  With (2) Emacs would choose nil when
 >> ‘set-frame-position’ explicitly asks for a window.  And with no
 >> 'minibuffer' specified we'd have to return t or a window in any case.
 >
 > Yes, but IMO nil is not a meaningful value.  If we know better, we
 > should return a more concrete value.

Sounds convincing.

 > Yes, of course, the current situation is not impossible.  We are
 > talking about improving it.

I attached a preliminary version of the code changes.  Please have a
look.

Thanks, martin

[-- Attachment #2: minibuffer-frame-parameter.diff --]
[-- Type: text/plain, Size: 4532 bytes --]

diff --git a/lisp/frameset.el b/lisp/frameset.el
index 2453f57..aa16647 100644
--- a/lisp/frameset.el
+++ b/lisp/frameset.el
@@ -908,10 +908,7 @@ frameset--reuse-frame
 	   (unless (or frame (eq (cdr (assq 'minibuffer parameters)) 'only))
 	     (setq frame (frameset--find-frame-if
 			  (lambda (f)
-			    (let ((w (frame-parameter f 'minibuffer)))
-			      (and (window-live-p w)
-				   (window-minibuffer-p w)
-				   (eq (window-frame w) f))))
+			    (eq (frame-parameter f 'minibuffer) t))
 			  display))))
 	  (mini
 	   ;; For minibufferless frames, check whether they already exist,
@@ -1028,7 +1025,7 @@ frameset-keep-original-display-p

 (defun frameset-minibufferless-first-p (frame1 _frame2)
   "Predicate to sort minibufferless frames before other frames."
-  (not (frame-parameter frame1 'minibuffer)))
+  (windowp (frame-parameter frame1 'minibuffer)))

 ;;;###autoload
 (cl-defun frameset-restore (frameset
diff --git a/src/frame.c b/src/frame.c
index 899c315..e550e51 100644
--- a/src/frame.c
+++ b/src/frame.c
@@ -658,6 +658,7 @@ struct frame *
       mw->mini = 1;
       wset_frame (mw, frame);
       fset_minibuffer_window (f, mini_window);
+      store_frame_param (f, Qminibuffer, Qt);
     }
   else
     {
@@ -770,6 +771,7 @@ struct frame *
     }

   fset_minibuffer_window (f, mini_window);
+  store_frame_param (f, Qminibuffer, mini_window);

   /* Make the chosen minibuffer window display the proper minibuffer,
      unless it is already showing a minibuffer.  */
@@ -807,6 +809,7 @@ struct frame *

   mini_window = f->root_window;
   fset_minibuffer_window (f, mini_window);
+  store_frame_param (f, Qminibuffer, Qonly);
   XWINDOW (mini_window)->mini = 1;
   wset_next (XWINDOW (mini_window), Qnil);
   wset_prev (XWINDOW (mini_window), Qnil);
@@ -2404,6 +2407,46 @@ of them (the selected terminal frame) is actually displayed.
 {
   register Lisp_Object old_alist_elt;

+  if (EQ (prop, Qminibuffer))
+    {
+      if (WINDOWP (val))
+	{
+	  if (!MINI_WINDOW_P (XWINDOW (val)))
+	    error ("The 'minibuffer' parameter does not specify a valid minibuffer window");
+	  else if (FRAME_HAS_MINIBUF_P (f))
+	    {
+	      if (EQ (val, FRAME_MINIBUF_WINDOW (f)))
+		val = Qt;
+	      else
+		error ("Can't change the minibuffer window of a frame with its own minibuffer");
+	    }
+	  else if (FRAME_MINIBUF_ONLY_P (f))
+	    {
+	      if (EQ (val, FRAME_MINIBUF_WINDOW (f)))
+		val = Qonly;
+	      else
+		error ("Can't change the minibuffer window of a minibuffer-only frame");
+	    }
+	  else
+	    /* Store the chosen minibuffer window.  */
+	    fset_minibuffer_window (f, val);
+	}
+      else
+	{
+	  Lisp_Object old_val = Fcdr (Fassq (Qminibuffer, f->param_alist));
+
+	  if (!NILP (old_val))
+	    {
+	      if (WINDOWP (old_val) && NILP (val))
+		/* Don't change the value for a minibuffer-less frame if
+		   only nil was specified as new value.  */
+		val = old_val;
+	      else if (!EQ (old_val, val))
+		error ("Can't change the 'minibuffer' parameter of this frame");
+	    }
+	}
+    }
+
   /* The buffer-list parameters are stored in a special place and not
      in the alist.  All buffers must be live.  */
   if (EQ (prop, Qbuffer_list))
@@ -2475,19 +2518,6 @@ of them (the selected terminal frame) is actually displayed.
       else if (EQ (prop, Qname))
 	set_term_frame_name (f, val);
     }
-
-  if (EQ (prop, Qminibuffer) && WINDOWP (val))
-    {
-      if (! MINI_WINDOW_P (XWINDOW (val)))
-	error ("Surrogate minibuffer windows must be minibuffer windows");
-
-      if ((FRAME_HAS_MINIBUF_P (f) || FRAME_MINIBUF_ONLY_P (f))
-	  && !EQ (val, f->minibuffer_window))
-	error ("Can't change the surrogate minibuffer of a frame with its own minibuffer");
-
-      /* Install the chosen minibuffer window, with proper buffer.  */
-      fset_minibuffer_window (f, val);
-    }
 }

 /* Return color matches UNSPEC on frame F or nil if UNSPEC
@@ -2565,10 +2595,6 @@ It is a list of elements of the form (PARM . VALUE), where PARM is a symbol.
 	   : FRAME_COLS (f));
   store_in_alist (&alist, Qwidth, make_number (width));
   store_in_alist (&alist, Qmodeline, (FRAME_WANTS_MODELINE_P (f) ? Qt : Qnil));
-  store_in_alist (&alist, Qminibuffer,
-		  (! FRAME_HAS_MINIBUF_P (f) ? Qnil
-		   : FRAME_MINIBUF_ONLY_P (f) ? Qonly
-		   : FRAME_MINIBUF_WINDOW (f)));
   store_in_alist (&alist, Qunsplittable, (FRAME_NO_SPLIT_P (f) ? Qt : Qnil));
   store_in_alist (&alist, Qbuffer_list, f->buffer_list);
   store_in_alist (&alist, Qburied_buffer_list, f->buried_buffer_list);