bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

This regression was introduced after a build from 2013-08-08.  It
appears in the build cited below, from 2013-08-18.  It makes Emacs
unusable (by me).

I have this as `after-make-frame-functions': `(fit-frame)', in order to
fit the new frame to its displayed buffer.'

When `after-make-frame-functions' is run in `make-frame' now, the
original frame, not the newly created frame, is selected when it runs.
So `fit-frame' is called with the original (wrong) frame selected.

Here is a snapshot from the debugger:

Debugger entered--entering a function:
* fit-frame(#<frame drews-lisp-20 036212e0>)
* run-hook-with-args(fit-frame #<frame drews-lisp-20 036212e0>)
* (let* ((display (cdr (assq (quote display) parameters))) (w (cond ((assq =
(quote terminal) parameters) (let ((type ...)) (cond (... nil) (... ...) (t=
 type)))) ((assq (quote window-system) parameters) (cdr (assq (quote window=
-system) parameters))) (display (or (window-system-for-display display) (er=
ror "Don't know how to interpret display \"%S\"" display))) (t window-syste=
m))) (frame-creation-function (cdr (assq w frame-creation-function-alist)))=
 (oldframe (selected-frame)) (params parameters) frame) (if frame-creation-=
function nil (error "Don't know how to create a frame on window system %s" =
w)) (if (get w (quote window-system-initialized)) nil (funcall (cdr (assq w=
 window-system-initialization-alist)) display) (setq x-display-name display=
) (put w (quote window-system-initialized) t)) (progn (let ((--dolist-tail-=
- (cdr (assq w window-system-default-frame-alist))) p) (while --dolist-tail=
-- (setq p (car --dolist-tail--)) (if (assq (car p) params) nil (setq param=
s (cons p params))) (setq --dolist-tail-- (cdr --dolist-tail--))))) (progn =
(let ((--dolist-tail-- default-frame-alist) p) (while --dolist-tail-- (setq=
 p (car --dolist-tail--)) (if (assq (car p) params) nil (setq params (cons =
p params))) (setq --dolist-tail-- (cdr --dolist-tail--))))) (run-hooks (quo=
te before-make-frame-hook)) (setq frame (funcall frame-creation-function pa=
rams)) (normal-erase-is-backspace-setup-frame frame) (progn (let ((--dolist=
-tail-- frame-inherited-parameters) param) (while --dolist-tail-- (setq par=
am (car --dolist-tail--)) (if (assq param parameters) nil (let ((val ...)) =
(if val (progn ...)))) (setq --dolist-tail-- (cdr --dolist-tail--))))) (run=
-hook-with-args (quote after-make-frame-functions) frame) frame)
* (lambda (&optional parameters) "Return a newly created frame displaying t=
he current buffer.\nOptional argument PARAMETERS is an alist of frame param=
eters for\nthe new frame.  Each element of PARAMETERS should have the\nform=
 (NAME . VALUE), for example:\n\n (name . STRING)	The frame should be named=
 STRING.\n\n (width . NUMBER)	The frame should be NUMBER characters in widt=
h.\n (height . NUMBER)	The frame should be NUMBER text lines high.\n\nYou c=
annot specify either `width' or `height', you must specify\nneither or both=
.\n\n (minibuffer . t)	The frame should have a minibuffer.\n (minibuffer . =
nil)	The frame should have no minibuffer.\n (minibuffer . only)	The frame s=
hould contain only a minibuffer.\n (minibuffer . WINDOW)	The frame should u=
se WINDOW as its minibuffer window.\n\n (window-system . nil)	The frame sho=
uld be displayed on a terminal device.\n (window-system . x)	The frame shou=
ld be displayed in an X window.\n\n (display . \":0\")     The frame should=
 appear on display :0.\n\n (terminal . TERMINAL)  The frame should use the =
terminal object TERMINAL.\n\nIn addition, any parameter specified in `defau=
lt-frame-alist',\nbut not present in PARAMETERS, is applied.\n\nBefore crea=
ting the frame (via `frame-creation-function-alist'),\nthis function runs t=
he hook `before-make-frame-hook'.  After\ncreating the frame, it runs the h=
ook `after-make-frame-functions'\nwith one arg, the newly created frame.\n\=
nIf a display parameter is supplied and a window-system is not,\nguess the =
window-system from the display.\n\nOn graphical displays, this function doe=
s not itself make the new\nframe the selected frame.  However, the window s=
ystem may select\nthe new frame according to its own rules." (interactive) =
(let* ((display (cdr (assq (quote display) parameters))) (w (cond ((assq (q=
uote terminal) parameters) (let (...) (cond ... ... ...))) ((assq (quote wi=
ndow-system) parameters) (cdr (assq ... parameters))) (display (or (window-=
system-for-display display) (error "Don't know how to interpret display \"%=
S\"" display))) (t window-system))) (frame-creation-function (cdr (assq w f=
rame-creation-function-alist))) (oldframe (selected-frame)) (params paramet=
ers) frame) (if frame-creation-function nil (error "Don't know how to creat=
e a frame on window system %s" w)) (if (get w (quote window-system-initiali=
zed)) nil (funcall (cdr (assq w window-system-initialization-alist)) displa=
y) (setq x-display-name display) (put w (quote window-system-initialized) t=
)) (progn (let ((--dolist-tail-- (cdr (assq w window-system-default-frame-a=
list))) p) (while --dolist-tail-- (setq p (car --dolist-tail--)) (if (assq =
(car p) params) nil (setq params (cons p params))) (setq --dolist-tail-- (c=
dr --dolist-tail--))))) (progn (let ((--dolist-tail-- default-frame-alist) =
p) (while --dolist-tail-- (setq p (car --dolist-tail--)) (if (assq (car p) =
params) nil (setq params (cons p params))) (setq --dolist-tail-- (cdr --dol=
ist-tail--))))) (run-hooks (quote before-make-frame-hook)) (setq frame (fun=
call frame-creation-function params)) (normal-erase-is-backspace-setup-fram=
e frame) (progn (let ((--dolist-tail-- frame-inherited-parameters) param) (=
while --dolist-tail-- (setq param (car --dolist-tail--)) (if (assq param pa=
rameters) nil (let (...) (if val ...))) (setq --dolist-tail-- (cdr --dolist=
-tail--))))) (run-hook-with-args (quote after-make-frame-functions) frame) =
frame))(nil)
* apply((lambda (&optional parameters) "Return a newly created frame displa=
ying the current buffer.\nOptional argument PARAMETERS is an alist of frame=
 parameters for\nthe new frame.  Each element of PARAMETERS should have the=
\nform (NAME . VALUE), for example:\n\n (name . STRING)	The frame should be=
 named STRING.\n\n (width . NUMBER)	The frame should be NUMBER characters i=
n width.\n (height . NUMBER)	The frame should be NUMBER text lines high.\n\=
nYou cannot specify either `width' or `height', you must specify\nneither o=
r both.\n\n (minibuffer . t)	The frame should have a minibuffer.\n (minibuf=
fer . nil)	The frame should have no minibuffer.\n (minibuffer . only)	The f=
rame should contain only a minibuffer.\n (minibuffer . WINDOW)	The frame sh=
ould use WINDOW as its minibuffer window.\n\n (window-system . nil)	The fra=
me should be displayed on a terminal device.\n (window-system . x)	The fram=
e should be displayed in an X window.\n\n (display . \":0\")     The frame =
should appear on display :0.\n\n (terminal . TERMINAL)  The frame should us=
e the terminal object TERMINAL.\n\nIn addition, any parameter specified in =
`default-frame-alist',\nbut not present in PARAMETERS, is applied.\n\nBefor=
e creating the frame (via `frame-creation-function-alist'),\nthis function =
runs the hook `before-make-frame-hook'.  After\ncreating the frame, it runs=
 the hook `after-make-frame-functions'\nwith one arg, the newly created fra=
me.\n\nIf a display parameter is supplied and a window-system is not,\ngues=
s the window-system from the display.\n\nOn graphical displays, this functi=
on does not itself make the new\nframe the selected frame.  However, the wi=
ndow system may select\nthe new frame according to its own rules." (interac=
tive) (let* ((display (cdr (assq (quote display) parameters))) (w (cond ((a=
ssq (quote terminal) parameters) (let (...) (cond ... ... ...))) ((assq (qu=
ote window-system) parameters) (cdr (assq ... parameters))) (display (or (w=
indow-system-for-display display) (error "Don't know how to interpret displ=
ay \"%S\"" display))) (t window-system))) (frame-creation-function (cdr (as=
sq w frame-creation-function-alist))) (oldframe (selected-frame)) (params p=
arameters) frame) (if frame-creation-function nil (error "Don't know how to=
 create a frame on window system %s" w)) (if (get w (quote window-system-in=
itialized)) nil (funcall (cdr (assq w window-system-initialization-alist)) =
display) (setq x-display-name display) (put w (quote window-system-initiali=
zed) t)) (progn (let ((--dolist-tail-- (cdr (assq w window-system-default-f=
rame-alist))) p) (while --dolist-tail-- (setq p (car --dolist-tail--)) (if =
(assq (car p) params) nil (setq params (cons p params))) (setq --dolist-tai=
l-- (cdr --dolist-tail--))))) (progn (let ((--dolist-tail-- default-frame-a=
list) p) (while --dolist-tail-- (setq p (car --dolist-tail--)) (if (assq (c=
ar p) params) nil (setq params (cons p params))) (setq --dolist-tail-- (cdr=
 --dolist-tail--))))) (run-hooks (quote before-make-frame-hook)) (setq fram=
e (funcall frame-creation-function params)) (normal-erase-is-backspace-setu=
p-frame frame) (progn (let ((--dolist-tail-- frame-inherited-parameters) pa=
ram) (while --dolist-tail-- (setq param (car --dolist-tail--)) (if (assq pa=
ram parameters) nil (let (...) (if val ...))) (setq --dolist-tail-- (cdr --=
dolist-tail--))))) (run-hook-with-args (quote after-make-frame-functions) f=
rame) frame)) nil)
* make-frame(nil)
  (lambda nil (make-frame pop-up-frame-alist))()
  display-buffer-pop-up-frame(#<buffer autofit-frame.el> ((inhibit-same-win=
dow . t)))
  display-buffer--maybe-pop-up-frame-or-window(#<buffer autofit-frame.el> (=
(inhibit-same-window . t)))
  display-buffer(#<buffer autofit-frame.el> t)
  pop-to-buffer(#<buffer autofit-frame.el> t nil)
  switch-to-buffer-other-window(#<buffer autofit-frame.el>)
  find-file-other-window("~/drews-lisp-20/autofit-frame.el" WILDCARDS)


In GNU Emacs 24.3.50.1 (i686-pc-mingw32)
 of 2013-08-17 on ODIEONE
Bzr revision: 113938 eliz@gnu.org-20130817171807-fxigtkbc6yy8m9iw
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --prefix=3D/c/Devel/emacs/binary --enable-checking=3Dyes,glyphs
 CFLAGS=3D-O0 -g3 LDFLAGS=3D-Lc:/Devel/emacs/lib
 CPPFLAGS=3D-Ic:/Devel/emacs/include'

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

HTH: There was no code change in frame.el itself between the two builds
cited, so the regression must have been introduced by other code.

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[martin rudalics]

 > This regression was introduced after a build from 2013-08-08.  It
 > appears in the build cited below, from 2013-08-18.  It makes Emacs
 > unusable (by me).
 >
 > I have this as `after-make-frame-functions': `(fit-frame)', in order to
 > fit the new frame to its displayed buffer.'
 >
 > When `after-make-frame-functions' is run in `make-frame' now, the
 > original frame, not the newly created frame, is selected when it runs.
 > So `fit-frame' is called with the original (wrong) frame selected.

If it can be fixed by commenting in the second line of the following
comment in `pop-to-buffer'

   ;; This should be done by `select-window' below.
   ;; (set-buffer buffer)

we have the following problem: Your `fit-frame' will work when using
`pop-to-buffer' but not when using `display-buffer'.  So maybe it's
better to do the funcall in `display-buffer-pop-up-frame' as

	       (with-current-buffer buffer
		 (setq frame (funcall fun)))

Please try them both, tell me whether they work for you, and, if so,
which one you prefer.

Thanks, martin

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

>  > This regression was introduced after a build from 2013-08-08.  It
>  > appears in the build cited below, from 2013-08-18.  It makes Emacs
>  > unusable (by me).
>  >
>  > I have this as `after-make-frame-functions': `(fit-frame)', in order to
>  > fit the new frame to its displayed buffer.'
>  >
>  > When `after-make-frame-functions' is run in `make-frame' now, the
>  > original frame, not the newly created frame, is selected when it runs.
>  > So `fit-frame' is called with the original (wrong) frame selected.
> 
> If it can be fixed by commenting in the second line of the following
> comment in `pop-to-buffer'
> 
>    ;; This should be done by `select-window' below.
>    ;; (set-buffer buffer)
> 
> we have the following problem: Your `fit-frame' will work when using
> `pop-to-buffer' but not when using `display-buffer'.  So maybe it's
> better to do the funcall in `display-buffer-pop-up-frame' as
> 
> 	       (with-current-buffer buffer
> 		 (setq frame (funcall fun)))
> 
> Please try them both, tell me whether they work for you, and, if so,
> which one you prefer.

Sorry, I don't understand.  What "both" would you like me to try?  This
needs to work as it did before the regression - both `pop-to-buffer'
and `display-buffer'.

But first, I don't understand either why there should be any difference.
Why shouldn't functions on `after-make-frame-functions' always be passed
the new frame as argument, as has been the case in the past?  There is a
`before-make-frame-functions' hook for passing the originally selected
frame.

Perhaps you are thinking that this is about _selecting_ the new frame?
(I mistakenly mentioned "is selected" above, when I meant is passed to
the hook functions.)

I can understand that `pop-to-buffer' and `display-buffer' might act
differently wrt selecting the buffer's frame.  But I do not understand
why suddenly the functions on hook `after-make-frame-functions' should
be passed the original frame as arg instead of the new frame.

How else can someone invoke a function on the new frame as part of the
process of frame creation?

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[martin rudalics]

 > Sorry, I don't understand.  What "both" would you like me to try?  This
 > needs to work as it did before the regression - both `pop-to-buffer'
 > and `display-buffer'.

Did `display-buffer' work correctly?

 > But first, I don't understand either why there should be any difference.
 > Why shouldn't functions on `after-make-frame-functions' always be passed
 > the new frame as argument, as has been the case in the past?  There is a
 > `before-make-frame-functions' hook for passing the originally selected
 > frame.

The problem is that the new frame doesn't yet show the buffer you want
to display when `after-make-frame-functions' is called.

 > Perhaps you are thinking that this is about _selecting_ the new frame?
 > (I mistakenly mentioned "is selected" above, when I meant is passed to
 > the hook functions.)
 >
 > I can understand that `pop-to-buffer' and `display-buffer' might act
 > differently wrt selecting the buffer's frame.  But I do not understand
 > why suddenly the functions on hook `after-make-frame-functions' should
 > be passed the original frame as arg instead of the new frame.

Do they really get passed the original frame?

 > How else can someone invoke a function on the new frame as part of the
 > process of frame creation?

Please check again.

martin

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

>  > Sorry, I don't understand.  What "both" would you like me to try?  This
>  > needs to work as it did before the regression - both `pop-to-buffer'
>  > and `display-buffer'.
> 
> Did `display-buffer' work correctly?

Not sure what you mean.  I haven't seen a problem until this build.
Hence the report of this being a regression.

>  > But first, I don't understand either why there should be any difference.
>  > Why shouldn't functions on `after-make-frame-functions' always be passed
>  > the new frame as argument, as has been the case in the past?  There is a
>  > `before-make-frame-functions' hook for passing the originally selected
>  > frame.
> 
> The problem is that the new frame doesn't yet show the buffer you want
> to display when `after-make-frame-functions' is called.

I see.  So you are saying that the new frame object is passed to the hook
functions, but that new frame has not yet been displayed.  If so, that is
presumably the cause of the regression.

What's the point of passing the newly created frame object to hook functions
intended to act on it, if that frame has not yet been displayed so they can
do so?

Perhaps you are allowing for hook functions that do not assume the frame is
displayed.  Is that the point of this change?  Should I change the hook
function here to, say, (lambda (fr) (raise-frame fr) (fit-frame fr))?  Or
perhaps `make-frame-visible' instead of `raise-frame'?

I just tried those, and they does not work either.  If I want to apply a
function such as `fit-frame' to the new frame, and it is not yet displayed,
what do I need to call in the hook function to display it first?

The doc string of `make-frame' suggests, BTW, that it should both (a) make
a frame object and (b) display it, as I have always thought it did do and
should do.  It says: "Return a newly created frame displaying the current
buffer."

You will perhaps say that the PARAMETERS argument could include `invisible'
or `iconified' or some such that has the effect of creating a frame that
is not visible (displayed).  If that is the idea behind this change then
I might not have anything against it, provided the frame is in fact
displayed when PARAMETERS does not do something to make it invisible etc.

IOW, in the use cases I have, an ordinary frame is created displayed, and
after that happens I want to fit the frame.  `after-make-frame-functions'
has always been the right place to do that, in the past.  Seems like this
is being redefined now. 

Please advise.  This should be simple.  And it's still not clear to me
why it should not be as it was before (i.e., since forever).

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[martin rudalics]

 >> Did `display-buffer' work correctly?
 >
 > Not sure what you mean.  I haven't seen a problem until this build.
 > Hence the report of this being a regression.

So you didn't check?

 >> The problem is that the new frame doesn't yet show the buffer you want
 >> to display when `after-make-frame-functions' is called.
 >
 > I see.  So you are saying that the new frame object is passed to the hook
 > functions, but that new frame has not yet been displayed.  If so, that is
 > presumably the cause of the regression.

No.  I am saying that at the time `after-make-frame-functions' is
called, the new frame does not yet show the buffer you want to display
in it via plain `display-buffer'.

 > What's the point of passing the newly created frame object to hook functions
 > intended to act on it, if that frame has not yet been displayed so they can
 > do so?
 >
 > Perhaps you are allowing for hook functions that do not assume the frame is
 > displayed.  Is that the point of this change?  Should I change the hook
 > function here to, say, (lambda (fr) (raise-frame fr) (fit-frame fr))?  Or
 > perhaps `make-frame-visible' instead of `raise-frame'?
 >
 > I just tried those, and they does not work either.  If I want to apply a
 > function such as `fit-frame' to the new frame, and it is not yet displayed,
 > what do I need to call in the hook function to display it first?

You want to apply `fit-frame' to the buffer you eventually want to
display on the new frame.  Right?

 > The doc string of `make-frame' suggests, BTW, that it should both (a) make
 > a frame object and (b) display it, as I have always thought it did do and
 > should do.  It says: "Return a newly created frame displaying the current
 > buffer."

The _current_ buffer.

 > You will perhaps say that the PARAMETERS argument could include `invisible'
 > or `iconified' or some such that has the effect of creating a frame that
 > is not visible (displayed).  If that is the idea behind this change then
 > I might not have anything against it, provided the frame is in fact
 > displayed when PARAMETERS does not do something to make it invisible etc.
 >
 > IOW, in the use cases I have, an ordinary frame is created displayed, and
 > after that happens I want to fit the frame.  `after-make-frame-functions'
 > has always been the right place to do that, in the past.  Seems like this
 > is being redefined now.
 >
 > Please advise.  This should be simple.  And it's still not clear to me
 > why it should not be as it was before (i.e., since forever).

Because the "since forever" behavior is inconsistent.  Consider the
following forms:

(defun mess (frame)
   (message "selected: %s ... frame: %s ... buffer: %s"
	   (selected-frame) frame
	   (window-buffer (frame-root-window frame))))

(let ((pop-up-frames t))
   (add-hook 'after-make-frame-functions 'mess)
   (display-buffer "*Messages*")
   (remove-hook 'after-make-frame-functions 'mess))

(let ((pop-up-frames t))
   (add-hook 'after-make-frame-functions 'mess)
   (pop-to-buffer "*Messages*")
   (remove-hook 'after-make-frame-functions 'mess))

With emacs -Q evaluate the first and and then try the other two with
some older version and the current trunk.  You should notice that the
FRAME argument of `mess' is always different from the selected frame.
But the buffer FRAME displays is different.  With the older versions the
buffer is *scratch* for plain `display-buffer' and *Messages* for
`pop-to-buffer'.  With current trunk it is *scratch* for both.

So I suppose that since forever your `fit-frame' function works on the
"wrong" buffer when you call plain `display-buffer' and the buffer you
want to display is not current at that time.

What I propose is to use the following as substitute for the old
`display-buffer-pop-up-frame':

(defun display-buffer-pop-up-frame (buffer alist)
   "Display BUFFER in a new frame.
This works by calling `pop-up-frame-function'.  If successful,
return the window used; otherwise return nil.

If ALIST has a non-nil `inhibit-switch-frame' entry, avoid
raising the new frame.

If ALIST has a non-nil `pop-up-frame-parameters' entry, the
corresponding value is an alist of frame parameters to give the
new frame."
   (let* ((params (cdr (assq 'pop-up-frame-parameters alist)))
	 (pop-up-frame-alist (append params pop-up-frame-alist))
	 (fun pop-up-frame-function)
	 frame window)
     (when (and fun
	       (with-current-buffer buffer
		 (setq frame (funcall fun)))
	       (setq window (frame-selected-window frame)))
       (prog1 (window--display-buffer
	      buffer window 'frame alist display-buffer-mark-dedicated)
	(unless (cdr (assq 'inhibit-switch-frame alist))
	  (window--maybe-raise-frame frame))))))

With this the buffer printed by `mess' should be *Messages* for both,
the plain `display-buffer' case and `pop-to-buffer'.

martin

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

Replied in order, as I read.  If you want to cut to the chase, see the end.

>  >> Did `display-buffer' work correctly?
>  >
>  > Not sure what you mean.  I haven't seen a problem until this build.
>  > Hence the report of this being a regression.
> 
> So you didn't check?

Can you please tell me what to check, and how?  It's not clear to me what
you are asking/suggesting; sorry.

>  >> The problem is that the new frame doesn't yet show the buffer you want
>  >> to display when `after-make-frame-functions' is called.
>  >
>  > I see.  So you are saying that the new frame object is passed to the hook
>  > functions, but that new frame has not yet been displayed.  If so, that is
>  > presumably the cause of the regression.
> 
> No.  I am saying that at the time `after-make-frame-functions' is
> called, the new frame does not yet show the buffer you want to display
> in it via plain `display-buffer'.
> 
>  > What's the point of passing the newly created frame object to hook
>  > functions intended to act on it, if that frame has not yet been displayed
>  > so they can do so?
>  >
>  > Perhaps you are allowing for hook functions that do not assume the frame
>  > is displayed.  Is that the point of this change?  Should I change the hook
>  > function here to, say, (lambda (fr) (raise-frame fr) (fit-frame fr))?  Or
>  > perhaps `make-frame-visible' instead of `raise-frame'?
>  >
>  > I just tried those, and they does not work either.  If I want to apply a
>  > function such as `fit-frame' to the new frame, and it is not yet
>  > displayed, what do I need to call in the hook function to display it first?
> 
> You want to apply `fit-frame' to the buffer you eventually want to
> display on the new frame.  Right?

I want to apply it to the frame (not to a buffer) _after_ it has been
displayed.  Previously, creating the frame (`make-frame') also displayed it,
and it did so before hook `after-create-frame-functions' was invoked.

>  > The doc string of `make-frame' suggests, BTW, that it should both (a)
>  > make a frame object and (b) display it, as I have always thought it did
>  > do and should do.  It says: "Return a newly created frame displaying the
>  > current buffer."
> 
> The _current_ buffer.

The question is which buffer should be current for that frame at that point.

I'm not sure the doc string should really have said "current buffer", as in
`current-buffer'.  Seems like what was intended - what makes sense in terms
of the hook, and what I have always thought has been the behavior until now -
is the buffer the frame was created to display (which is harder to say than
"current buffer").  IOW, the buffer that ends up displayed in the frame.

The key part of that doc string, for me, is that it returns a new frame that
is already displayed.  No one ever _sees_ a newly created frame first display
the `current-buffer' and then switch to the actual buffer to be displayed in
the frame.

What you see is only the frame displayed (immediately) with the proper buffer.
And that displayed frame, with the buffer it displays, is what has always
been passed to the `after-make-frame-functions'.

And previously that frame was always displayed before `after-make-frame-*'
was invoked.  So in a hook function you could do this, for example:

 (save-window-excursion
   (select-frame frame)
   (setq specbuf-p
         (and empty-buf-p
              (special-display-p (buffer-name (window-buffer))))))

And the `window-buffer' was the buffer that the frame displayed, which was
already the buffer that the frame was created to display.

> Because the "since forever" behavior is inconsistent.  Consider the
> following forms:
> 
> (defun mess (frame)
>    (message "selected: %s ... frame: %s ... buffer: %s"
> 	   (selected-frame) frame
> 	   (window-buffer (frame-root-window frame))))
> 
> (let ((pop-up-frames t))
>    (add-hook 'after-make-frame-functions 'mess)
>    (display-buffer "*Messages*")
>    (remove-hook 'after-make-frame-functions 'mess))
> 
> (let ((pop-up-frames t))
>    (add-hook 'after-make-frame-functions 'mess)
>    (pop-to-buffer "*Messages*")
>    (remove-hook 'after-make-frame-functions 'mess))
> 
> With emacs -Q evaluate the first and and then try the other two with
> some older version and the current trunk.  You should notice that the
> FRAME argument of `mess' is always different from the selected frame.
> But the buffer FRAME displays is different.  With the older versions the
> buffer is *scratch* for plain `display-buffer' and *Messages* for
> `pop-to-buffer'.  With current trunk it is *scratch* for both.
> 
> So I suppose that since forever your `fit-frame' function works on the
> "wrong" buffer when you call plain `display-buffer' and the buffer you
> want to display is not current at that time.

I see.  Yes, I see the behavior you describe, from emacs -Q.

However, I have never noticed a problem in this regard with my code -
dunno why.  For a moment I wondered if this is perhaps because on MS
Windows a new frame gets a certain kind of focus (not sure how to
characterize that behavior).  But I also use my code on GNU/Linux with
Emacs 21.3, and there too I have never see `fit-frame' fit a new frame
to the wrong buffer (the previously current one) etc.  Mystery.

It is true that I also make other calls to `fit-frame', including on
`temp-buffer-show-hook' for one-window frames.  Perhaps one of those
other calls has been compensating for the discrepency in Emacs -Q that
you point out.  Dunno.

Anyway, here is my question, given this discrepancy.  You want to make
the buffer be consistent.  OK, But you have so far chosen the
`current-buffer' as the buffer to use, perhaps from a reading of the
doc string.  Why not instead choose the buffer that will be displayed
in the frame, consistently?

IOW, for your test above, always have the buffer be *Messages*, since
that is the buffer displayed in the frame that is passed to
`after-make-frame-functions'?  What is the use case for passing the
frame, which will display (or is already displaying?) the *Messages*
buffer, have as its root-window buffer some other buffer (*scratch*) that
is not even displayed in that frame and perhaps never will be?

> What I propose is to use the following as substitute for the old
> `display-buffer-pop-up-frame':
> (defun display-buffer-pop-up-frame (buffer alist)...
> 
> With this the buffer printed by `mess' should be *Messages* for both,
> the plain `display-buffer' case and `pop-to-buffer'.

Sounds like you are suggesting the same thing I suggested immediately
above, and you are providing code that implements that.  And yes, a
quick trial indicates that that does seem to work, so far.  I will
load that code at startup and let you know if I notice any problems.
Thx.

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[martin rudalics]

 > Can you please tell me what to check, and how?  It's not clear to me what
 > you are asking/suggesting; sorry.

Whether with older versions your `fit-frame' function works with plain
`display-buffer', that is, bypassing `pop-to-buffer'.  Obviously,
`special-display-popup-frame' is disallowed as well because it does

	       (with-current-buffer buffer
		 (make-frame (append args special-display-frame-alist))))

and `temp-buffer-window-show' is disallowed because it does

     (with-current-buffer buffer
        ...
	      (setq window (display-buffer buffer action)))

 >> You want to apply `fit-frame' to the buffer you eventually want to
 >> display on the new frame.  Right?
 >
 > I want to apply it to the frame (not to a buffer) _after_ it has been
 > displayed.  Previously, creating the frame (`make-frame') also displayed it,
 > and it did so before hook `after-create-frame-functions' was invoked.

It displayed it if and only if it was current at the time `make-frame'
was called.  Also, at the time `after-make-frame-functions' is called
the buffer is not yet "displayed".  `make-frame' simply sets a slot in
the frame structure to reference that buffer.

 >> The _current_ buffer.
 >
 > The question is which buffer should be current for that frame at that point.

It's up to the caller to determine that, `make-frame' wouldn't know.

 > I'm not sure the doc string should really have said "current buffer", as in
 > `current-buffer'.  Seems like what was intended - what makes sense in terms
 > of the hook, and what I have always thought has been the behavior until now -
 > is the buffer the frame was created to display (which is harder to say than
 > "current buffer").  IOW, the buffer that ends up displayed in the frame.

This is not what `make-frame' does.

 > The key part of that doc string, for me, is that it returns a new frame that
 > is already displayed.

No.  Otherwise with `display-buffer' you would first see one buffer and
then another - or at least some kind of flicker.

 > No one ever _sees_ a newly created frame first display
 > the `current-buffer' and then switch to the actual buffer to be displayed in
 > the frame.

Then put a `sit-for' in `display-buffer-pop-up-frame' before it calls
`window--display-buffer'.

 > What you see is only the frame displayed (immediately) with the proper buffer.
 > And that displayed frame, with the buffer it displays, is what has always
 > been passed to the `after-make-frame-functions'.

Not necessarily - only when the buffer current at that time was also the
buffer finally displayed in that frame.

 > And previously that frame was always displayed before `after-make-frame-*'
 > was invoked.  So in a hook function you could do this, for example:
 >
 >  (save-window-excursion
 >    (select-frame frame)
 >    (setq specbuf-p
 >          (and empty-buf-p
 >               (special-display-p (buffer-name (window-buffer))))))
 >
 > And the `window-buffer' was the buffer that the frame displayed, which was
 > already the buffer that the frame was created to display.

I miss you here.  For `display-buffer' the hook is called after the
frame has been "created" but before it is ascertained that it displays
the "right" buffer.

 > Anyway, here is my question, given this discrepancy.  You want to make
 > the buffer be consistent.

I want `display-buffer' DTRT.  This means that it should call
`make-frame' with the buffer to be displayed current.

 > OK, But you have so far chosen the
 > `current-buffer' as the buffer to use, perhaps from a reading of the
 > doc string.  Why not instead choose the buffer that will be displayed
 > in the frame, consistently?

But that's what I do: I make the buffer "that will be displayed in the
frame" current so `make-frame' assigns it immediately to the new frame's
root window.  And I have to do this in the buffer display code because I
cannot pass a buffer to `make-frame' just as I cannot pass a buffer to
`split-window'.

 > IOW, for your test above, always have the buffer be *Messages*, since
 > that is the buffer displayed in the frame that is passed to
 > `after-make-frame-functions'?

It's not passed.  It's the current buffer at the time
`after-make-frame-functions' is called.

 > What is the use case for passing the
 > frame, which will display (or is already displaying?) the *Messages*
 > buffer, have as its root-window buffer some other buffer (*scratch*) that
 > is not even displayed in that frame and perhaps never will be?

`make-frame' has been designed that way - display the current buffer in
the new frame.  Just as `split-window' displays the buffer shown in the
window to be split in the new window too.

 > Sounds like you are suggesting the same thing I suggested immediately
 > above, and you are providing code that implements that.  And yes, a
 > quick trial indicates that that does seem to work, so far.  I will
 > load that code at startup and let you know if I notice any problems.

Thanks, martin

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

>  > Can you please tell me what to check, and how?  It's not clear to me what
>  > you are asking/suggesting; sorry.
> 
> Whether with older versions your `fit-frame' function works with plain
> `display-buffer', that is, bypassing `pop-to-buffer'.

Yes, it does.  But I cannot tell you now what part of my code takes care
of that.  If I try emacs -Q, load fit-frame.el, (setq pop-up-frames t),
and (add-hook 'after-make-frame-functions 'fit-frame) then no, it does
not work, as you expected.

But in my full setup, it does work (i.e., (display-buffer "foo") shows
buffer foo in a new frame, which is fit to the buffer contents).  Dunno
why it works, but it does.

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[martin rudalics]

> What I propose is to use the following as substitute for the old
> `display-buffer-pop-up-frame':

Installed as revision #113977.  Please check and close the bug if
it works.

Thanks, martin

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

> > What I propose is to use the following as substitute for the old
> > `display-buffer-pop-up-frame':
> 
> Installed as revision #113977.  Please check and close the bug if
> it works.

Will do when I get the next Emacs binary available.
Thanks.

bug#15133: 24.3.50; REGRESSION: `after-make-frame-functions' now run with wrong frame selected

[Drew Adams]

> > > What I propose is to use the following as substitute for the old
> > > `display-buffer-pop-up-frame':
> >
> > Installed as revision #113977.  Please check and close the bug if
> > it works.
> 
> Will do when I get the next Emacs binary available.
> Thanks.

Seems to fix the problem.  Thanks; closing.