From 9f508cef85df64ccbafada477bbb17a8439bc839 Mon Sep 17 00:00:00 2001 From: martin rudalics Date: Sun, 12 Feb 2023 10:33:11 +0100 Subject: [PATCH] Fix 'display-buffer-use-least-recent-window' * src/window.c (Fwindow_use_time): Doc fix. (Fwindow_bump_use_time): Bump use time of the seleceted window as well. Doc fix. * lisp/window.el (display-buffer-avoid-small-windows): Remove. All users changed. (window--display-buffer): Bump window use time when requested. (display-buffer--lru-window): New function. (display-buffer-use-some-window): Use it. (display-buffer-use-least-recent-window): Rewrite and enhance doc string. * doc/lispref/windows.texi (Selecting Windows) (Buffer Display Action Functions, Buffer Display Action Alists) (The Zen of Buffer Display): Improve and update documentation of window selection and display facilities. --- doc/lispref/windows.texi | 233 ++++++++++++++++++++++++++++----------- lisp/window.el | 169 +++++++++++++++++++++++----- src/window.c | 33 ++++-- 3 files changed, 339 insertions(+), 96 deletions(-) diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi index 441e7f1b16d..1ec645721ca 100644 --- a/doc/lispref/windows.texi +++ b/doc/lispref/windows.texi @@ -629,6 +629,12 @@ example, by calling @code{select-window} with argument @var{norecord} @code{nil}. Hence, this macro is the preferred way to temporarily work with @var{window} as the selected window without needlessly running @code{buffer-list-update-hook}. + +Note that this macro temporarily puts the window management code in a +unstable state. In particular, the most recently window (see below) +will not necessarily match the selected one. Hence, functions like +@code{get-lru-window} and @code{get-mru-window} may return unexpected +results when called from within the scope of this macro. @end defmac @defmac with-selected-frame frame forms@dots{} @@ -650,15 +656,16 @@ The @dfn{use time} of a window is not really a time value, but an integer that does increase monotonically with each call of @code{select-window} with a @code{nil} @var{norecord} argument. The window with the lowest use time is usually called the least recently -used window while the window with the highest use time is called the -most recently used one (@pxref{Cyclic Window Ordering}). +used window. The window with the highest use time is called the most +recently used one (@pxref{Cyclic Window Ordering}) and is usually the +selected window unless @code{with-selected-window} has been used. @end defun @defun window-bump-use-time &optional window -This function marks @var{window} as being the most recently used -one. This can be useful when writing certain @code{pop-to-buffer} -scenarios (@pxref{Switching Buffers}). @var{window} must be a live -window and defaults to the selected one. +This function marks @var{window} as being the second most recently used +one. It does nothing if @var{window} is the selected window or the +selected window does not have the highest use time among all windows +which may happen within the scope of @code{with-selected-window}. @end defun @anchor{Window Group}Sometimes several windows collectively and @@ -2755,14 +2762,40 @@ before. @defun display-buffer-use-some-window buffer alist This function tries to display @var{buffer} by choosing an existing -window and displaying the buffer in that window. It can fail if all -windows are dedicated to other buffers (@pxref{Dedicated Windows}). +window and displaying the buffer in that window. It first tries to find +a window that has not been used recently (@pxref{Cyclic Window +Ordering}) on any frame specified by a @code{lru-frames} @var{alist} +entry, falling back to the selected frame if no such entry exists. It +also prefers windows that satisfy the constraints specified by +@code{window-min-width} and @code{window-min-height} @var{alist} +entries; preferring full-width windows if no @code{window-min-width} +entry is found. Finally, it will not return a window whose use time is +higher than that specified by any @code{lru-time} entry provided by +@var{alist}. + +If no less recently used window is found, this function will try to use +some other window, preferably a large window on some visible frame. It +can fail if all windows are dedicated to other buffers (@pxref{Dedicated +Windows}). @end defun @defun display-buffer-use-least-recent-window buffer alist -This function is like @code{display-buffer-use-some-window}, but will -not reuse the current window, and will use the least recently -switched-to window. +This function is similar to @code{display-buffer-use-some-window}, but +will try harder to not use the a recently used window. In particular, +it does not use the selected window. In addition, it will first try to +reuse a window that shows @var{buffer} already, base the decision +whether it should use a window showing another buffer on that window's +use time alone and pop up a new window if no usable window is found. + +Finally, this function will bump the use time (@pxref{Selecting +Windows}) of any window it returns in order to avoid that further +invocations will use that window for showing another buffer. An +application that wants to display several buffers in a row can help this +function by providing a @code{lru-time} @var{alist} entry it has +initially set to the value of the selected window's use time. Each +invocation of this function will then bump the use time of the window +returned to a value higher than that and a subsequent invocation will +inhibit this function to use a window it returned earlier. @end defun @defun display-buffer-in-direction buffer alist @@ -3032,12 +3065,39 @@ The value specifies an alist of window parameters to give the chosen window. All action functions that choose a window should process this entry. +@vindex window-min-width@r{, a buffer display action alist entry} +@item window-min-width +The value specifies a minimum width of the window used, in canonical +frame columns. The special value @code{full-width} means the window +chosen should be one has no other windows on the left or right it in its +frame. + +This entry is currently honored by @code{display-buffer-use-some-window} +and @code{display-buffer-use-least-recent-window} who try hard to avoid +returning a less recently used window that does not satisfy it. + +Note that providing such an entry alone does not necessarily make the +window as wide as specified by its value. To actually resize an +existing window or make a new window as wide as specified by that value, +a @code{window-width} entry specifying that value should be provided as +well. Such a @code{window-width} entry can, however, specify a +completely different value or ask the window width to be fit to that of +its buffer in which case the @code{window-min-width} entry provides the +guaranteed minimum width of the window used. + @vindex window-min-height@r{, a buffer display action alist entry} @item window-min-height -The value specifies a minimum height of the window used, in lines. If -a window is not or cannot be made as high as specified by this entry, -the window is not considered for use. The only client of this entry -is presently @code{display-buffer-below-selected}. +The value specifies a minimum height of the window used, in canonical +frame lines. The special value @code{full-height} means the window +chosen should be a full-height window, one has no other windows above or +below it in its frame. + +This entry is currently honored by @code{display-buffer-below-selected} +which does not use a window that is not as high as specified by this +entry. It's also honored by @code{display-buffer-use-some-window} and +@code{display-buffer-use-least-recent-window} which try hard to avoid +returning a less recently used window if it does not satisfy this +constraint. Note that providing such an entry alone does not necessarily make the window as tall as specified by its value. To actually resize an @@ -3166,6 +3226,40 @@ preserve both, its width and its height. This entry should be processed only under certain conditions which are specified right after this list. +@vindex lru-frames@r{, a buffer display action alist entry} +@item lru-frames +The value specifies the set of frames to search for window that can be +used to display the buffer. It is honored by +@code{display-buffer-use-some-window} and +@code{display-buffer-use-least-recent-window} when trying to find a less +recently used window showing some other buffer. Its values are the same +as for the @code{reusable-frames} entry described above. + +@vindex lru-time@r{, a buffer display action alist entry} +@item lru-time +The value is supposed to specify a use time (@pxref{Selecting Windows}). +This entry is honored by @code{display-buffer-use-some-window} and +@code{display-buffer-use-least-recent-window} when trying to find a less +recently used window showing some other buffer. If a window's use time +is higher than the value specified by this option, these action +functions will not consider such a window for displaying the buffer. + +@vindex bump-use-time@r{, a buffer display action alist entry} +@item bump-use-time +If non-@code{nil}, such an entry will cause @code{display-buffer} to +bump the use time (@pxref{Selecting Windows}) of the window it uses. +This should avoid that this window is later used by action functions +like @code{display-buffer-use-some-window} and +@code{display-buffer-use-least-recent-window} for showing another +buffer. + +There is a fine difference between using this entry and using the action +function @code{display-buffer-use-least-recent-window}. Calling the +latter means to only bump the use times of windows that function uses +for displaying the buffer. The entry described here will cause +@code{display-buffer} to bump the use time of @emph{any} window used for +displaying a buffer. + @vindex pop-up-frame-parameters@r{, a buffer display action alist entry} @item pop-up-frame-parameters The value specifies an alist of frame parameters to give a new frame, @@ -3321,13 +3415,6 @@ window has at least that many columns. If the value is @code{nil}, that means not to split this way. @end defopt -@defopt display-buffer-avoid-small-windows -If non-@code{nil}, this should be a number. Windows that have fewer -lines than that will be avoided when choosing an existing window. The -value is interpreted in units of the frame's canonical line height, -like @code{window-total-height} does (@pxref{Window Sizes}). -@end defopt - @defopt even-window-sizes This variable, if non-@code{nil}, causes @code{display-buffer} to even window sizes whenever it reuses an existing window, and that window is @@ -3992,53 +4079,75 @@ related to the new window. For non-input related actions @code{display-buffer-below-selected} might be preferable because the selected window usually already has the user's attention. -@item Handle subsequent invocations of @code{display-buffer} -@code{display-buffer} is not overly well suited for displaying several -buffers in sequence and making sure that all these buffers are shown -orderly in the resulting window configuration. Again, the standard -action functions @code{display-buffer-pop-up-window} and -@code{display-buffer-use-some-window} are not very suited for this -purpose due to their somewhat chaotic nature in more complex -configurations. +@item Take care about which window is selected +Many applications call @code{display-buffer} from within window +excursions produced by @code{with-selected-window} or +@code{select-window} calls with a non-@code{nil} @var{norecord} +argument. This is almost always a bad idea because the window selected +within such an excursion is usually not the window selected in the +configuration presented to the user. + +If, for example, a user had added an @code{inhibit-same-window} alist +entry, that entry would have avoided the window selected within the +scope of the excursion and not the window selected in the resulting +configuration. Even if no such entry has been added, the resulting +behavior might be strange. While in a frame containing one live +window, evaluating the following form - To produce a window configuration displaying multiple buffers (or -different views of one and the same buffer) in one and the same -display cycle, Lisp programmers will unavoidably have to write -their own action functions. A few tricks listed below might help in -this regard. +@example +@group +(progn + (split-window) + (display-buffer "*Messages*")) +@end group +@end example -@itemize @bullet -@item -Making windows atomic (@pxref{Atomic Windows}) avoids breaking an -existing window composition when popping up a new window. -The new window will pop up outside the composition instead. +will display a window showing the @file{*Messages*} buffer at the bottom +and leave the other window selected. Evaluating the next form -@item -Temporarily dedicating windows to their buffers (@pxref{Dedicated -Windows}) avoids using a window for displaying a different -buffer. A non-dedicated window will be used instead. +@example +@group +(with-selected-window (split-window) + (display-buffer "*Messages*")) +@end group +@end example -@item -Calling @code{window-preserve-size} (@pxref{Preserving Window Sizes}) -will try to keep the size of the argument window unchanged when -popping up a new window. You have to make sure that another window in -the same combination can be shrunk instead, though. +will display @file{*Messages*} in a window on the top and select it +which is usually not what @code{display-buffer} is supposed to do. -@item -Side windows (@pxref{Side Windows}) can be used for displaying -specific buffers always in a window at the same position of a frame. -This permits grouping buffers that do not compete for being shown at -the same time on a frame and showing any such buffer in the same window -without disrupting the display of other buffers. +On the other hand, while evaluating the following form -@item -Child frames (@pxref{Child Frames}) can be used to display a buffer -within the screen estate of the selected frame without disrupting that -frame's window configuration and without the overhead associated with -full-fledged frames as inflicted by @code{display-buffer-pop-up-frame}. -@end itemize -@end table +@example +@group +(progn + (split-window) + (pop-to-buffer "*Messages*")) +@end group +@end example + +will correctly select the @file{*Messages*} buffer, the next form + +@example +@group +(progn + (split-window) + (with-selected-window (selected-window) + (pop-to-buffer "*Messages*"))) +@end group +@end example + +will not. +Also, invocations of action functions like +@code{display-buffer-use-some-window} and +@code{display-buffer-use-least-recent-window} that expect the selected +window to have the highest use time among all windows, may fail to +produce a window according to their specifications. + +Hence, an application that relies on using a window excursion should try +to postpone the @code{display-buffer} call until after the excursion has +terminated. +@end table @node Window History @section Window History diff --git a/lisp/window.el b/lisp/window.el index 2d9f746d8fb..083fa9bfd2f 100644 --- a/lisp/window.el +++ b/lisp/window.el @@ -2484,14 +2484,6 @@ and no others." (defalias 'some-window 'get-window-with-predicate) -(defcustom display-buffer-avoid-small-windows nil - "If non-nil, windows that have fewer lines than this are avoided. -This is used by `get-lru-window'. The value is interpreted in units -of the frame's canonical line height, like `window-total-height' does." - :type '(choice (const nil) number) - :version "29.1" - :group 'windows) - (defun get-lru-window (&optional all-frames dedicated not-selected no-other) "Return the least recently used window on frames specified by ALL-FRAMES. Return a full-width window if possible. A minibuffer window is @@ -2517,11 +2509,7 @@ have special meanings: - A frame means consider all windows on that frame only. Any other value of ALL-FRAMES means consider all windows on the -selected frame and no others. - -`display-buffer-avoid-small-windows', if non-nil, is also taken into -consideration. Windows whose height is smaller that the value of that -variable will be avoided if larger windows are available." +selected frame and no others." (let ((windows (window-list-1 nil 'nomini all-frames)) best-window best-time second-best-window second-best-time time) (dolist (window windows) @@ -2531,9 +2519,6 @@ variable will be avoided if larger windows are available." (not (window-parameter window 'no-other-window)))) (setq time (window-use-time window)) (if (or (eq window (selected-window)) - (and display-buffer-avoid-small-windows - (< (window-height window) - display-buffer-avoid-small-windows)) (not (window-full-width-p window))) (when (or (not second-best-time) (< time second-best-time)) (setq second-best-time time) @@ -7274,6 +7259,11 @@ entry. Otherwise, if WINDOW is new and the value of dedicated flag to that value. In any other case, reset WINDOW's dedicated flag to nil. +If ALIST contains a non-nil `bump-use-time' entry, bump use time +of WINDOW so further calls of `display-buffer-use-some-window' +and `display-buffer-use-least-recent-window' will try to avoid +it. + Return WINDOW if BUFFER and WINDOW are live." (when (and (buffer-live-p buffer) (window-live-p window)) (display-buffer-record-window type window buffer) @@ -7281,6 +7271,10 @@ Return WINDOW if BUFFER and WINDOW are live." ;; Unless WINDOW already shows BUFFER reset its dedicated flag. (set-window-dedicated-p window nil) (set-window-buffer window buffer)) + (when (cdr (assq 'bump-use-time alist)) + ;; Bump WINDOW's use time so 'display-buffer--lru-window' will try + ;; to avoid it. + (window-bump-use-time window)) (let ((alist-dedicated (assq 'dedicated alist))) ;; Maybe dedicate WINDOW to BUFFER if asked for. (cond @@ -8502,15 +8496,64 @@ indirectly called by the latter." (when (setq window (or best-window second-best-window)) (window--display-buffer buffer window 'reuse alist)))) -(defun display-buffer-use-least-recent-window (buffer alist) - "Display BUFFER in an existing window, but that hasn't been used lately. -This `display-buffer' action function is like -`display-buffer-use-some-window', but will cycle through windows -when displaying buffers repeatedly, and if there's only a single -window, it will split the window." - (when-let ((window (display-buffer-use-some-window - buffer (cons (cons 'inhibit-same-window t) alist)))) - (window-bump-use-time window))) +(defun display-buffer--lru-window (alist) + "Return the least recently used window according to ALIST. +Do not return a minibuffer window or a window dedicated to its +buffer. ALIST is a buffer display action alist as compiled by +`display-buffer'. The following ALIST entries are honored: + +- `lru-frames' specifies the frames to investigate and has the + same meaning as the ALL-FRAMES argument of `get-lru-window'. + +- `lru-time' specifies a use time. Do not return a window whose + use time is higher than this. + +- `window-min-width' specifies a preferred minimum width in + canonical frame columns. If it is the constant `full-width', + prefer a full-width window. + +- `window-min-height' specifies a preferred minimum height in + canonical frame lines. If it is the constant `full-height', + prefer a full-height window. + +If ALIST contains a non-nil `inhibit-same--window' entry, do not +return the selected window." + (let ((windows + (window-list-1 nil 'nomini (cdr (assq 'lru-frames alist)))) + (lru-time (cdr (assq 'lru-time alist))) + (min-width (cdr (assq 'window-min-width alist))) + (min-height (cdr (assq 'window-min-height alist))) + (not-this-window (cdr (assq 'inhibit-same-window alist))) + best-window best-time second-best-window second-best-time time) + (dolist (window windows) + (when (and (not (window-dedicated-p window)) + (or (not not-this-window) + (not (eq window (selected-window))))) + (setq time (window-use-time window)) + (unless (and (numberp lru-time) (> time lru-time)) + (if (or (eq window (selected-window)) + (and min-width + (or (and (numberp min-width) + (< (window-width window) min-width)) + (and (eq min-width 'full-width) + (not (window-full-width-p window))))) + (and min-height + (or (and (numberp min-height) + (< (window-height window) min-height)) + (and (eq min-height 'full-height) + (not (window-full-height-p window)))))) + ;; This window is either selected or does not meet the size + ;; restrictions - so it's only a second best choice. Try to + ;; find a more recently used one that fits. + (when (or (not second-best-time) (< time second-best-time)) + (setq second-best-time time) + (setq second-best-window window)) + ;; This window is not selected and does meet the size + ;; restrictions. It's the best choice so far. + (when (or (not best-time) (< time best-time)) + (setq best-time time) + (setq best-window window)))))) + (or best-window second-best-window))) (defun display-buffer-use-some-window (buffer alist) "Display BUFFER in an existing window. @@ -8534,7 +8577,11 @@ indirectly called by the latter." (window--frame-usable-p (last-nonminibuffer-frame)))) (window ;; Reuse an existing window. - (or (get-lru-window frame nil not-this-window) + (or (display-buffer--lru-window + ;; If ALIST specifies 'lru-frames' or 'window-min-width' + ;; let them prevail. + (append alist `((lru-frames . ,frame) + (window-min-width . full-width)))) (let ((window (get-buffer-window buffer 'visible))) (unless (and not-this-window (eq window (selected-window))) @@ -8564,6 +8611,76 @@ indirectly called by the latter." (unless (cdr (assq 'inhibit-switch-frame alist)) (window--maybe-raise-frame (window-frame window))))))) +(defun display-buffer-use-least-recent-window (buffer alist) + "Display BUFFER trying to avoid windows used recently. +This is similar to `display-buffer-use-some-window' but tries +hard to avoid using a window recently used by `display-buffer'. + +Distinctive features are: + +- Do not use the selected window. + +- Try first to reuse a window that shows BUFFER already on a + frame specified by a `reusable-frames' ALIST entry, using the + selected frame if no such entry has been specified. + +- Next try to show BUFFER in the least recently used window. The + frames to search for such a window can be specified via a + `lru-frames' ALIST entry; if no such entry exists, search the + selected frame only. In addition, try to satisfy constraints + specified by the following ALIST entries, if present: + + `lru-time' specifies a use time. Do not return a window whose + use time is higher than this. When calling this action + function repeatedly (presumably to display several buffers in + a row), an application should first save the use time of the + selected window and pass that same value via such an entry in + each call of `display-buffer'. This reduces the probability + that `display-buffer' uses the same window as a previous + call. + + `window-min-width' specifies a preferred minimum width in + canonical frame columns. If it is the constant `full-width', + prefer a full-width window. + + `window-min-height' specifies a preferred minimum height in + canonical frame lines. If it is the constant `full-height', + prefer a full-height window. + +- If the preceding steps fail, try to pop up a new window on the + selected frame. + +If a window is found, bump the use time of that window to the +highest use time after the selected window. This makes it less +probable that a future invocation of this function uses that +window for another buffer." + (let* ((alist (cons (cons 'inhibit-same-window t) alist)) + (window + (or (display-buffer-reuse-window buffer alist) + (let ((window (display-buffer--lru-window alist))) + (when (window-live-p window) + (let* ((quit-restore (window-parameter window 'quit-restore)) + (quad (nth 1 quit-restore))) + ;; If the window was used by `display-buffer' before, try to + ;; resize it to its old height but don't signal an error. + (when (and (listp quad) + (integerp (nth 3 quad)) + (> (nth 3 quad) (window-total-height window))) + (condition-case nil + (window-resize + window (- (nth 3 quad) (window-total-height window))) + (error nil))) + (prog1 + (window--display-buffer buffer window 'reuse alist) + (window--even-window-sizes window) + (unless (cdr (assq 'inhibit-switch-frame alist)) + (window--maybe-raise-frame (window-frame window))))))) + (display-buffer-pop-up-window buffer alist)))) + ;; Don't bump use time twice. + (when (and window (not (cdr (assq 'bump-use-time alist)))) + (window-bump-use-time window)) + window)) + (defun display-buffer-no-window (_buffer alist) "Display BUFFER in no window. ALIST is an association list of action symbols and values. See diff --git a/src/window.c b/src/window.c index 6201a6f4a36..a94e1d611c7 100644 --- a/src/window.c +++ b/src/window.c @@ -762,10 +762,15 @@ future use. */) DEFUN ("window-use-time", Fwindow_use_time, Swindow_use_time, 0, 1, 0, doc: /* Return the use time of window WINDOW. -WINDOW must be a live window and defaults to the selected one. -The window with the highest use time is the most recently selected -one. The window with the lowest use time is the least recently -selected one. */) +WINDOW must specify a live window and defaults to the selected one. + +The window with the highest use time is usually the one most recently +selected by calling `select-window' with NORECORD nil. The window with +the lowest use time is usually the least recently selected one chosen in +such a way. + +Note that the use time of a window can be also changed by calling +`window-bump-use-time' for that window. */) (Lisp_Object window) { return make_fixnum (decode_live_window (window)->use_time); @@ -773,15 +778,27 @@ selected one. */) DEFUN ("window-bump-use-time", Fwindow_bump_use_time, Swindow_bump_use_time, 0, 1, 0, - doc: /* Mark WINDOW as having been most recently used. -WINDOW must be a live window and defaults to the selected one. */) + doc: /* Mark WINDOW as second most recently used. +WINDOW must specify a live window. + +If WINDOW is not selected and the selected window has the highest use +time of all windows, set the use time of WINDOW to that of the selected +window, increase the use time of the selected window by one and return +the new use time of WINDOW. Otherwise, do nothing and return nil. */) (Lisp_Object window) { struct window *w = decode_live_window (window); + struct window *sw = XWINDOW (selected_window); - w->use_time = ++window_select_count; + if (w != sw && sw->use_time == window_select_count) + { + w->use_time = window_select_count; + sw->use_time = ++window_select_count; - return Qnil; + return make_fixnum (w->use_time); + } + else + return Qnil; } DEFUN ("window-pixel-width", Fwindow_pixel_width, Swindow_pixel_width, 0, 1, 0, -- 2.39.5