Re: About the 'minibuffer' frame parameter

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sat, 06 Aug 2016 11:33:55 +0200
> From: martin rudalics <rudalics@gmx.at>
> Cc: emacs-devel@gnu.org

I must admit I'm a bit confused about the issues you raise.  So let me
make a step or two back, before we continue.

You started by saying:

> (progn
>   (setq minibuffer-less-frame (make-frame '((minibuffer . nil))))
>   (setq minibuffer-only-frame (make-frame '((minibuffer . only))))
> 
>   (set-frame-parameter
>    minibuffer-less-frame 'minibuffer (frame-root-window minibuffer-only-frame))
>   (frame-parameter minibuffer-less-frame 'minibuffer))
> [...]
> But if the frame is minibuffer-less and uses the minibuffer window of
> some other frame, we return as value nil although the real, internal
> frame parameter's value (not the one produced by the
> 
>   store_in_alist (&alist, Qminibuffer,
>                   (! FRAME_HAS_MINIBUF_P (f) ? Qnil
>                    : FRAME_MINIBUF_ONLY_P (f) ? Qonly
>                    : FRAME_MINIBUF_WINDOW (f)));
> 
> construct) is actually that window.  Otherwise, evaluating the
> ‘set-frame-parameter’ above would have produced an error.

I don't understand the last sentence: it starts with "otherwise",
which seems to imply that if frame-parameter did not produce nil for a
minibuffer-less frame, the call to set-frame-parameter would somehow
signal an error.  However, I don't see any relation between what
(frame-parameter FRAME 'minibuffer) returns and any calls to
set-frame-parameter for the same frame.  So what error were you
talking about?

Next, the documentation issue:

  `minibuffer'
       Whether this frame has its own minibuffer.  The value `t' means
       yes, `nil' means no, `only' means this frame is just a minibuffer.
       If the value is a minibuffer window (in some other frame), the
       frame uses that minibuffer.

       This frame parameter takes effect when the frame is created, and
       can not be changed afterwards.

  The sentence "If the value is a minibuffer window (in some other frame),
  the frame uses that minibuffer." is misleading.  A minibuffer window is
  reported iff that window is on the _same_ frame and that frame is not a
  minibuffer-only frame.  A minibuffer window in some other frame is never
  reported.

So when do we report t?  It sounds like the answer is "never", right?
IOW, the documentation seems to describe some situation that existed
in the past, and is now OBE due to code changes.  Correct?

> Note in this context that ‘minibuffer-window’ returns the correct
> minibuffer window for its FRAME argument while ‘set-minibuffer-window’
> does not allow to set the minibuffer window for a specific frame.

IMO, this is a separate, albeit probably related, issue.  Are there
any problems to let set-minibuffer-window allow setting the minibuffer
window of a frame?

> I'm not sure how to deal with this situation.  Personally, I'd prefer to
> report the real, internal 'minibuffer' parameter

And I implicitly agreed with that.

> but am afraid that might break existing code.

And for that, I proposed:

> > But I haven't checked all places because I rather soonish stumbled upon
> > things like
> > 
> > (eq (cdr (or (assq 'minibuffer initial-frame-alist)
> >            (assq 'minibuffer window-system-frame-alist)
> >            (assq 'minibuffer default-frame-alist)
> >            '(minibuffer . t)))
> >      t)
> > 
> > in ‘frame-notice-user-settings’.  And one revealing comment is in
> > ‘set-frame-configuration’:
> > 
> >                 ;; Since we can't set a frame's minibuffer status,
> >                 ;; we might as well omit the parameter altogether.
> 
> We could simply change the above code to follow suit.

To which you replied:

> Change what?

Obviously, change the Lisp snippet shown above, which expects to see a
nil minibuffer parameter for minibuffer-less frames.  Change it not to
expect that, and instead test the minibuffer window for whether it is
on the same frame or not.

> Initially I only wanted to simplify code like
> 
> (FRAME_HAS_MINIBUF_P (f) && !FRAME_MINIBUF_ONLY_P (f))
> 
> because once f has been created, FRAME_HAS_MINIBUF_P (f) and
> !FRAME_MINIBUF_ONLY_P (f) invariantly hold for the entire lifetime of f.
> A bit field telling whether a frame owns a minibuffer or is
> minibuffer-only/-less seems more practical instead of these macros.  The
> value stored in that bit field would have to reflect the value stored in
> the 'minibuffer' frame parameter.  But for a minibuffer-less frame we
> OT1H store the minibuffer window in that parameter and OTOH we report
> the value nil for that parameter in ‘frame-parameters’.

I think we should report the window, i.e. the actual value stored in
that parameter.

> We could either modify that code in store_frame_param
> 
>   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);
>     }
> 
> to store Qnil instead of the minibuffer window or do away with the
> special treatment of the 'minibuffer' parameter in ‘frame-parameters’ as
> I mentioned earlier.

I like the latter possibility much better.  In general, I prefer to
report the actual values whenever possible, especially when we have no
reason to hide the value from Lisp applications.

> As mentioned before, removing the special treatment of the 'minibuffer'
> parameter in ‘frame-parameters’ would imply that Elisp code relying on
> the values we report currently might be broken in the future.

And my suggestion to that was to fix that code, wherever we find it.

> Things would be much clearer if we had provided some orthogonality of
> ‘minibuffer-window’ and ‘set-minibuffer-window’.

Not sure what you mean by "orthogonality" here.