bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

In my code I check (essentially) this:

(let ((swin  (selected-window)))
  (when (window-minibuffer-p swin) (setq swin  (minibuffer-selected-window)))
  (and (get-buffer-window "*Completions*")
       (window-live-p swin)
       (not (eq (window-buffer swin) (get-buffer "*Completions*")))
       (interactive-p)))

It does what I expect in all Emacs releases, but not in this
prerelease.  My `*Completions*' window is in its own one-window frame,
with the window dedicated.  Input for that frame is redirected to my
standalone minibuffer frame.

It should be the case in my context that this returns the minibuffer
window:

(cdr (assoc 'minibuffer (frame-parameters
                          (#<frame *Completions* 000000000cacf6c8>))))

Instead, it now returns nil.  Parameter `minibuffer' for the
`*Completions*' frame is no longer the minibuffer window; it is nil.

This is the code that redirects the frame focus, which I guess is
relevant:

(let ((redirect  (if (active-minibuffer-window)
                     1on1-minibuffer-frame
                   (and completion-reference-buffer
                        (get-buffer-window completion-reference-buffer 'visible)
                        (not (eq (get-buffer "*Completions*") completion-reference-buffer))
                        (window-frame (get-buffer-window completion-reference-buffer t))))))
  (when redirect (redirect-frame-focus (selected-frame) redirect)))

That code is in a function, `1on1-display-*Completions*-frame',
that I use with `special-display-buffer-names':

(add-to-list 'special-display-buffer-names
             `("*Completions*" 1on1-display-*Completions*-frame
               ((background-color ,@1on1-completions-frame-background)
                (mouse-color      ,@1on1-completions-frame-mouse+cursor-color)
                (cursor-color     ,@1on1-completions-frame-mouse+cursor-color)
                (menu-bar-lines . 0) (tool-bar-lines . 0) ; No menu bar or tool bar.
                ,@(and 1on1-completions-frame-width
                       `((width   ,@1on1-completions-frame-width))))))

In GNU Emacs 26.0.90 (build 3, x86_64-w64-mingw32)
 of 2017-10-13
Repository revision: 906224eba147bdfc0514090064e8e8f53160f1d4
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --host=x86_64-w64-mingw32
 --without-compress-install 'CFLAGS=-O2 -static -g3''

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > It should be the case in my context that this returns the minibuffer
 > window:
 >
 > (cdr (assoc 'minibuffer (frame-parameters
 >                            (#<frame *Completions* 000000000cacf6c8>))))
 >
 > Instead, it now returns nil.  Parameter `minibuffer' for the
 > `*Completions*' frame is no longer the minibuffer window; it is nil.

Since the 'minibuffer' parameter of a frame cannot be changed (from nil
to some non-nil value and vice-versa at least) after its creation, your
code somehow now seems to fail to set up that parameter.  Please show
the corresponding part where your code tries to set the parameter.  If
necessary, debug it to show that after the corresponding ‘make-frame’
call the frame does not have that parameter set as requested.

Thanks, martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

Thanks for looking into this.

>  > It should be the case in my context that this returns the minibuffer
>  > window: (cdr (assoc 'minibuffer (frame-parameters
>  >                                  (#<frame *Completions*
>  >                                   000000000cacf6c8>))))
>  >
>  > Instead, it now returns nil.  Parameter `minibuffer' for the
>  > `*Completions*' frame is no longer the minibuffer window; it is nil.
> 
> Since the 'minibuffer' parameter of a frame cannot be changed (from nil
> to some non-nil value and vice-versa at least) after its creation, 

Is that something new?

> your code somehow now seems to fail to set up that parameter.
> Please show the corresponding part where your code tries to
> set the parameter.  If necessary, debug it to show that after
> the corresponding ‘make-frame’ call the frame does not have
> that parameter set as requested.

I don't call `make-frame' to create frame `*Completions*'.
It is created when ` 1on1-display-*Completions*-frame' is
called, and that is done by `special-display-function'.
I showed the code for that in my previous message.

I don't ever set parameter `minibuffer' explicitly for
*Completions*.  I'm guessing that it has always gotten
set automatically when frame input was redirected from
frame *Completions* to the standalone minibuffer.

(It is redirected to `completion-reference-buffer' if
the minibuffer is not active (and if `c-r-b' is not
frame *Completions*)).

See previous message for that code.  The complete code is
here: https://www.emacswiki.org/emacs/download/oneonone.el

As I say, the code works in all Emacs releases.  It
does not work in this Emacs 26 prerelease (only).

Here is (some of) the code (from icicles-mcmd.el) that
relies on the parameter having being set:

(defun icicle-delete-windows-on (buffer)
  "Delete all windows showing BUFFER.
If such a window is alone in its frame, then delete the frame
- unless it is the only frame or a standalone minibuffer frame."
  (interactive
   (list (let ((enable-recursive-minibuffers  t))
           (read-buffer "Remove all windows showing buffer: "
                        (current-buffer)
                        'existing))))
  (setq buffer  (get-buffer buffer))
  (when buffer
    (let* ((this-buffer-frames  (icicle-frames-on buffer))
           (this-frame          (car this-buffer-frames))
           mini-param)
      (unless (and this-frame
                   (frame-visible-p this-frame)
                   (null (cdr this-buffer-frames))
                   (setq mini-param
                         (cdr
                          (assoc
                           'minibuffer ; <=================
                           (frame-parameters this-frame))))
                   (eq mini-param (active-minibuffer-window))
                   (save-window-excursion
                     (select-frame this-frame)
                     (one-window-p t 'SELECTED-FRAME-ONLY)))
        (let (win)
          (dolist (fr  this-buffer-frames)
            (setq win  (get-buffer-window buffer fr))
            (select-window win)
            (if (and (one-window-p t)
                     (cdr (visible-frame-list)))
                (delete-frame)
              (delete-window (selected-window)))))))))

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 >> Since the 'minibuffer' parameter of a frame cannot be changed (from nil
 >> to some non-nil value and vice-versa at least) after its creation,
 >
 > Is that something new?

Hardly.  The text in the Elisp manual

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

is the same for Emacs 23 as for Emacs 26.  It's not correct because we
allow to set the parameter from one minibuffer window to another but I
didn't change the doc yet simply because I never read the corresponding
code attentively enough.

 > I don't call `make-frame' to create frame `*Completions*'.
 > It is created when ` 1on1-display-*Completions*-frame' is
 > called, and that is done by `special-display-function'.
 > I showed the code for that in my previous message.

The 'minibuffer' parameter must be set up specially by whoever calls
‘make-frame’.  If this is not done, you can't change it afterwards.  The
default value of ‘special-display-function’ is
‘special-display-popup-frame’ and I don't see the latter setting up the
'minibuffer' parameter anywhere.

 > I don't ever set parameter `minibuffer' explicitly for
 > *Completions*.  I'm guessing that it has always gotten
 > set automatically when frame input was redirected from
 > frame *Completions* to the standalone minibuffer.
 >
 > (It is redirected to `completion-reference-buffer' if
 > the minibuffer is not active (and if `c-r-b' is not
 > frame *Completions*)).

Maybe you mean "focus redirection" here which is something different
from setting up the 'minibuffer' parameter.

 > See previous message for that code.  The complete code is
 > here: https://www.emacswiki.org/emacs/download/oneonone.el
 >
 > As I say, the code works in all Emacs releases.  It
 > does not work in this Emacs 26 prerelease (only).
 >
 > Here is (some of) the code (from icicles-mcmd.el) that
 > relies on the parameter having being set:

I understand that you want that parameter to have a non-nil value there.
So make sure that it is.  For this you will have to debug your earlier
version to see how they set up the 'miniuffer' parameter and compare
them with the current version to see how it fails to do that.

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

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

>  > I don't call `make-frame' to create frame `*Completions*'.
>  > It is created when ` 1on1-display-*Completions*-frame' is
>  > called, and that is done by `special-display-function'.
>  > I showed the code for that in my previous message.
> 
> The 'minibuffer' parameter must be set up specially by whoever calls
> ‘make-frame’.  If this is not done, you can't change it afterwards.  The
> default value of ‘special-display-function’ is
> ‘special-display-popup-frame’ and I don't see the latter setting up the
> 'minibuffer' parameter anywhere.

I use the default value of `special-display-function',
`special-display-popup-frame'.

I don't call `make-frame' for `*Completions*'.  I just
provide an alist to `special-display-buffer-names'.
That alist includes, as its second element, function
`1on1-display-*Completions*-frame', which displays the
frame.

This is `special-display-buffer-names':

(("*Completions*" 1on1-display-*Completions*-frame
  ((background-color . "LavenderBlush2")
   (mouse-color . "VioletRed")
   (cursor-color . "VioletRed")
   (menu-bar-lines . 0)
   (tool-bar-lines . 0)
   (width . 100)))
 ("*Help*" 1on1-display-*Help*-frame
  ((background-color . "Thistle")
   (mouse-color . "Blue Violet")
   (cursor-color . "Blue Violet")
   (height . 40))))

The only thing that matters there is
`1on1-display-*Completions*-frame'.

>  > I don't ever set parameter `minibuffer' explicitly for
>  > *Completions*.  I'm guessing that it has always gotten
>  > set automatically when frame input was redirected from
>  > frame *Completions* to the standalone minibuffer.
>  >
>  > (It is redirected to `completion-reference-buffer' if
>  > the minibuffer is not active (and if `c-r-b' is not
>  > frame *Completions*)).
> 
> Maybe you mean "focus redirection" here which is something
> different from setting up the 'minibuffer' parameter.

Yes, I mean input/focus redirection, with
`redirect-frame-focus'.  I'm guessing that redirecting
focus is somehow behind the changed value of parameter
`minibuffer' for frame `*Completions*'.  The redirection
of frame focus is the only connection I'm aware of
between those two frames.

I don't change parameter `minibuffer' for *Completions*,
that I'm aware of.  Frame *Completions* is created by
the special-display code, just by my using
`special-display-buffer-names'.  I don't create it
directly - I don't call `make-frame' for it.  If it
is now getting the wrong value of parameter `minibuffer'
I don't see how it could be my code that is doing
that.
 
> I understand that you want that parameter to have a non-nil value there.
> So make sure that it is.  For this you will have to debug your earlier
> version to see how they set up the 'miniuffer' parameter and compare
> them with the current version to see how it fails to do that.

I have no idea what part of the special-display code might
have managed parameter `minibuffer' before vs now, or
whether that is done in Emacs C code or Emacs Lisp code.
I don't know what has changed in the Emacs code.

However, I realize now that I reported the exact opposite
of the real problem.  Apologies for that.

The problem is NOT that (1) PREVIOUSLY, frame *Completions*
had a `minibuffer' parameter whose value was the active
minibuffer window (on the minibuffer frame) and (2) NOW,
frame *Completions* has a nil `minibuffer' parameter.

The problem is that (1) PREVIOUSLY, frame *Completions*
had a nil `minibuffer' and (2) NOW, frame *Completions*
has a `minibuffer' parameter whose value is the active
minibuffer window (on the minibuffer frame).

IOW, the problem is that the separate, dedicated frame
*Completions* somehow gets a `minibuffer' parameter
(whose value is the minibuffer window on the minibuffer
frame).

`*Completions*' is not a minibuffer frame.  The only
minibuffer frame has a `minibuffer' parameter whose
value is `only' - the value is not a minibuffer
_window_.

Does the problem description make more sense now?
Sorry for the confusion.

I am not setting parameter `minibuffer'.  The only
connection I know of between the *Completions* frame
and the standalone minibuffer frame is the focus
redirection.  Is it possible that that redirection
code now mistakenly gives a non-nil `minibuffer'
parameter to the frame whose input focus is
redirected to the minibuffer frame?

Attached is the debug output that I see, in case it
helps.  The cause of my problem seems to be that
somehow frame `*Completions*' is getting a non-nil
parameter `minibuffer' - the value being the
minibuffer window (on the minibuffer frame).
Or do you interpret the attached debug info
differently?

Hoping you can set me straight about this.
I don't think I should need to do anything
different in my code, but if I need to make
some minor adjustment for some change in
Emacs, please let me know what I'll need to do.

So far, this seems like an Emacs bug, to me.


[-- Attachment #2: throw-bug-28978.el --]
[-- Type: application/octet-stream, Size: 2837 bytes --]

(defun icicle-delete-windows-on (buffer)
  "Delete all windows showing BUFFER.
If such a window is alone in its frame, then delete the frame - unless
it is the only frame or a standalone minibuffer frame."
  (interactive
   (list (let ((enable-recursive-minibuffers  t))
           (read-buffer "Remove all windows showing buffer: " (current-buffer) 'existing))))
  (setq buffer  (get-buffer buffer))
  (message "111, BUF: %S" buffer)
  (when buffer
    ;; Avoid error message "Attempt to delete minibuffer or sole ordinary window".
    (message "222, this-buf-frs: %S, this fr: %S" (icicle-frames-on buffer) (car (icicle-frames-on buffer)))
    (let* ((this-buffer-frames  (icicle-frames-on buffer))
           (this-frame          (car this-buffer-frames))
           mini-param)
      (message "333, fr-visib: %S, only-1: %S, miniparam: %S, eq: %S"
               (frame-visible-p this-frame)
               (null (cdr this-buffer-frames))
               (cdr (assoc 'minibuffer (frame-parameters this-frame)))
               (eq (cdr (assoc 'minibuffer (frame-parameters this-frame))) (active-minibuffer-window))
               )
      (unless (and this-frame
                   (frame-visible-p this-frame)
                   (null (cdr this-buffer-frames)) ; Only one frame shows BUFFER.
                   (setq mini-param ; <==========================
                         (cdr (assoc 'minibuffer (frame-parameters this-frame))))
                   (eq mini-param (active-minibuffer-window)) ; Has an active minibuffer.
                   (save-window-excursion
                     (select-frame this-frame)
                     (one-window-p t 'SELECTED-FRAME-ONLY))) ; Only one window.
        (message "444, must be one win")
        (let (win)
          (dolist (fr  this-buffer-frames)
            (setq win  (get-buffer-window buffer fr))
            (select-window win)
            (message "555, one-win: %S, not-1-fr: %S" (one-window-p t) (cdr (visible-frame-list)))
            (if (and (one-window-p t)  (cdr (visible-frame-list))) ; Sole window but not sole frame.
                (delete-frame)
              (delete-window (selected-window)))))))))

In Emacs 24.5 I see this debug output:

111, BUF: #<buffer *Completions*>
222, this-buf-frs: (#<frame *Completions* 052d3778>), this fr: #<frame *Completions* 052d3778>
333, fr-visib: t, only-1: t, miniparam: nil, eq: nil
444, must be one win
555, one-win: t, not-1-fr: (#<frame drews-lisp-20 014b3420> #<frame *scratch* 047b6360> #<frame *Messages* 015c8588> #<frame *Completions* 052d3778>)

But in Emacs 26 pretest 1 I see this output:

111, BUF: #<buffer *Completions*>
222, this-buf-frs: (#<frame *Completions* 000000000cacf6c8>), this fr: #<frame *Completions* 000000000cacf6c8>
333, fr-visib: t, only-1: t, miniparam: #<window 6 on  *Minibuf-1*>, eq: t

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > The problem is that (1) PREVIOUSLY, frame *Completions*
 > had a nil `minibuffer' and (2) NOW, frame *Completions*
 > has a `minibuffer' parameter whose value is the active
 > minibuffer window (on the minibuffer frame).
 >
 > Does the problem description make more sense now?
 > Sorry for the confusion.

It does so indeed.  The change and its motivation are:

	Fix semantics of 'minibuffer' frame parameter

	The 'minibuffer' frame parameter is now t for a normal frame
	(a frame with a root window plus a minibuffer window) and the
	frame's minibuffer window for a minibuffer-less frame (a frame
	whose minibuffer window is on another frame).  See also:
	https://lists.gnu.org/archive/html/emacs-devel/2016-07/msg01259.html

	* src/frame.c (make_frame, make_frame_without_minibuffer)
	(make_minibuffer_frame): When assigning the frame's minibuffer
	window also store corresponding 'minibuffer' frame parameter.
	(store_frame_param): Move the 'minibuffer' parameter checks to
	the beginning so we can silently override the value before it
	gets stored in the parameter alist.  Fix error handling.
	(Fframe_parameters): Return value of 'minibuffer' parameter
	unmodified.

	* lisp/frameset.el (frameset-filter-minibuffer): When the cdr of
	the parameter is a minibuffer window, save (minibuffer . nil)
	instead of (minibuffer . t).
	(frameset--reuse-frame): To find a non-minibuffer-only frame
	look out for a frame whose 'minibuffer' parameter is t instead
	of that frame's minibuffer window.
	(frameset-minibufferless-first-p): To find a minibuffer-less
	frame look out for a frame whose 'minibuffer' parameter is a
	window instead of nil.

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

> The change and its motivation are:
> 
> 	Fix semantics of 'minibuffer' frame parameter
> 
> 	The 'minibuffer' frame parameter is now t for a normal frame
> 	(a frame with a root window plus a minibuffer window) and the
> 	frame's minibuffer window for a minibuffer-less frame (a frame
> 	whose minibuffer window is on another frame).  See also:
> 	https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__lists.gnu.org_archive_html_emacs-2Ddevel_2016-
> 2D07_msg01259.html&d=DwICaQ&c=RoP1YumCXCgaWHvlZYR8PQcxBKCX5YTpkKY057SbK10
> &r=qZLZosY6GBAaJtOG9v8QX7nD4BS9t9s5otgxC3d4IFU&m=iis9a2_IADv9pVb4yf4HsSV4
> tsAWIhyObsr30ujC7gQ&s=zcJ6cBmJCUoJPZS4bM11HngUAPBn-_GqScRVTZzkcas&e=
> 
> 	* src/frame.c (make_frame, make_frame_without_minibuffer)
> 	(make_minibuffer_frame): When assigning the frame's minibuffer
> 	window also store corresponding 'minibuffer' frame parameter.
> 	(store_frame_param): Move the 'minibuffer' parameter checks to
> 	the beginning so we can silently override the value before it
> 	gets stored in the parameter alist.  Fix error handling.
> 	(Fframe_parameters): Return value of 'minibuffer' parameter
> 	unmodified.
> 
> 	* lisp/frameset.el (frameset-filter-minibuffer): When the cdr of
> 	the parameter is a minibuffer window, save (minibuffer . nil)
> 	instead of (minibuffer . t).
> 	(frameset--reuse-frame): To find a non-minibuffer-only frame
> 	look out for a frame whose 'minibuffer' parameter is t instead
> 	of that frame's minibuffer window.
> 	(frameset-minibufferless-first-p): To find a minibuffer-less
> 	frame look out for a frame whose 'minibuffer' parameter is a
> 	window instead of nil.

So the meaning of frame-parameter `minibuffer' has changed.
I will need to adjust my code somehow.

How would you suggest I change the test I have been using,
to detect a frame that has the active minibuffer (versus
the case I reported, where dedicated frame `*Completions*'
has no minibuffer)?

Previously this was sufficient: only a frame with a
the active minibuffer returned non-nil:

(cdr (assoc 'minibuffer (frame-parameters this-frame)))

Please see the code I use, in the attachment to an
earlier message.  Here, again, is the important
part of that code, showing the test I use:

(let* ((this-buffer-frames  (icicle-frames-on BUFFER))
       (this-frame          (car this-buffer-frames))
       mini-param)
  (unless
      (and this-frame
           (frame-visible-p this-frame)
           ;; Only one frame shows BUFFER.
           (null (cdr this-buffer-frames))

           ;; That frame has parameter `minibuffer'.
           (setq mini-param         ; <=============
                 (cdr
                  (assoc 'minibuffer
                         (frame-parameters this-frame))))
           ;; Param `minibuffer' is the active minibuffer.
           (eq mini-param (active-minibuffer-window))

           (save-window-excursion
             (select-frame this-frame)
             ;; Frame has only one window.
             (one-window-p t 'SELECTED-FRAME-ONLY)))
    ...))

I'm looking for an alternative test that will continue
to work.  (I'd prefer one test that works across Emacs
versions, but a test that works only for Emacs 26 and
later is OK too.)

In the case I cited in this bug report, `this-frame' in
that code is frame `*Completions*', which is dedicated
to buffer `*Completions*' and has no minibuffer.  That
frame is displayed via `special-display-buffer-names'.

But the code above is not just for my own use, i.e.,
with such a dedicated `*Completions*' frame.  It
needs to work also for other cases, including where
`*Completions*' is alone in its frame but that frame
has the active minibuffer.  That's what the `unless'
is for: the code should delete the frame in the case
I reported, but it should not delete the frame (or
the window) if the frame with `*Completions*' has
the active minibuffer.

Thanks for your help adjusting to this change.

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > So the meaning of frame-parameter `minibuffer' has changed.

Not really.  It should specify the minibuffer window used by that frame
if the frame doesn't have its own minibuffer window.  Otherwise, it's t
if this is a normal frame with its own minibuffer window and 'only if
it's a minibuffer-only frame.  nil only serves as an initial value where
it's up to Emacs to decide which minibuffer window to choose (something
it eventually may have to do anyway).

 > I will need to adjust my code somehow.
 >
 > How would you suggest I change the test I have been using,
 > to detect a frame that has the active minibuffer (versus
 > the case I reported, where dedicated frame `*Completions*'
 > has no minibuffer)?

I would try

(eq (minibuffer-selected-window) (frame-selected-window this-frame))

It's a pity that you were not around when I tried to discuss the
associated code here.  Maybe together we would have found a more
convenient solution.

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

>  > So the meaning of frame-parameter `minibuffer' has changed.
>  > I will need to adjust my code somehow.
> 
> Not really.  It should specify the minibuffer window used by that frame
> if the frame doesn't have its own minibuffer window.  Otherwise, it's t
> if this is a normal frame with its own minibuffer window and 'only if
> it's a minibuffer-only frame.  nil only serves as an initial value where
> it's up to Emacs to decide which minibuffer window to choose (something
> it eventually may have to do anyway).

Yes, that is what the doc says and has said.  The actual
meaning, i.e., what Emacs actually does, has changed.

But I don't wish to argue about whether the meaning
has changed - "meaning" can mean intended meaning or
behavior, and intended meaning can mean what was
coded or what was documented for it.

The actual behavior has changed.  I hope we can agree
on that wording.  And I will therefore need to adjust
my code.

>  > How would you suggest I change the test I have been using,
>  > to detect a frame that has the active minibuffer (versus
>  > the case I reported, where dedicated frame `*Completions*'
>  > has no minibuffer)?
> 
> I would try (eq (minibuffer-selected-window)
>                 (frame-selected-window this-frame))

I will try that.  At first sight it solves the problem.
As there are many different use cases I'll need to see
whether it works in all cases and doesn't break anything.

I suppose you can close this bug now.  I can follow up
later if it seems there is still a problem.  Thanks.

BTW, I find the doc string for `minibuffer-selected-window'
a bit confusing:

 Return the window which was selected when entering the minibuffer.
                         ^^^^^^^^^^^^^^^^^^^^^^^^^^
 Returns nil, if selected window is not a minibuffer window.

The phrase "window which was selected when entering" is
somewhat ambiguous.  It can easily be understood as the
window that was selected before the minibuffer was entered,
rather than the (minibuffer) window that is selected after
the minibuffer was entered.

I think it would be clearer to identify the "when" clearly
as after entering, not before - when "entering' is unclear,
especially when used with "was" selected.

(Also, it should probably say "Return nil..." instead of
"Returns nil...".)

> It's a pity that you were not around when I tried to discuss the
> associated code here.  Maybe together we would have found a more
> convenient solution.

I'm sorry I wasn't.  As you know, I'm no expert in this
(or any other Emacs) area.  I can only see a change in
behavior and then ask about that.  I didn't see the
change before.

If it could help, next time you might ask me to try a
proposed Lisp change that you suspect might affect code
I have.  I would try it and let you know.  If there are
changes in C (as in this case) then I won't be able to
help, but if it is Lisp only then I will likely be glad
to check.

I just looked at the emacs-devel thread you cited, and
I see that I did follow it at the time, and flagged
some of the messages as noteworthy.  I didn't have
anything particular to say about it, as I didn't know
how it might affect my code.  So I "was there"
(reading), though I was "not there" (speaking up).

As you yourself said then, you would need to wait for
reports of problems (regressions etc.).  And here is
one. ;-)

I just need something that works.  At first sight your
suggestion looks like it will fill the bill.  Thanks.

BTW, I don't see anything yet in NEWS about this change.
I consider this an incompatible change (you might not).
In any case, it should be called out, I think.  I see
a section called "New frame parameters and changed
semantics for older ones".  That might be a good place
to mention this.

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > The actual behavior has changed.  I hope we can agree
 > on that wording.

We can.

 >> I would try (eq (minibuffer-selected-window)
 >>                  (frame-selected-window this-frame))
 >
 > I will try that.  At first sight it solves the problem.
 > As there are many different use cases I'll need to see
 > whether it works in all cases and doesn't break anything.

You never said what you really wanted to achieve.  I guess that you want
to delete a frame around the time you do some minibuffer interaction and
base the decision of which frame to delete on whether it is the frame
initiating that interaction.  If so, then any code based on the value of
the 'minibuffer' parameter would have been already wrong before my
change as well - an arbitrary number of frames could have had the
'minibuffer' parameter set to nil and there would have been no way to
tell from that parameter which of them initiated the interaction.

 > BTW, I find the doc string for `minibuffer-selected-window'
 > a bit confusing:
 >
 >   Return the window which was selected when entering the minibuffer.
 >                           ^^^^^^^^^^^^^^^^^^^^^^^^^^
 >   Returns nil, if selected window is not a minibuffer window.
 >
 > The phrase "window which was selected when entering" is
 > somewhat ambiguous.  It can easily be understood as the
 > window that was selected before the minibuffer was entered,
 > rather than the (minibuffer) window that is selected after
 > the minibuffer was entered.

Exactly that's the meaning.  I'm afraid you're confusing things here.
The window returned by that function was the selected window until the
minibuffer interaction started.  When the minibuffer action terminates,
that function will return nil again.  The active minibuffer window is
obviously returned by ‘active-minibuffer-window’.

 > I think it would be clearer to identify the "when" clearly
 > as after entering, not before - when "entering' is unclear,
 > especially when used with "was" selected.
 >
 > (Also, it should probably say "Return nil..." instead of
 > "Returns nil...".)

Fixed, hopefully.  I also rewrote the related documentation (to some
extent).

 > BTW, I don't see anything yet in NEWS about this change.
 > I consider this an incompatible change (you might not).
 > In any case, it should be called out, I think.  I see
 > a section called "New frame parameters and changed
 > semantics for older ones".  That might be a good place
 > to mention this.

Done.

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

>  >> I would try (eq (minibuffer-selected-window)
>  >>                 (frame-selected-window this-frame))
>  >
>  > I will try that.  At first sight it solves the problem.
>  > As there are many different use cases I'll need to see
>  > whether it works in all cases and doesn't break anything.
> 
> You never said what you really wanted to achieve.  I guess that you want
> to delete a frame around the time you do some minibuffer interaction and
> base the decision of which frame to delete on whether it is the frame
> initiating that interaction.

I sent the `icicle-delete-windows-on', which includes
the doc string:

 "Delete all windows showing BUFFER.
  If such a window is alone in its frame, then delete
  the frame - unless it is the only frame or a standalone
  minibuffer frame."

It's about deleting _windows_.  It's only about deleting
frames if the windows to be deleted are alone in their
frames - such frames are the only frames that are deleted.

That's what I want to achieve.  Is it not clear?

As for when that function is called from Lisp in my code
(to see its use other than interactive), currently it is
called only in command `icicle-remove-Completions-window'.

But that function is called many times, throughout my
code.  A _typical_ use is to remove the display of
buffer `*Completions*' at the end of a given minibuffer
interaction.  A user can also invoke it during completion
using `C-x 0' - that's the interactive case.

"End of a given minibuffer interaction" might be at or
near the end of a command.  It might be at or near the
end of the use of a minibuffer.  But it might NOT be,
in the case of a recursive minibuffer, or even in the
case of a non-recursive minibuffer with which interaction
is not finished.  E.g., you show completions for `forw',
then you type `x', so your input is `forwx'.  Display
of `*Completions*' is removed because there are none.

`icicle-remove-Completions-window' is invoked to stop
showing `*Completions*' - wherever it might be shown;
nothing more.  And that can be appropriate in lots of
different contexts.  The code:

(defun icicle-remove-Completions-window (&optional force)
  "Remove the `*Completions*' window.
If not called interactively and `*Completions*' is the selected
window, then do not remove it unless optional arg FORCE is non-nil."
  (interactive)
  ;; Do nothing if `*Completions*' is the selected window or the
  ;; minibuffer window is selected and `*Completions*' window was
  ;; selected just before.
  (let ((swin  (selected-window)))
    (when (window-minibuffer-p swin)
      (setq swin  (minibuffer-selected-window)))
    (cond
      (;; `*Completions*' is shown in the selected frame.
       (and (get-buffer-window "*Completions*")
            (or force;; `C-g' gets rid of it even if selected
                (and (window-live-p swin)
                     (not (eq (window-buffer swin)
                              (get-buffer "*Completions*"))))
                (interactive-p)))
       (condition-case nil;; Ignore "Attempt to delete the..."
           (delete-window (get-buffer-window "*Completions*"))
         (error nil))
       (bury-buffer (get-buffer "*Completions*")))
      (;; `*Completions*' is shown in a different frame.
       (and (get-buffer-window "*Completions*" 'visible)
            (or force;; `C-g' gets rid of it even if selected
                (and (window-live-p swin)
                     (not (eq (window-buffer swin)
                              (get-buffer "*Completions*"))))
                (interactive-p)))
       (when (window-dedicated-p
              (get-buffer-window "*Completions*" 'visible))
         (condition-case nil
             (icicle-delete-windows-on "*Completions*")
           (error nil)))
       (bury-buffer (get-buffer "*Completions*"))))))

> If so, then any code based on the value of
> the 'minibuffer' parameter would have been already wrong before my
> change as well - an arbitrary number of frames could have had the
> 'minibuffer' parameter set to nil and there would have been no way to
> tell from that parameter which of them initiated the interaction.

No.  As I said, the code, as it was, works fine in all
Emacs releases.  It does not work in prerelease 26.1.

If multiple frames show the BUFFER (typically
`*Completions*') whose display is to be removed then
the behavior (intended and realized) is to remove all
such displays of BUFFER - i.e., all windows showing
BUFFER.

And no, there is no problem such as (I think) you
describe.  In my own setup (but the code is not just
for my setup) ALL of the frames except my standalone
minibuffer frame have a nil `minibuffer' parameter.
And none are mistakenly removed.  Only the windows
and frames showing BUFFER are affected, and only
windows showing BUFFER are removed.

>  > BTW, I find the doc string for `minibuffer-selected-window'
>  > a bit confusing:
>  >
>  >  Return the window which was selected when entering the minibuffer.
>  >                           ^^^^^^^^^^^^^^^^^^^^^^^^^^
>  >  Returns nil, if selected window is not a minibuffer window.
>  >
>  > The phrase "window which was selected when entering" is
>  > somewhat ambiguous.  It can easily be understood as the
>  > window that was selected before the minibuffer was entered,
>  > rather than the (minibuffer) window that is selected after
>  > the minibuffer was entered.
> 
> Exactly that's the meaning.  I'm afraid you're confusing things here.
> The window returned by that function was the selected window until the
> minibuffer interaction started.

Really?  The window that was selected before minibuffer
interaction?

I truly do not understand, in that case.  And that is
not what I think I see.  If what you say is true then
I should never see that the same minibuffer window 
as `active-minibuffer-window' (or even an inactive
minibuffer window) is itself returned by the function.

How can the minibuffer window that becomes selected
for the minibuffer interaction have been the selected 
window before "the minibuffer interaction started"?

I suppose a _different_ minibuffer window could be
returned, e.g., in the case of a recursive minibuffer.
But so far, what you just said, about the function
returning the window that is selected before the
minibuffer interaction starts, makes no sense to me.
I don't get that.

That description seems to apply to a window showing
`completion-reference-buffer'.  That is not what I
think I see.  Before `M-x', with buffer `foo.el'
current in the selected window, I don't think I ever
see that window as the value of function
`minibuffer-selected-window'.

And even if that is the actual meaning/behavior of
that function, the doc string is not appropriate.
In that case it is inappropriate because it allows
the other meaning: after instead of before.  Either
way, the doc string is misleading/ambiguous.

Let me ask you the same question you asked me about
my function for removing displays of a buffer: what
are the use cases for `minibuffer-selected-window'?
What is it intended for?  I thought I understood it
until your reply saying that it returns the window
selected _before_ the minibuffer interaction.

> When the minibuffer action terminates,
> that function will return nil again.  The active minibuffer 
> window is obviously returned by ‘active-minibuffer-window’.

Yes, `active-minibuffer-window' I understand, and
have been using.  It is `minibuffer-selected-window'
that was, and apparently still is, unclear to me.

>  > I think it would be clearer to identify the "when" clearly
>  > as after entering, not before - when "entering' is unclear,
>  > especially when used with "was" selected.
>  >
>  > (Also, it should probably say "Return nil..." instead of
>  > "Returns nil...".)
> 
> Fixed, hopefully.  I also rewrote the related documentation
> (to some extent).

Could you please post the fixed string here, so I
can see it?  Clearly I don't understand this yet.
Hopefully that will help.  Thx.

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > And no, there is no problem such as (I think) you
 > describe.  In my own setup (but the code is not just
 > for my setup) ALL of the frames except my standalone
 > minibuffer frame have a nil `minibuffer' parameter.
 > And none are mistakenly removed.  Only the windows
 > and frames showing BUFFER are affected, and only
 > windows showing BUFFER are removed.

Then I have difficulties to understand what this part of your code tries
to do:

                    (setq mini-param
                          (cdr
                           (assoc
                            'minibuffer
                            (frame-parameters this-frame))))
                    (eq mini-param (active-minibuffer-window))

You set mini-param to the 'minibuffer' parameter of this-frame which is
IIUC your *Completions* frame.  If you say that this parameter is always
nil for this-frame, why do you retrieve it?  If all you want to check is
whether this-frame is the active minibuffer window's frame, then there
should be easier ways to do that like, for example,

(and (active-minibuffer-window)
      (eq this-frame (window-frame (active-minibuffer-window))))

 > Really?  The window that was selected before minibuffer
 > interaction?

Yes.

 > I truly do not understand, in that case.  And that is
 > not what I think I see.  If what you say is true then
 > I should never see that the same minibuffer window
 > as `active-minibuffer-window' (or even an inactive
 > minibuffer window) is itself returned by the function.

Invoke

emacs -Q --eval "(setq default-frame-alist '((minibuffer . nil)))"

and switch to the minibuffer-less frame.  Type C-h f to enter the
minibuffer.  Once there, type

M-: (active-minibuffer-window)

This gets me the window of the minibuffer frame.  Now type

M-: (minibuffer-selected-window)

This gets me the window of the minibuffer-less frame.

What do you get?

 > How can the minibuffer window that becomes selected
 > for the minibuffer interaction have been the selected
 > window before "the minibuffer interaction started"?

I don't know.  It doesn't happen with the scenario sketched above.

 > And even if that is the actual meaning/behavior of
 > that function, the doc string is not appropriate.
 > In that case it is inappropriate because it allows
 > the other meaning: after instead of before.  Either
 > way, the doc string is misleading/ambiguous.

Please suggest a better one.

 > Let me ask you the same question you asked me about
 > my function for removing displays of a buffer: what
 > are the use cases for `minibuffer-selected-window'?
 > What is it intended for?  I thought I understood it
 > until your reply saying that it returns the window
 > selected _before_ the minibuffer interaction.

IIUC it's mainly used to find the original buffer from where the
minibuffer was entered.  Have a look at the Elisp sources.

 > Could you please post the fixed string here, so I
 > can see it?  Clearly I don't understand this yet.
 > Hopefully that will help.  Thx.

The doc-string is probably not much better:

        doc: /* Return the window which was selected when entering the minibuffer.
Return nil if the selected window is not a minibuffer window.  */)

The Elisp documentation now has

      This function returns the window that was selected at the moment
      the minibuffer was entered.  If the currently selected window is
      not a minibuffer window, it returns `nil'.

which you might want to improve as well.

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

>  > And no, there is no problem such as (I think) you
>  > describe.  In my own setup (but the code is not just
>  > for my setup) ALL of the frames except my standalone
>  > minibuffer frame have a nil `minibuffer' parameter.
>  > And none are mistakenly removed.  Only the windows
>  > and frames showing BUFFER are affected, and only
>  > windows showing BUFFER are removed.
> 
> Then I have difficulties to understand what this part of your code tries
> to do:
>                     (setq mini-param
>                           (cdr
>                            (assoc
>                             'minibuffer
>                             (frame-parameters this-frame))))
>                     (eq mini-param (active-minibuffer-window))
> 
> You set mini-param to the 'minibuffer' parameter of this-frame which is
> IIUC your *Completions* frame.

Yes.

> If you say that this parameter is always nil for this-frame,
> why do you retrieve it?

In my setup all frames except the minibuffer frame have no
minibuffer.

The code is not only for my setup.  It is Icicles code, so
it must handle any kind of setup.

> If all you want to check is whether this-frame is the active
> minibuffer window's frame, then there should be easier ways
> to do that like, for example,
>
> (and (active-minibuffer-window)
>      (eq this-frame (window-frame (active-minibuffer-window))))

Yes, that looks like it corresponds to the check I've
been doing - thanks.

I was checking the `minibuffer' parameter of THIS-FRAME,
to see if it was the `active-minibuffer-window'.  But it
should be just as good to check that the frame of the
`active-minibuffer-window' is THIS-FRAME.

I don't think the code you showed earlier corresponds to
the same thing.  IIUC, the test you suggested earlier
checks whether the window that was selected immediately
before the current minibuffer window was selected is the
same as the selected window of THIS-FRAME.  That's not
the same thing as what I need to test, AFAICT.  But your
latest suggestion seems to check what I've been checking,
and it should work OK in all Emacs versions.

>  > And even if that is the actual meaning/behavior of
>  > that function, the doc string is not appropriate.
>  > In that case it is inappropriate because it allows
>  > the other meaning: after instead of before.  Either
>  > way, the doc string is misleading/ambiguous.
> 
> Please suggest a better one.

 Return the window that was selected immediately
 before the current minibuffer window was selected.

One way or another, it should say that the window
returned was selected BEFORE the minibuffer window
was selected.  It is not the minibuffer window
(which can easily be understood as "the window
selected when entering the minibuffer").

>  > Could you please post the fixed string here, so I
>  > can see it?  Clearly I don't understand this yet.
>  > Hopefully that will help.  Thx.
> 
> The doc-string is probably not much better:
> 
>  Return the window which was selected when entering the minibuffer.
>  Return nil if the selected window is not a minibuffer window.

Right - not better.  Same problem as before: "when
entering" is ambiguous.

Some window is the selected window before entering, and
some window is the selected window after entering.  But
"when entering" means little - whether it is regard as
an instant or a time period, either way it's unclear
which window is meant - before or after the minibuffer
window becomes the selected window.

Plus there is the ambiguity of "the minibuffer" when
talking about minibuffer windows, since there can be
multiple minibuffer windows.  And a minibuffer window
could be selected before another minibuffer window
gets selected.  "When entering the minibuffer" tells
you nothing about which of those minibuffer windows
is the `minibuffer-selected-window'.

> The Elisp documentation now has
> 
>       This function returns the window that was selected
>       at the moment
        ^^^^^^^^^^^^^
>       the minibuffer was entered.  If the currently selected
>       window isnot a minibuffer window, it returns `nil'.
> 
> which you might want to improve as well.

Same problem; same solution - see above.  At the moment
the change is made, which window is the selected window?

My suggestion is to say that it is the window that
was selected JUST BEFORE the minibuffer was entered.

Thanks.

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > I was checking the `minibuffer' parameter of THIS-FRAME,
 > to see if it was the `active-minibuffer-window'.  But it
 > should be just as good to check that the frame of the
 > `active-minibuffer-window' is THIS-FRAME.
 >
 > I don't think the code you showed earlier corresponds to
 > the same thing.  IIUC, the test you suggested earlier
 > checks whether the window that was selected immediately
 > before the current minibuffer window was selected is the
 > same as the selected window of THIS-FRAME.  That's not
 > the same thing as what I need to test, AFAICT.  But your
 > latest suggestion seems to check what I've been checking,
 > and it should work OK in all Emacs versions.

If you told me with plain words what you really wanted to check then I
might come up with another suggestion.  But "checking the `minibuffer'
parameter of THIS-FRAME, to see if it was the
`active-minibuffer-window'" could not have possibly done anything
reasonable even before my changes.  That test simply failed/succeeded
accidentally with Emacs 25 and just produces the opposite result with
Emacs 26.

 >   Return the window that was selected immediately
 >   before the current minibuffer window was selected.

Looks good but I need a first line that fits into 67 characters.  Pretty
please suggest a suitable first line and whatever you consider important
for the rest.  Also, since we nowhere specify "the current minibuffer
window" wouldn't "the currently active minibuffer window" be better?

martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

>  > I was checking the `minibuffer' parameter of THIS-FRAME,
>  > to see if it was the `active-minibuffer-window'.  But it
>  > should be just as good to check that the frame of the
>  > `active-minibuffer-window' is THIS-FRAME.
>  >
>  > I don't think the code you showed earlier corresponds to
>  > the same thing.  IIUC, the test you suggested earlier
>  > checks whether the window that was selected immediately
>  > before the current minibuffer window was selected is the
>  > same as the selected window of THIS-FRAME.  That's not
>  > the same thing as what I need to test, AFAICT.  But your
>  > latest suggestion seems to check what I've been checking,
>  > and it should work OK in all Emacs versions.
> 
> If you told me with plain words what you really wanted to check then I
> might come up with another suggestion.  But "checking the `minibuffer'
> parameter of THIS-FRAME, to see if it was the
> `active-minibuffer-window'" could not have possibly done anything
> reasonable even before my changes.  That test simply failed/succeeded
> accidentally with Emacs 25 and just produces the opposite result with
> Emacs 26.

It not only could possibly have done something reasonable
before your changes, it did something reasonable, useful,
and necessary before your changes.  And as I said: in ALL
Emacs releases.

I think the alternative code you proposed will likely work
as well (thank you), but there is no reason to claim that
the previous code did not work or did nothing reasonable.

Before your change, a separate *Completions* frame didn't
have a minibuffer-window value for its frame parameter
`minibuffer' - it used another frame's minibuffer.  Before
your change checking that parameter was a reasonable way
to check whether the frame had an active minibuffer window.

>  > Return the window that was selected immediately
>  > before the current minibuffer window was selected.
> 
> Looks good but I need a first line that fits into 67 characters.  Pretty
> please suggest a suitable first line and whatever you consider important
> for the rest.  Also, since we nowhere specify "the current minibuffer
> window" wouldn't "the currently active minibuffer window" be better?

 Return window selected just before minibuffer window was selected.

or even

 Return window selected just before minibuffer window.

Or if this is about an _active_ minibuffer then perhaps:

 Return window selected just before window of active minibuffer.

I don't think "currently active minibuffer window" is
the same thing as currently selected minibuffer window.

For one thing, a window is not "active".  (Nor is it
"current", you will say, and that's right.)  You probably
meant "window of the active minibuffer".

For another thing, if a minibuffer window is selected
does that necessarily mean that the minibuffer is active?
I don't think so, but I'm not an expert in this stuff.

Thanks for fixing this.

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[martin rudalics]

 > Before your change, a separate *Completions* frame didn't
 > have a minibuffer-window value for its frame parameter
 > `minibuffer' - it used another frame's minibuffer.  Before
 > your change checking that parameter was a reasonable way
 > to check whether the frame had an active minibuffer window.

Your check does not do that.  It tries to check whether this-frame has
no minibuffer window and no minibuffer window is active.  Given the
Emacs 25 manual which says about the 'minibuffer' parameter

      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.

your check was not reasonable.  A reasonable equivalent check working
for Emacs 25 and Emacs 26 could be

(and (not (memq (frame-parameter this-frame 'minibuffer) '(t only)))
      (not (active-minibuffer-window)))

 >   Return window selected just before minibuffer window was selected.

I used this.

 > I don't think "currently active minibuffer window" is
 > the same thing as currently selected minibuffer window.
 >
 > For one thing, a window is not "active".  (Nor is it
 > "current", you will say, and that's right.)  You probably
 > meant "window of the active minibuffer".

I tried to change the documentation in that sense.

 > For another thing, if a minibuffer window is selected
 > does that necessarily mean that the minibuffer is active?
 > I don't think so, but I'm not an expert in this stuff.

You can always switch to a minibuffer window without making its buffer
active.

 > Thanks for fixing this.

Thanks for the suggestions, martin

bug#28978: 26.0; Regression: separate, dedicated `*Completions*' frame no longer has parameter `minibuffer'

[Drew Adams]

>  > Before your change, a separate *Completions* frame didn't
>  > have a minibuffer-window value for its frame parameter
>  > `minibuffer' - it used another frame's minibuffer.  Before
>  > your change checking that parameter was a reasonable way
>  > to check whether the frame had an active minibuffer window.
> 
> Your check does not do that.  It tries to check whether this-frame has
> no minibuffer window and no minibuffer window is active.  Given the
> Emacs 25 manual which says about the 'minibuffer' parameter
> 
>       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.

Yes, the doc has said that since at least Emacs 22.  But it
has not been true until now, AFAICT.  In the case in point,
my *Completions* frame uses my standalone minibuffer frame,
but parameter `minibuffer' in *Completions* remained nil,
even during completion (i.e., during use of the active
minibuffer).