Subject: RE: bug#11939: 24.1; `save-buffers-kill-emacs'losesminibufferfocuswhenitcalls`list-processes'

From: "Drew Adams" <[email-masked]>
Date: Tue, 14 Aug 2012 07:36:45 -0700

>  > (save-selected-window
>  >   (select-window (minibuffer-window))
>  >   (one-window-p nil 'selected-frame))
> 
> Any reason why you don't use
> (one-window-p nil (window-frame (minibuffer-window)))
> here?

The doc for `one-window-p', for Emacs < 24, says this:

 "If ALL-FRAMES is neither nil nor t, count only the selected frame."

To me, that says that the frame must be _selected_.  It says that it makes no
difference if the non-nil & non-t value is itself a frame: the SELECTED frame is
used even if you pass a different frame as arg.

The Emacs 24 doc string says something _very_ different - something that accords
with your suggestion.  It says that a value that is a _frame_ means:

 "consider all windows on that frame only."

Here, "that frame" is presumably the frame passed as argument.

However, I wonder if that is correct.  The _code_ itself is unchanged from Emacs
20 through Emacs 24.  Yet the doc string says something very different suddenly.

I see nothing in the `one-window-p' code that deals with a frame value, or a
`visible' value for that matter.  Perhaps these special cases are handled by one
of the functions it calls?

Can you explain?  Is the Emacs 24 behavior actually different from before, or is
it just the doc that is different?  Is the Emacs 24 doc string correct for 24?
Is the 24 doc string perhaps correct also for older releases?  (I see nothing in
NEWS about a behavior change.)

If your suggestion is appropriate, including for older releases, then I'll be
glad to adopt it.  Please let me know.

Note: I have this comment in the code just before the call to `one-window-p'.  I
do not recall the issue, but the comment has been there for quite a while:

;; We should be able to use just (one-window-p),
;; but an Emacs bug means we need this:
(one-window-p nil 'selected-frame)))

Does that ring a bell?  Even if that bug has since been fixed, I will presumably
need to keep that, for older releases.  Just wondering if you recall anything
about that bug.

>  > The call to `save-selected-window' somehow gave the focus 
>  > to the *Completions* frame.  The solution was to change
>  > `save-selected-window' to `save-window-excursion' here.
>  > I don't know why, but I'm hoping you will.
> 
> You mean that `save-select[ed]-window' can redirect focus and not 
> direct it back to where it was before calling it?

That's certainly what it seems like.  And there is definitely a difference here
wrt what `save-window-excursion' does (that function DTRT here).  Perhaps this
info gives you a starting point for investigating.

> If this is the case, please check whether it happens in Emacs 23
> as well and file a bug report (without referencing
> `1on1-fit-minibuffer-frame', if possible).

Yes, AFAICT the behavior is the same in all Emacs versions.

Do you have a suggestion for a simpler test?  If so, perhaps you can test it.

>  > 2. This was called near the beginning of the 
>  >    `1on1-fit-minibuffer-frame' body:
>  >
>  > (let* ((frame (save-selected-window
>  >                 (select-window (minibuffer-window))
>  >                 (selected-frame)))
> 
> Do you mean (window-frame (minibuffer-window)) here?

I suppose so.  _Should_ that make a difference?

Certainly that is lighter weight than `save-window-excursion', and it seems to
work OK so far.  Thanks for the suggestion.  Please let me know about the other
occurrence (above) - I would like to change that also, if your suggestion will
work there as well (for all Emacs versions).

>  > 3. At the end of the `1on1-fit-minibuffer-frame' body 
>  > there is this call:
>  > (1on1-set-minibuffer-frame-top/bottom)
>  >
>  > The code for that function is this:
>  >
>  > (defun 1on1-set-minibuffer-frame-top/bottom ()
>  >   (when 1on1-minibuffer-frame
>  >     (condition-case nil
>  >         (if (fboundp 'redisplay)
>  > 	    (redisplay t)
>  > 	  (force-mode-line-update t))
>  >       (error nil)) ; Ignore errors from, e.g., killed buffers.
>  >     (modify-frame-parameters
>  >      1on1-minibuffer-frame
>  >      `((top ,@ (or 1on1-minibuffer-frame-top/bottom
>  >                    (- (* 2 (frame-char-height
>  >                              1on1-minibuffer-frame)))))))))
>  >
>  > Here the problem was the call to `redisplay'.  I changed 
>  > (foundp 'redisplay) to just nil to fix things -
>  > `force-mode-line-update' works without changing the focus.
>  >
>  > `redisplay' is defined in C code.  I do not know why it 
>  > causes a problem here (why would redisplay change the input
>  > focus?), and I don't know what the proper solution might be,
>  > except to just use `force-mode-line-update' as I was already
>  > doing in older Emacs versions (which do not have `redisplay').
>  >
>  > Do you have an idea about this?  Does any of this make 
>  > sense to you?
> 
> No.  Can't you distill a simple test case?  `redisplay' shouldn't care
> about frame focus either.

You are saying the same thing I am.  Can you distill a simpler test case?

>  > Each of these fixes is needed for 
>  > `1on1-fit-minibuffer-frame'.  Reverting any of
>  > them brings back the problem (giving *Completions* the 
>  > focus).  I do not understand any of these fixes (why they work),
>  > and I am certainly not claiming anything about them or about
>  > the problem, which I do not understand.  I am only
>  > saying that together these changes fix the problem I encountered.
>  >
>  > But if you can explain things I will be glad to learn.
> 
> I can't explain any of these.  In the past, I tried to make 
> most window functions work on any window/frame to work independently
> from the selected window.  Earlier, selecting a window must have
> been a very simple procedure.  Nowadays, this incurs so many
> side-effects in the window/frame/display area that you should
> try to avoid `select-window' wherever possible.

You seem to be saying that Emacs has introduced bugs, and nothing more.  OK, so
I hear the admonition to avoid `select-window' wherever possible.  That's a
sorry state, though.  You seem to be saying that things that used to work, and
work simply, are now so bugged or complicated that all we can advise is to avoid
using `select-window'.

Anyway, my updated code seems to take care of the problem I encountered.

I would like to know about `one-window-p', however.  Is the Emacs 24 doc string
wrong?  The code is identical to the code for Emacs 20.7, but suddenly we have a
very different behavior description.  Is the new description correct for Emacs
24 plus older releases?  That would be good news (just an old-doc bug).  For the
moment, the situation is not clear.