Re: [PATCH] Fix display-buffer-use-some-window to honor reusable-frames

[martin rudalics <rudalics@gmx.at>]

 > Martin, any comments?

Sorry but I had no idea what 'display-buffer-use-least-recent-window' is
and what it is supposed to accomplish.  This part of its doc-string "but
will cycle through windows when displaying buffers repeatedly" seems to
describe what 'display-buffer-use-some-window' does.  The "if there's
only a single window, it will split the window" does not fit at all - it
would rely on some extraneous mechanism like a base action to do that
(and IIUC Tom's patch tries to address that issue).  OTOH, the fact that
'display-buffer-use-least-recent-window' bumps a window's use time is
nowhere mentioned AFAICT.

IIUC the idea of bumping the use time is to make sure that a subsequent
'display-buffer' does not reuse the window that's used here because we
pretend that that window was selected recently and so 'get-lru-window'
will avoid it.  The doc-string should have said that.

IMO the present approach has three defects which break things and
ultimately make it useless for 'display-buffer'.


(1) 'display-buffer-use-least-recent-window' claims to be a buffer
display action function but IIUC it does not return the window used.
For example, with emacs -Q do C-x 2, C-x o and evaluate

(display-buffer
  (get-buffer-create "*foo*")
  '((display-buffer-use-least-recent-window display-buffer-at-bottom)))

You get two windows showing *foo*.  This one is easy to fix.


(2) The current version of 'window-bump-use-time' changes the semantics
of "least recently used window" without even mentioning that anywhere.
For example, this code in sql.el

     (let ((one-win (eq (selected-window)
                        (get-lru-window))))

will conclude that there is only one window even if another window
recently created by 'display-buffer-use-least-recent-window' exists.  I
have no idea how 'get-mru-window' could be affected.

This is a grave bug.  'window-bump-use-time' could try

   struct window *sw = XWINDOW (selected_window);

   if (w != sw)
     {
       w->use_time = ++window_select_count;
       sw->use_time = ++window_select_count;
     }

to make sure that the selected window invariably stays the most recently
used one but this would require some testing.


(3) The idea of having one action function bump use times works iff
'display-buffer-use-least-recent-window' is the _only_ action function
called by both user and application.  It breaks as soon as another
action function kicks in - either because the other action function does
not call 'get-lru-window' or because that other action function does not
bump the use time of the window used.  This is impossible to fix with
the current approach.

One thing we could do is to add a new action alist entry say
'bump-use-time'.  People could use this to indicate that they want to
display several buffers in a row.  'window--display-buffer' would then
bump the use time of the window it uses when ALIST contains that entry.
I have no idea how this would work in practice.


A few words about Tom's patch: 'display-buffer-use-some-window' must not
pop up a new window.  It's doc-string clearly says to "Display BUFFER in
an existing window."  Please don't ever try to change that.  Also, it
should not care about 'reusable-frames'.  The latter is about reusing a
window that already shows BUFFER and other action functions might be
affected badly if 'display-buffer-use-some-window' tried to handle this
too.  Finally, 'display-buffer-no-window' has no place in another action
function.  It is strictly reserved to callers and users.

martin