bug#45688: 28.0.50; New action for display-buffer?

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

Reading this:

https://www.jwz.org/blog/2021/01/very-much-do-not-have-my-emacs-setup-just-how-i-like-it/

I was curious how XEmacs did this command, so I installed XEmacs.  If
there's two (or one) window, the command works the same in Emacs and
XEmacs, but it's interesting what XEmacs does when there's more than two
windows: It cycles the windows.

Say you start with this:

-----
buf 1
-----
buf 2
-----
buf 3
-----

and you're in buf 1.  `M-x display-buffer RET RET' then gives you:

-----
buf 1
-----
buf 4
-----
buf 3
-----

Repeat, and:

-----
buf 1
-----
buf 4
-----
buf 2
-----

Repeat, and:

-----
buf 1
-----
buf 3
-----
buf 2
-----

Repeat, and:

-----
buf 1
-----
buf 3
-----
buf 4
-----

Repeat, and:

-----
buf 1
-----
buf 2
-----
buf 4
-----

That is, it cycles the windows.  If there are more windows, it'll go
through all the other windows in order, choosing the
least-recently-switched-to window.

This does indeed seem quite nice, and apparently none of the built-in
actions has this effect?

Action functions and the action they try to perform are:
 ‘display-buffer-same-window’ -- Use the selected window.
 ‘display-buffer-reuse-window’ -- Use a window already showing
    the buffer.
 ‘display-buffer-in-previous-window’ -- Use a window that did
    show the buffer before.
 ‘display-buffer-use-some-window’ -- Use some existing window.
 ‘display-buffer-pop-up-window’ -- Pop up a new window.
 ‘display-buffer-below-selected’ -- Use or pop up a window below
    the selected one.
 ‘display-buffer-at-bottom’ -- Use or pop up a window at the
    bottom of the selected frame.
 ‘display-buffer-pop-up-frame’ -- Show the buffer on a new frame.
 ‘display-buffer-in-child-frame’ -- Show the buffer in a
    child frame.
 ‘display-buffer-no-window’ -- Do not display the buffer and
    have ‘display-buffer’ return nil immediately.

So I wonder whether it would make sense to add this. 



In GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.23, cairo version 1.16.0)
 of 2020-12-31 built on xo
Repository revision: 72b8430fea79cb1ebb8c3652babf42a12569ab8a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >   ‘display-buffer-use-some-window’ -- Use some existing window.

This would use 'get-lru-window' but if you display always the same small
set of buffers, 'display-buffer-in-previous-window' prevails.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>>   ‘display-buffer-use-some-window’ -- Use some existing window.
>
> This would use 'get-lru-window' but if you display always the same small
> set of buffers, 'display-buffer-in-previous-window' prevails.

I tried

(setq display-buffer-overriding-action '(display-buffer-use-some-window))

but I don't see any difference -- display-buffer always uses the same
window when displaying buffers.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > I tried
 >
 > (setq display-buffer-overriding-action '(display-buffer-use-some-window))
 >
 > but I don't see any difference -- display-buffer always uses the same
 > window when displaying buffers.

You really have to "use" such a window which means selecting it.  Just
displaying a buffer in a window does not count as using it.  The use
time of a window that was never selected stays at zero, so such a window
is always a good candidate for 'display-buffer-use-some-window'.  That's
how use times and `get-lru-window' work.  Feel free to install something
more appropriate here.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> You really have to "use" such a window which means selecting it.  Just
> displaying a buffer in a window does not count as using it.

Ah, thanks.  That makes sense; I'll try playing around with a new action
that calls display-buffer-use-some-window, but marks the window as
having been used.

I've never actually encountered this bit of Emacs before -- I'm just
using the defaults for all the window stuff.  :-)

I think the doc string for `display-buffer' is pretty intimidating now,
and seems more geared towards programmers than usage now.  Would it make
sense to rearrange it a bit?  That is, start off with something like:

-----

Display BUFFER-OR-NAME in some window, without selecting it.  To change
what window is used, set `display-buffer-overriding-action' to a list
containing one or more of the following functions:

Action functions and the action they try to perform are:
 ‘display-buffer-same-window’ -- Use the selected window.
 ‘display-buffer-reuse-window’ -- Use a window already showing
    the buffer.
 ‘display-buffer-in-previous-window’ -- Use a window that did
    show the buffer before.

etc etc, and then give the details about ACTION and the rest?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > I think the doc string for `display-buffer' is pretty intimidating now,
 > and seems more geared towards programmers than usage now.  Would it make
 > sense to rearrange it a bit?  That is, start off with something like:
 >
 > -----
 >
 > Display BUFFER-OR-NAME in some window, without selecting it.  To change
 > what window is used, set `display-buffer-overriding-action' to a list
 > containing one or more of the following functions:
 >
 > Action functions and the action they try to perform are:
 >   ‘display-buffer-same-window’ -- Use the selected window.
 >   ‘display-buffer-reuse-window’ -- Use a window already showing
 >      the buffer.
 >   ‘display-buffer-in-previous-window’ -- Use a window that did
 >      show the buffer before.
 >
 > etc etc, and then give the details about ACTION and the rest?

By no means.  `display-buffer-overriding-action' is _not_ intended for
the user.  It's an emergency exit for applications.  Better not even
mention it in the user manual and the doc-string.

In every other regard, feel free to change it in a way that sounds less
imitating.  For example, listing the action functions and alist entries
in the doc-string is merely distracting.  We started out with small sets
and every time a new function or entry was added, we also added it to
those lists.  And while you're there you could try to make the Emacs
manual entry more amenable for its audience.  IIUC that's what the blog
you mentioned criticized even more than the doc-string.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> By no means.  `display-buffer-overriding-action' is _not_ intended for
> the user.  It's an emergency exit for applications.  Better not even
> mention it in the user manual and the doc-string.

See, I didn't even get that bit from the doc string.  :-)

> In every other regard, feel free to change it in a way that sounds less
> imitating.  For example, listing the action functions and alist entries
> in the doc-string is merely distracting.  We started out with small sets
> and every time a new function or entry was added, we also added it to
> those lists.

It's an overwhelming doc string, but I think listing the action
functions is one of the best bits about it.  :-)  I've now shuffled it
to the front, and given a bit more introductory text to make it sound
less intimidating.

> And while you're there you could try to make the Emacs
> manual entry more amenable for its audience.  IIUC that's what the blog
> you mentioned criticized even more than the doc-string.

Yeah, the manual is rather abrupt here, too, but the entire thing is
really complicated, and caters for many different use cases, so I don't
really even know where to begin.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >> By no means.  `display-buffer-overriding-action' is _not_ intended for
 >> the user.  It's an emergency exit for applications.  Better not even
 >> mention it in the user manual and the doc-string.
 >
 > See, I didn't even get that bit from the doc string.  :-)

Given that 'display-buffer-overriding-action' figures first in

   ‘display-buffer’ builds a list of action functions and an action
   alist by combining any action functions and alists specified by
   ‘display-buffer-overriding-action’, ‘display-buffer-alist’, the
   ACTION argument, ‘display-buffer-base-action’, and
   ‘display-buffer-fallback-action’ (in order).  Then it calls each
   function in the combined function list in turn, passing the
   buffer as the first argument and the combined action alist as the
   second argument, until one of the functions returns non-nil.

your interpretation is quite logical.  The problem here is that we talk
in operative terms like "builds" "combining" "calls" "passing" which
hardly contributes anything to the understanding of an average user.

In a first approximation it should suffice to talk here about
`display-buffer-alist' and the ACTION argument only and ...

 >> In every other regard, feel free to change it in a way that sounds less
 >> imitating.  For example, listing the action functions and alist entries
 >> in the doc-string is merely distracting.  We started out with small sets
 >> and every time a new function or entry was added, we also added it to
 >> those lists.
 >
 > It's an overwhelming doc string, but I think listing the action
 > functions is one of the best bits about it.  :-)

... here I'd just say what they can do - reuse the selected window, a
window showing the buffer already, some other window, pop up a new
window on this or another frame.

 > I've now shuffled it
 > to the front, and given a bit more introductory text to make it sound
 > less intimidating.
 >
 >> And while you're there you could try to make the Emacs
 >> manual entry more amenable for its audience.  IIUC that's what the blog
 >> you mentioned criticized even more than the doc-string.
 >
 > Yeah, the manual is rather abrupt here, too, but the entire thing is
 > really complicated, and caters for many different use cases, so I don't
 > really even know where to begin.

By dropping what you understand least.  If this makes it an empty
entrance, start from scratch with what you wanted to know first.  If
anything is incorrect, I'll fix it.  But making this readable for a user
is next to impossible, at least for me (I'm repeating myself whenever I
talk about this subject).  Have a look at the Elisp manual where I try,
maybe it helps.

martin

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > I've now shuffled it
 > to the front, and given a bit more introductory text to make it sound
 > less intimidating.

Sorry, I didn't notice that you've pushed that already.  So

+To change which window is used, set `display-buffer-base-action'
+to a list containing one of these \"action\" functions:

is another wrong interpretation - a user should not set (or at least not
start with setting) `display-buffer-base-action'.  The user's domain is
`display-buffer-alist' the application's domain is the ACTION argument.
Please try to stick to these two, maybe not even mentioning the rest at
all.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Sorry, I didn't notice that you've pushed that already.  So
>
> +To change which window is used, set `display-buffer-base-action'
> +to a list containing one of these \"action\" functions:
>
> is another wrong interpretation

:-)

> - a user should not set (or at least not start with setting)
> `display-buffer-base-action'.  The user's domain is
> `display-buffer-alist' the application's domain is the ACTION
> argument.  Please try to stick to these two, maybe not even mentioning
> the rest at all.

I've now changed it to `display-buffer-alist'.

By the way, the example in the Emacs manual is this:

(customize-set-variable
 'display-buffer-alist
 '("\\*scratch\\*" (display-buffer-same-window)))

Which isn't an alist?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > By the way, the example in the Emacs manual is this:
 >
 > (customize-set-variable
 >   'display-buffer-alist
 >   '("\\*scratch\\*" (display-buffer-same-window)))
 >
 > Which isn't an alist?

Darn.  No wonder that people complain.  Maybe we should use one of the
examples from the Zen of Buffer Display.  In either case this is for
Emacs 27.2!

martin

bug#45688: 28.0.50; New action for display-buffer?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Date: Wed, 06 Jan 2021 13:02:12 +0100
> 
> 
> Say you start with this:
> 
> -----
> buf 1
> -----
> buf 2
> -----
> buf 3
> -----
> 
> and you're in buf 1.  `M-x display-buffer RET RET' then gives you:
> 
> -----
> buf 1
> -----
> buf 4
> -----
> buf 3
> -----
> 
> Repeat, and:
> 
> -----
> buf 1
> -----
> buf 4
> -----
> buf 2
> -----
> 
> Repeat, and:
> 
> -----
> buf 1
> -----
> buf 3
> -----
> buf 2
> -----
> 
> Repeat, and:
> 
> -----
> buf 1
> -----
> buf 3
> -----
> buf 4
> -----
> 
> Repeat, and:
> 
> -----
> buf 1
> -----
> buf 2
> -----
> buf 4
> -----
> 
> That is, it cycles the windows.  If there are more windows, it'll go
> through all the other windows in order, choosing the
> least-recently-switched-to window.

I'm not sure I understand what kind of use case would find the above
convenient.  display-buffer is supposed to be for temporary display of
some buffer that you soon enough want to go away, so why does it
matter which buffer it replaces?  Or if it does matter, wouldn't it be
more reasonable to have explicit control on that, instead of letting
it cycle?

I feel that I'm missing something here.

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure I understand what kind of use case would find the above
> convenient.  display-buffer is supposed to be for temporary display of
> some buffer that you soon enough want to go away, so why does it
> matter which buffer it replaces? 

It's a user command (on `C-x 4 C-o'), and apparently some people use it
a lot?  I've never used it myself, but I see the charm -- if you want
to look at a different buffer while working in the current buffer, it
seems quite convenient.  (You don't have to pop back to the original
buffer as with `switch-to-buffer-other-window' or the like.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: 45688@debbugs.gnu.org
> Date: Wed, 06 Jan 2021 18:20:20 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > I'm not sure I understand what kind of use case would find the above
> > convenient.  display-buffer is supposed to be for temporary display of
> > some buffer that you soon enough want to go away, so why does it
> > matter which buffer it replaces? 
> 
> It's a user command (on `C-x 4 C-o'), and apparently some people use it
> a lot?

I use it myself, when some buffer pops up in a window I use for
another buffer, and fails to pop down (example: *Shell Command
Output*).  But then why would I want the buffer be displayed in a
random window?

> I've never used it myself, but I see the charm -- if you want
> to look at a different buffer while working in the current buffer, it
> seems quite convenient.

Sure, but once again: why do that in some random window, as opposed to
a fixed one?  I'm not asking about the utility of display-buffer, I'm
asking about this particular aspect: the fact that it doesn't pop up
in a fixed window.

I guess I'm missing something here, if this feature is deemed so
important that it caused jwz to post a complete blog about that.

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

Eli Zaretskii <eliz@gnu.org> writes:

> I use it myself, when some buffer pops up in a window I use for
> another buffer, and fails to pop down (example: *Shell Command
> Output*).  But then why would I want the buffer be displayed in a
> random window?

Not in a random window, but not in the last-chosen window, either.  That
way, if you're `display-buffer'-ing a number of buffers, you'll get them
all in the other windows, instead of reusing the same one.

> I guess I'm missing something here, if this feature is deemed so
> important that it caused jwz to post a complete blog about that.

It would have been nice if he had been more specific; yes.  But the
XEmacs behaviour seems quite natural to me -- if you have three windows,
and you're working in one, and you want to look at two other buffers at
the same time, then the XEmacs `display-buffer' does the right thing for
that.

As Martin explained, display-buffer-use-some-window almost does this,
but since switching to the buffer doesn't count as "use", it doesn't
work for this particular use case.

Adding another action here (that just calls
display-buffer-use-some-window, but then marks the window as having been
used) seems trivial, so I think I'll try doing that and see what that
feels like.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > As Martin explained, display-buffer-use-some-window almost does this,
 > but since switching to the buffer doesn't count as "use", it doesn't
 > work for this particular use case.

Switching to the buffer does count as "use" but 'display-buffer' doesn't
switch to the buffer because it doesn't select it.

 > Adding another action here (that just calls
 > display-buffer-use-some-window, but then marks the window as having been
 > used) seems trivial, so I think I'll try doing that and see what that
 > feels like.

Add a function say 'window-bump-use-time' that does

       w->use_time = ++window_select_count;

and call that from `display-buffer'.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> As Martin explained, display-buffer-use-some-window almost does this,
>> but since switching to the buffer doesn't count as "use", it doesn't
>> work for this particular use case.
>
> Switching to the buffer does count as "use" but 'display-buffer' doesn't
> switch to the buffer because it doesn't select it.

Sorry; I meant "use the window".

> Add a function say 'window-bump-use-time' that does
>
>       w->use_time = ++window_select_count;
>
> and call that from `display-buffer'.

Thanks!

I've now pushed this -- like I said, my familiarity with this area in
Emacs is approx. nil, so if this implementation is beyond stupid, please
feel free to pipe up.  :-)

But with this setting:

  (setq display-buffer-base-action '(display-buffer-use-least-recent-window))

it's quite XEmacs-like?  I think?  At least in my tests.  So now we've
got that all-important ex-XEmacs market segment covered.

The only oddity is that it'll resize windows to their previous sizes --
is there any way to inhibit that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > But with this setting:
 >
 >    (setq display-buffer-base-action '(display-buffer-use-least-recent-window))
 >
 > it's quite XEmacs-like?

But this should be expressed in terms of `display-buffer-alist'.

 > I think?  At least in my tests.  So now we've
 > got that all-important ex-XEmacs market segment covered.

This

+  (when-let ((window (display-buffer-use-some-window
+                      buffer (cons (cons 'inhibit-same-window t) alist))))
+    (window-bump-use-time window)))

alone will bump the use time for a _reused_ window only.  A freshly
popped up window will continue to be the first candidate for reuse and
only then enter the cycle of windows to reuse.  XEmacs treats _all_
windows it creates or uses equal in this regard including those on other
frames.

 > The only oddity is that it'll resize windows to their previous sizes --
 > is there any way to inhibit that?

How comes that window gets ever resized in the first place?

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> This
>
> +  (when-let ((window (display-buffer-use-some-window
> +                      buffer (cons (cons 'inhibit-same-window t) alist))))
> +    (window-bump-use-time window)))
>
> alone will bump the use time for a _reused_ window only.  A freshly
> popped up window will continue to be the first candidate for reuse and
> only then enter the cycle of windows to reuse.

How do we bump the use to for a new window, then?

> XEmacs treats _all_ windows it creates or uses equal in this regard
> including those on other frames.

Oh, other frames, too...  I think we'll leave that as an exercise for
the reader.

>> The only oddity is that it'll resize windows to their previous sizes --
>> is there any way to inhibit that?
>
> How comes that window gets ever resized in the first place?

I have no idea, but it's totally repeatable -- the window that's
switched to becomes the largest window, and it's rather distracting.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >> This
 >>
 >> +  (when-let ((window (display-buffer-use-some-window
 >> +                      buffer (cons (cons 'inhibit-same-window t) alist))))
 >> +    (window-bump-use-time window)))
 >>
 >> alone will bump the use time for a _reused_ window only.  A freshly
 >> popped up window will continue to be the first candidate for reuse and
 >> only then enter the cycle of windows to reuse.
 >
 > How do we bump the use to for a new window, then?

By bumping the time stamp of _any_ window 'display-buffer' uses for
displaying a buffer (just like XEmacs does).

 >> XEmacs treats _all_ windows it creates or uses equal in this regard
 >> including those on other frames.
 >
 > Oh, other frames, too...  I think we'll leave that as an exercise for
 > the reader.

Then when you switch from one frame to another, any problems you've
found on the previous frame will immediately reemerge on the new frame.

 >>> The only oddity is that it'll resize windows to their previous sizes --
 >>> is there any way to inhibit that?
 >>
 >> How comes that window gets ever resized in the first place?
 >
 > I have no idea, but it's totally repeatable -- the window that's
 > switched to becomes the largest window, and it's rather distracting.

Please put some silly function on 'window-size-change-functions' and
make it produce a backtrace.

martin

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > Please put some silly function on 'window-size-change-functions' and
 > make it produce a backtrace.

Sorry, no.  Please assert first that the backsizing gets done inside the
following chunk:

       (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)))

Then we can try to find out who created that quad before and why.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

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

martin rudalics <rudalics@gmx.at> writes:

> Sorry, no.  Please assert first that the backsizing gets done inside the
> following chunk:
>
>       (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)))
>
> Then we can try to find out who created that quad before and why.

I put some messaging in that code, and it is indeed that bit that does
the resizing.

Here's a reproducer from emacs -Q:

(progn
  (setq display-buffer-alist '((".*" display-buffer-use-least-recent-window)))
  (pop-to-buffer "file1")
  (pop-to-buffer "file2")
  (split-window-below)
  (pop-to-buffer "file3"))

I end up with the following, and file3 in an oddly large window.


[-- Attachment #2: Type: image/png, Size: 68725 bytes --]

[-- Attachment #3: Type: text/plain, Size: 106 bytes --]




-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

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

 > I put some messaging in that code, and it is indeed that bit that does
 > the resizing.
 >
 > Here's a reproducer from emacs -Q:
 >
 > (progn
 >    (setq display-buffer-alist '((".*" display-buffer-use-least-recent-window)))
 >    (pop-to-buffer "file1")
 >    (pop-to-buffer "file2")
 >    (split-window-below)
 >    (pop-to-buffer "file3"))
 >
 > I end up with the following, and file3 in an oddly large window.

So you have been splicing in a 'split-window-below' between those calls.
Why didn't you tell me before?  'split-window-below' copies the
'quit-restore' parameter to the new window and that's what you get.

Let's try to clean up the height value when copying the 'quit-restore'
parameter as attached.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: split-window-quit-restore.diff --]
[-- Type: text/x-patch; name="split-window-quit-restore.diff", Size: 2338 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index a6cdd4dec2..8651d7502f 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5528,6 +5528,24 @@ split-window-keep-point
   :type 'boolean
   :group 'windows)

+(defun window--split-window-quit-restore (old-window new-window)
+  "Copy 'quit-restore' parameter to new window after splitting.
+OLD-WINDOW is the window that was split, NEW-WINDOW is the new
+window.  This cleans up any height value that could be used to
+restore the old height of NEW-WINDOW in a later invocation of
+`display-buffer'."
+  (let* ((quit-restore (window-parameter old-window 'quit-restore))
+	 (quad (nth 1 quit-restore)))
+    (when quit-restore
+      (when (and (listp quad) (integerp (nth 3 quad)))
+        ;; nth 3 of quad is the old-window height of OLD-WINDOW.  This
+        ;; value is meaningless in NEW-WINDOW so set it to the present
+        ;; height of NEW-WINDOW.
+        (setq quit-restore (copy-tree quit-restore))
+        (rplaca (cdddr (nth 1 quit-restore)) (window-total-height new-window)))
+
+      (set-window-parameter new-window 'quit-restore quit-restore))))
+
 (defun split-window-below (&optional size)
   "Split the selected window into two windows, one above the other.
 The selected window is above.  The newly split-off window is
@@ -5575,9 +5593,7 @@ split-window-below
 	     (set-window-point new-window old-point)
 	     (select-window new-window))))
     ;; Always copy quit-restore parameter in interactive use.
-    (let ((quit-restore (window-parameter old-window 'quit-restore)))
-      (when quit-restore
-	(set-window-parameter new-window 'quit-restore quit-restore)))
+    (window--split-window-quit-restore old-window new-window)
     new-window))

 (defalias 'split-window-vertically 'split-window-below)
@@ -5604,9 +5620,7 @@ split-window-right
       (error "Size of new window too small"))
     (setq new-window (split-window nil size t))
     ;; Always copy quit-restore parameter in interactive use.
-    (let ((quit-restore (window-parameter old-window 'quit-restore)))
-      (when quit-restore
-	(set-window-parameter new-window 'quit-restore quit-restore)))
+    (window--split-window-quit-restore old-window new-window)
     new-window))

 (defalias 'split-window-horizontally 'split-window-right)

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

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

 > Let's try to clean up the height value when copying the 'quit-restore'
 > parameter as attached.

Just found a related bug in the code.  When 'display-buffer' makes a new
window on the left or right, it sets that value to the window's width
which doesn't make any sense.  The current code can handle only height
values as for 'shrink-window-if-larger-than-buffer' or the default
'fit-window-to-buffer'.  So please use the patch attached here.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: split-window-quit-restore.diff --]
[-- Type: text/x-patch; name="split-window-quit-restore.diff", Size: 2456 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index a6cdd4dec2..c09d9532e3 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -5543,10 +5543,12 @@ split-window-below
 Otherwise, the window starts are chosen so as to minimize the
 amount of redisplay; this is convenient on slow terminals."
   (interactive "P")
-  (let ((old-window (selected-window))
-	(old-point (window-point))
-	(size (and size (prefix-numeric-value size)))
-        moved-by-window-height moved new-window bottom)
+  (let* ((old-window (selected-window))
+	 (old-point (window-point))
+	 (size (and size (prefix-numeric-value size)))
+         (quit-restore (window-parameter old-window 'quit-restore))
+	 (quad (nth 1 quit-restore))
+         moved-by-window-height moved new-window bottom)
     (when (and size (< size 0) (< (- size) window-min-height))
       ;; `split-window' would not signal an error here.
       (error "Size of new window too small"))
@@ -5574,10 +5576,18 @@ split-window-below
 	     (<= (window-start new-window) old-point)
 	     (set-window-point new-window old-point)
 	     (select-window new-window))))
-    ;; Always copy quit-restore parameter in interactive use.
-    (let ((quit-restore (window-parameter old-window 'quit-restore)))
-      (when quit-restore
-	(set-window-parameter new-window 'quit-restore quit-restore)))
+
+    (when quit-restore
+      (when (and (listp quad) (integerp (nth 3 quad)))
+        ;; nth 3 of quad is the old-window height of OLD-WINDOW.  This
+        ;; value is meaningless in NEW-WINDOW so set it to the present
+        ;; height of NEW-WINDOW.
+        (setq quit-restore (copy-tree quit-restore))
+        (rplaca (cdddr (nth 1 quit-restore))
+                (window-total-height new-window)))
+
+      (set-window-parameter new-window 'quit-restore quit-restore))
+
     new-window))

 (defalias 'split-window-vertically 'split-window-below)
@@ -6440,9 +6450,7 @@ display-buffer-record-window
 		     ;; Preserve window-point-insertion-type (Bug#12855).
 		     (copy-marker
 		      (window-point window) window-point-insertion-type)
-		     (if (window-combined-p window)
-                         (window-total-height window)
-                       (window-total-width window)))
+                     (window-total-height window))
 	       (selected-window) buffer)))))
    ((eq type 'window)
     ;; WINDOW has been created on an existing frame.

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Just found a related bug in the code.  When 'display-buffer' makes a new
> window on the left or right, it sets that value to the window's width
> which doesn't make any sense.  The current code can handle only height
> values as for 'shrink-window-if-larger-than-buffer' or the default
> 'fit-window-to-buffer'.  So please use the patch attached here.

That one seems to fix the reproducer, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> Here's a reproducer from emacs -Q:
>>
>> (progn
>>    (setq display-buffer-alist '((".*"
>> display-buffer-use-least-recent-window)))
>>    (pop-to-buffer "file1")
>>    (pop-to-buffer "file2")
>>    (split-window-below)
>>    (pop-to-buffer "file3"))
>>
>> I end up with the following, and file3 in an oddly large window.
>
> So you have been splicing in a 'split-window-below' between those calls.
> Why didn't you tell me before?

I wasn't -- I was jumping around between windows manually, but that's
the reproducer I came up with.

> 'split-window-below' copies the
> 'quit-restore' parameter to the new window and that's what you get.
>
> Let's try to clean up the height value when copying the 'quit-restore'
> parameter as attached.

This fixes the reproducer, but not the case of actually jumping around
manually -- `M-x display-buffer' sometimes chooses to resize the
windows.  :-/

In related news, get-lru-window doesn't seem to work reliably?  I don't
have a reproducer for that, either, but it seems to happen if I have a
three window frame, and I call:

(setq lru (get-lru-window (selected-frame) nil t))
(window-bump-use-time lru)
(get-lru-window (selected-frame) nil t)

will then return the same window as `lru'...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > This fixes the reproducer, but not the case of actually jumping around
 > manually -- `M-x display-buffer' sometimes chooses to resize the
 > windows.  :-/

Have you ever read the Elisp manual where it says

      ‘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 ‘display-buffer-pop-up-window’ and
      ‘display-buffer-use-some-window’ are not very suited for this
      purpose due to their somewhat chaotic nature in more complex
      configurations.

Maybe you bumped into this with your experiments, in particular when
trying to reuse a window that you've already used before.

'display-buffer' still lives in the paradigm of <= 2 windows frames set
up in the past century and probably still the only reasonable one for
TTY frames.  For that it supports things like 'even-window-heights' and
a 'split-height-threshold' of 80 with its "If, however, a window is the
only window on its frame, or all the other ones are dedicated,
‘split-window-sensibly’ may split it vertically disregarding the value
of this variable." exception.  Hence a first step to get your rampages
produce more reasonable results might be to ask whether that paradigm is
still a valid one.

If we don't want to do that, we can try to do some cosmetics in the size
restoring mechanism.  Basically, that mechanism is used when you display
a temporary buffer in a two windows layout and you have
'temp-buffer-resize-mode' enabled.  Suppose with emacs -Q you enable
that mode and type C-h f push RET and then C-x 4 f to find some file.
This will re-enlarge the window used for showing the 'push' help to its
prior size so you get what you intend for showing that file.

If we want 'display-buffer-use-some-window' to not do such size
restorations, we can just add a 'do-no-restore-size' alist entry and an
appropriate check like

       (when (and (not (cdr (assq 'do-no-restore-size alist)))
                  (listp quad)
                  (integerp (nth 3 quad))
                  (> (nth 3 quad) (window-total-height window)))

'display-buffer-use-least-recent-window' (and/or the application/user)
could then set that entry at their like.

 > In related news, get-lru-window doesn't seem to work reliably?  I don't
 > have a reproducer for that, either, but it seems to happen if I have a
 > three window frame, and I call:
 >
 > (setq lru (get-lru-window (selected-frame) nil t))
 > (window-bump-use-time lru)
 > (get-lru-window (selected-frame) nil t)
 >
 > will then return the same window as `lru'...

How do you "call"?  I suppose there's no chance to make another window
but the selected one the mru one.  We would have to look into the inner
workings of that "call".

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> 'display-buffer' still lives in the paradigm of <= 2 windows frames set
> up in the past century and probably still the only reasonable one for
> TTY frames.  For that it supports things like 'even-window-heights' and
> a 'split-height-threshold' of 80 with its "If, however, a window is the
> only window on its frame, or all the other ones are dedicated,
> ‘split-window-sensibly’ may split it vertically disregarding the value
> of this variable." exception.  Hence a first step to get your rampages
> produce more reasonable results might be to ask whether that paradigm is
> still a valid one.

Well, it's certainly the most common one, but having more than two
windows in a frame isn't unheard of, either.

> If we don't want to do that, we can try to do some cosmetics in the size
> restoring mechanism.  Basically, that mechanism is used when you display
> a temporary buffer in a two windows layout and you have
> 'temp-buffer-resize-mode' enabled.  Suppose with emacs -Q you enable
> that mode and type C-h f push RET and then C-x 4 f to find some file.
> This will re-enlarge the window used for showing the 'push' help to its
> prior size so you get what you intend for showing that file.

I think that makes sense...  if you have that mode enabled.  But if
you're not asking Emacs to resize windows in this way, then having
`display-buffer' resizing windows is somewhat confusing.

> If we want 'display-buffer-use-some-window' to not do such size
> restorations, we can just add a 'do-no-restore-size' alist entry and an
> appropriate check like
>
>       (when (and (not (cdr (assq 'do-no-restore-size alist)))
>                  (listp quad)
>                  (integerp (nth 3 quad))
>                  (> (nth 3 quad) (window-total-height window)))
>
> 'display-buffer-use-least-recent-window' (and/or the application/user)
> could then set that entry at their like.

Makes sense.

>> In related news, get-lru-window doesn't seem to work reliably?  I don't
>> have a reproducer for that, either, but it seems to happen if I have a
>> three window frame, and I call:
>>
>> (setq lru (get-lru-window (selected-frame) nil t))
>> (window-bump-use-time lru)
>> (get-lru-window (selected-frame) nil t)
>>
>> will then return the same window as `lru'...
>
> How do you "call"?  I suppose there's no chance to make another window
> but the selected one the mru one.  We would have to look into the inner
> workings of that "call".

eval the expressions.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > Well, it's certainly the most common one, but having more than two
 > windows in a frame isn't unheard of, either.

Right.  But how reconcile the two approaches?

 > I think that makes sense...  if you have that mode enabled.  But if
 > you're not asking Emacs to resize windows in this way, then having
 > `display-buffer' resizing windows is somewhat confusing.

The one you cited earlier certainly is.  The basic idea is that a window
that was good for 'display-buffer' once, should be good again.  Unless
it shrunk in between.

 >>> In related news, get-lru-window doesn't seem to work reliably?  I don't
 >>> have a reproducer for that, either, but it seems to happen if I have a
 >>> three window frame, and I call:
 >>>
 >>> (setq lru (get-lru-window (selected-frame) nil t))
 >>> (window-bump-use-time lru)
 >>> (get-lru-window (selected-frame) nil t)
 >>>
 >>> will then return the same window as `lru'...
 >>
 >> How do you "call"?  I suppose there's no chance to make another window
 >> but the selected one the mru one.  We would have to look into the inner
 >> workings of that "call".
 >
 > eval the expressions.

Where and how?   If you do that in the minibuffer window alone, you
should get a result not affected by 'select-window'.  Otherwise, all
bets are off.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Where and how?   If you do that in the minibuffer window alone, you
> should get a result not affected by 'select-window'.  Otherwise, all
> bets are off.

`C-x C-e'  :-)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >> Where and how?   If you do that in the minibuffer window alone, you
 >> should get a result not affected by 'select-window'.  Otherwise, all
 >> bets are off.
 >
 > `C-x C-e'  :-)

With emacs -Q, do C-x 2 twice so you get three windows (since you don't
want to count the selected one) at least and do C-x C-e in the selected
window with point after the following form:

(let ((lru (get-lru-window (next-frame) nil t)))
   (window-bump-use-time lru)
   (message "%s .. %s" lru (get-lru-window (next-frame) nil t)))

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> If we don't want to do that, we can try to do some cosmetics in the size
> restoring mechanism.  Basically, that mechanism is used when you display
> a temporary buffer in a two windows layout and you have
> 'temp-buffer-resize-mode' enabled.  Suppose with emacs -Q you enable
> that mode and type C-h f push RET and then C-x 4 f to find some file.
> This will re-enlarge the window used for showing the 'push' help to its
> prior size so you get what you intend for showing that file.

I didn't know this trick.  But it works only for 'C-x 4 f',
not for 'C-x 4 1':

with enabled temp-buffer-resize-mode
1. C-h f push RET
2. C-x o TAB
3. C-x 4 1 RET

doesn't re-enlarge the window to its prior size.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > I didn't know this trick.  But it works only for 'C-x 4 f',
 > not for 'C-x 4 1':
 >
 > with enabled temp-buffer-resize-mode
 > 1. C-h f push RET
 > 2. C-x o TAB
 > 3. C-x 4 1 RET
 >
 > doesn't re-enlarge the window to its prior size.

Only 'display-buffer-use-some-window' currently does.  What does C-x 4 1
call?

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> I didn't know this trick.  But it works only for 'C-x 4 f',
>> not for 'C-x 4 1':
>>
>> with enabled temp-buffer-resize-mode
>> 1. C-h f push RET
>> 2. C-x o TAB
>> 3. C-x 4 1 RET
>>
>> doesn't re-enlarge the window to its prior size.
>
> Only 'display-buffer-use-some-window' currently does.  What does C-x 4 1
> call?

It calls same-window-prefix, i.e. display-buffer-same-window.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > It calls same-window-prefix, i.e. display-buffer-same-window.

Restoring the height of the selected window could be quite surprising.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> It calls same-window-prefix, i.e. display-buffer-same-window.
>
> Restoring the height of the selected window could be quite surprising.

Indeed, better would be to add an option to not resize it by describe-function.
Maybe by adding a new option 'after-display-buffer-function' and set it
by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > Indeed, better would be to add an option to not resize it by describe-function.
 > Maybe by adding a new option 'after-display-buffer-function' and set it
 > by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?

You can already use a 'window-height' alist entry and set it to
'shrink-window-if-larger-than-buffer'.  What would an
'after-display-buffer-function' option do differently?

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> Indeed, better would be to add an option to not resize it by describe-function.
>> Maybe by adding a new option 'after-display-buffer-function' and set it
>> by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?
>
> You can already use a 'window-height' alist entry and set it to
> 'shrink-window-if-larger-than-buffer'.  What would an
> 'after-display-buffer-function' option do differently?

The problem is that currently shrink-window-if-larger-than-buffer
is used in many commands by default, but this behavior is hard-coded
and can't be disabled by customization.  These commands include
help commands,  vc-log, vc-diff, ...

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > The problem is that currently shrink-window-if-larger-than-buffer
 > is used in many commands by default, but this behavior is hard-coded
 > and can't be disabled by customization.  These commands include
 > help commands,  vc-log, vc-diff, ...

If these commands use 'display-buffer', we should rewrite them anyway so
the user can customize the size changing behavior.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> Indeed, better would be to add an option to not resize it by describe-function.
>> Maybe by adding a new option 'after-display-buffer-function' and set it
>> by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?
>
> You can already use a 'window-height' alist entry and set it to
> 'shrink-window-if-larger-than-buffer'.  What would an
> 'after-display-buffer-function' option do differently?

It seems the patch that you recently sent to emacs-devel fixes this problem?

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >>> Indeed, better would be to add an option to not resize it by describe-function.
 >>> Maybe by adding a new option 'after-display-buffer-function' and set it
 >>> by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?
 >>
 >> You can already use a 'window-height' alist entry and set it to
 >> 'shrink-window-if-larger-than-buffer'.  What would an
 >> 'after-display-buffer-function' option do differently?
 >
 > It seems the patch that you recently sent to emacs-devel fixes this problem?

You mean this problem

   The problem is that currently shrink-window-if-larger-than-buffer
   is used in many commands by default, but this behavior is hard-coded
   and can't be disabled by customization.  These commands include
   help commands,  vc-log, vc-diff, ...

supposedly caused by 'temp-buffer-resize-mode'?  Would we then be able
to close this bug or is there anything else left?

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>>>> Indeed, better would be to add an option to not resize it by describe-function.
>>>> Maybe by adding a new option 'after-display-buffer-function' and set it
>>>> by default to 'shrink-window-if-larger-than-buffer' in *Help* buffers?
>>>
>>> You can already use a 'window-height' alist entry and set it to
>>> 'shrink-window-if-larger-than-buffer'.  What would an
>>> 'after-display-buffer-function' option do differently?
>>
>> It seems the patch that you recently sent to emacs-devel fixes this problem?
>
> You mean this problem
>
>   The problem is that currently shrink-window-if-larger-than-buffer
>   is used in many commands by default, but this behavior is hard-coded
>   and can't be disabled by customization.  These commands include
>   help commands,  vc-log, vc-diff, ...
>
> supposedly caused by 'temp-buffer-resize-mode'?

Your recent patch in emacs-devel allows customization of help commands,
but not vc-log, vc-diff.  A separate request could be created for
customization of shrink-window-if-larger-than-buffer in vc-log, vc-diff.
(I mistakenly assumed that help-mode uses shrink-window-if-larger-than-buffer,
but actually help-mode resizes with 'temp-buffer-resize-mode').

> Would we then be able to close this bug or is there anything else left?

I don't know, Lars opened this request.  And I reported this problem
only after you mentioned 'temp-buffer-resize-mode'.

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

>> Would we then be able to close this bug or is there anything else left?
>
> I don't know, Lars opened this request.  And I reported this problem
> only after you mentioned 'temp-buffer-resize-mode'.

I think the originally requested XEmacs-like action for display-buffer
works OK?  At least it did the last time I tested it, if I remember
correctly.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > Your recent patch in emacs-devel allows customization of help commands,
 > but not vc-log, vc-diff.  A separate request could be created for
 > customization of shrink-window-if-larger-than-buffer in vc-log, vc-diff.
 > (I mistakenly assumed that help-mode uses shrink-window-if-larger-than-buffer,
 > but actually help-mode resizes with 'temp-buffer-resize-mode').

I don't understand any more which patch you are talking about now and
whether that patch fixes any problems or something is still unresolved.
Please enlighten me.

Thanks, martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> Your recent patch in emacs-devel allows customization of help commands,
>> but not vc-log, vc-diff.  A separate request could be created for
>> customization of shrink-window-if-larger-than-buffer in vc-log, vc-diff.
>> (I mistakenly assumed that help-mode uses shrink-window-if-larger-than-buffer,
>> but actually help-mode resizes with 'temp-buffer-resize-mode').
>
> I don't understand any more which patch you are talking about now and
> whether that patch fixes any problems or something is still unresolved.
> Please enlighten me.

I meant this patch:

https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >>> Your recent patch in emacs-devel allows customization of help commands,
 >>> but not vc-log, vc-diff.  A separate request could be created for
 >>> customization of shrink-window-if-larger-than-buffer in vc-log, vc-diff.
 >>> (I mistakenly assumed that help-mode uses shrink-window-if-larger-than-buffer,
 >>> but actually help-mode resizes with 'temp-buffer-resize-mode').
 >>
 >> I don't understand any more which patch you are talking about now and
 >> whether that patch fixes any problems or something is still unresolved.
 >> Please enlighten me.
 >
 > I meant this patch:
 >
 > https://lists.gnu.org/archive/html/emacs-devel/2021-09/msg01873.html

In which sense does this patch allow "customization of help commands,
but not vc-log, vc-diff"?  This patch is only about inhibiting resizing
of temporary buffer windows when a 'window-height', 'window-width' or
'window-size' action alist entry was specified for such a window.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> A separate request could be created for customization of
> shrink-window-if-larger-than-buffer in vc-log, vc-diff.  (I mistakenly
> assumed that help-mode uses shrink-window-if-larger-than-buffer, but
> actually help-mode resizes with 'temp-buffer-resize-mode').

A separate request was created in bug#51062.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

close 45688 28.1
quit

According to Lars

 >>> Would we then be able to close this bug or is there anything else left?
 >>
 >> I don't know, Lars opened this request.  And I reported this problem
 >> only after you mentioned 'temp-buffer-resize-mode'.
 >
 > I think the originally requested XEmacs-like action for display-buffer
 > works OK?  At least it did the last time I tested it, if I remember
 > correctly.

and according to you

 >> A separate request could be created for customization of
 >> shrink-window-if-larger-than-buffer in vc-log, vc-diff.  (I mistakenly
 >> assumed that help-mode uses shrink-window-if-larger-than-buffer, but
 >> actually help-mode resizes with 'temp-buffer-resize-mode').
 >
 > A separate request was created in bug#51062.

I'm closing this bug then to avoid further confusions.

Thanks, martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>>> alone will bump the use time for a _reused_ window only.  A freshly
>>> popped up window will continue to be the first candidate for reuse and
>>> only then enter the cycle of windows to reuse.
>>
>> How do we bump the use to for a new window, then?
>
> By bumping the time stamp of _any_ window 'display-buffer' uses for
> displaying a buffer (just like XEmacs does).

But where would we do that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> then the XEmacs `display-buffer' does the right thing for that.

XEmacs does absolutely the wrong thing.  When I have a window
with a source buffer, then run grep in another window, then
want to visit grep hits in the third window, visiting
the next file from the grep window obscures the window
with the source buffer, it doesn't use the same window
where I'm visiting grep hits.

I tried to replace get-lru-window with get-mru-window
but it selects the current window which is mru indeed,
but makes no sense - what is needed is mru-1, then
I tried to set the arg NOT-SELECTED of get-mru-window to t
in display-buffer-use-some-window, then it works sensibly.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > XEmacs does absolutely the wrong thing.  When I have a window
 > with a source buffer, then run grep in another window, then
 > want to visit grep hits in the third window, visiting
 > the next file from the grep window obscures the window
 > with the source buffer, it doesn't use the same window
 > where I'm visiting grep hits.

We're asking too much from 'display-buffer' here.  It can't second-guess
a user's intentions in particular with the interpretation of grep and
xref hits.  I see three basic patterns here:

- a hit is detected in a buffer currently shown but that window's point
   is the position where I want to continue to work - show the hit in
   another window,

- a hit is detected in a buffer currently shown and I don't care about
   that window's current point - move that window's point to the hit,

- one or a few hits are already shown in windows and I would like to
   either compare them or at least for the moment not lose any of these
   windows (no matter how small they are) - try popping up yet another
   window.

Do you have more?  In either case, I cannot generally predict beforehand
which of these will apply in a specific situation so I'd like to see
some versatile grep/xref-DWIM layer that handles these patterns ad hoc,
possibly assisted by some simple M-g binding.  But we have to identify
all potentially useful patterns first.

 > I tried to replace get-lru-window with get-mru-window
 > but it selects the current window which is mru indeed,
 > but makes no sense - what is needed is mru-1, then
 > I tried to set the arg NOT-SELECTED of get-mru-window to t
 > in display-buffer-use-some-window, then it works sensibly.

Neither of these will catch all needs: mru-ish replaces the previous hit
found, lru-ish the oldest hit found so far.  Any of these might be the
most interesting one found so far.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> We're asking too much from 'display-buffer' here.  It can't second-guess
> a user's intentions in particular with the interpretation of grep and
> xref hits.  I see three basic patterns here:

I'm wondering whether there should be a separate
`display-buffer-command' that's bound to `C-x 4 C-o' with its own
display rules.  :-)  That is, when the user is using this command
explicitly, then it's not obvious that the user wants the same rules as
with the "automatic" display-buffer stuff (popping from *grep* buffers
and the like).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>> We're asking too much from 'display-buffer' here.  It can't second-guess
>> a user's intentions in particular with the interpretation of grep and
>> xref hits.  I see three basic patterns here:
>
> I'm wondering whether there should be a separate
> `display-buffer-command' that's bound to `C-x 4 C-o' with its own
> display rules.  :-)  That is, when the user is using this command
> explicitly, then it's not obvious that the user wants the same rules as
> with the "automatic" display-buffer stuff (popping from *grep* buffers
> and the like).

Currently I'm using windmove-display-* group of commands to
instruct display-buffer where I want to display the buffer, e.g.:

's-<down> C-o' to open a grep/xref hit in the bottom window,
's-<left> C-o' to open a grep/xref hit in the left window, etc.

It's quite tedious to type that prefix before every key that
visits a grep hit.  What is still missing is some kind of DWIM
to make display-buffer smarter.

Here is what seems to work to visit grep/xref hits in the same window
where all previous hits were visited:

#+begin_src emacs-lisp
(defun display-buffer-from-grep-p (_buffer-name _action)
  (with-current-buffer (window-buffer)
    (derived-mode-p 'compilation-mode)))

(push '(display-buffer-from-grep-p display-buffer-reuse-previous-window)
      display-buffer-alist)

(defvar-local display-buffer-previous-window nil)

(defun display-buffer-reuse-previous-window (buffer alist)
  "Display BUFFER in the same window that was already used as destination."
  (let ((window display-buffer-previous-window))
    (if (window-live-p window)
        (window--display-buffer buffer window 'reuse alist)
      (setq window (display-buffer-use-some-window
                    buffer (cons (cons 'inhibit-same-window t) alist)))
      (setq-local display-buffer-previous-window window)
      window)))
#+end_src

One problem is that other display-buffer actions don't set
'display-buffer-previous-window'.  It would be nice to do
this in some low-level function in window.el.

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

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

> One problem is that other display-buffer actions don't set
> 'display-buffer-previous-window'.  It would be nice to do
> this in some low-level function in window.el.

If this is impossible to add this to window.el then a workaround is
to use such advice that helps to visit grep/xref hits in the window
where all previous hits were visited:

#+begin_src emacs-lisp
(advice-add 'window--display-buffer :around
            (lambda (orig-fun &rest args)
              (let ((buffer (current-buffer))
                    (window (apply orig-fun args)))
                (with-current-buffer buffer
                  (setq-local display-buffer-previous-window window))
                window))
            '((name . window--display-buffer-set-previous-window)))

(defvar-local display-buffer-previous-window nil)

(defun display-buffer-from-grep-p (_buffer-name _action)
  (with-current-buffer (window-buffer)
    (derived-mode-p 'compilation-mode)))

(add-to-list 'display-buffer-alist
             '(display-buffer-from-grep-p
               nil (previous-window . display-buffer-previous-window)))
#+end_src

The only problem is that display-buffer-in-previous-window
doesn't support 'previous-window' as a variable, only as a value,
i.e. currently it only supports:

  `(previous-window . ,display-buffer-previous-window)

maybe the following patch could be installed to support also

  '(previous-window . display-buffer-previous-window)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: previous-window.patch --]
[-- Type: text/x-diff, Size: 1011 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index 0a37d16273..d6b3d69b3d 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8239,6 +8239,7 @@ display-buffer-in-previous-window
 		   0)
 		  (display-buffer-reuse-frames 0)
 		  (t (last-nonminibuffer-frame))))
+         (previous-window (cdr (assq 'previous-window alist)))
 	 best-window second-best-window window)
     ;; Scan windows whether they have shown the buffer recently.
     (catch 'best
@@ -8252,7 +8253,9 @@ display-buffer-in-previous-window
 	    (throw 'best t)))))
     ;; When ALIST has a `previous-window' entry, that entry may override
     ;; anything we found so far.
-    (when (and (setq window (cdr (assq 'previous-window alist)))
+    (when (and previous-window (boundp previous-window))
+      (setq previous-window (symbol-value previous-window)))
+    (when (and (setq window previous-window)
 	       (window-live-p window)
 	       (or (eq buffer (window-buffer window))
                    (not (window-dedicated-p window))))

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> The only problem is that display-buffer-in-previous-window
> doesn't support 'previous-window' as a variable, only as a value,
> i.e. currently it only supports:
>
>   `(previous-window . ,display-buffer-previous-window)
>
> maybe the following patch could be installed to support also
>
>   '(previous-window . display-buffer-previous-window)

Now pushed to master.  Martin, please check if everything is right.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >> The only problem is that display-buffer-in-previous-window
 >> doesn't support 'previous-window' as a variable, only as a value,
 >> i.e. currently it only supports:
 >>
 >>    `(previous-window . ,display-buffer-previous-window)
 >>
 >> maybe the following patch could be installed to support also
 >>
 >>    '(previous-window . display-buffer-previous-window)
 >
 > Now pushed to master.  Martin, please check if everything is right.

Sorry.  I suppose you forgot to push it.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

>>> The only problem is that display-buffer-in-previous-window
>>> doesn't support 'previous-window' as a variable, only as a value,
>>> i.e. currently it only supports:
>>>
>>>    `(previous-window . ,display-buffer-previous-window)
>>>
>>> maybe the following patch could be installed to support also
>>>
>>>    '(previous-window . display-buffer-previous-window)
>>
>> Now pushed to master.  Martin, please check if everything is right.
>
> Sorry.  I suppose you forgot to push it.

Sorry, I pushed it now in commit fb05199b0b.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > Sorry, I pushed it now in commit fb05199b0b.

Thanks.  This strongly needs an example in the Elisp manual somewhere in
the "Handle subsequent invocations of ‘display-buffer’" area.  Could you
polish up the one using 'display-buffer-reuse-previous-window',
explaining how it is "more deterministic" in flavor?  And in a way that
makes it directly executable by the reader?  Then we could also try to
add the more chaotic approach Lars takes via 'get-lru-window' there.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> Thanks.  This strongly needs an example in the Elisp manual somewhere in
> the "Handle subsequent invocations of ‘display-buffer’" area.  Could you
> polish up the one using 'display-buffer-reuse-previous-window',
> explaining how it is "more deterministic" in flavor?  And in a way that
> makes it directly executable by the reader?  Then we could also try to
> add the more chaotic approach Lars takes via 'get-lru-window' there.

Maybe something:

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index f305d1a8ee..60f1cc1bc5 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -3840,6 +3840,9 @@ The Zen of Buffer Display
 popping up a new window.  You have to make sure that another window in
 the same combination can be shrunk instead, though.
 
+@item
+Using @code{previous-window} will try to display the buffer more deterministically.
+
 @item
 Side windows (@pxref{Side Windows}) can be used for displaying
 specific buffers always in a window at the same position of a frame.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 > +@item
 > +Using @code{previous-window} will try to display the buffer more deterministically.
 > +

I'm afraid this wouldn't be overly helpful.  We'd need to say at least
what "deterministically" means in this context and in which cases I
would want to use this option.  And we should specify the new semantics
of "previous" - earlier it referred to a window that showed _this_
buffer already, now it also refers to a window that was previously used
for showing _any_ buffer.

martin

bug#45688: 28.0.50; New action for display-buffer?

[Juri Linkov]

> Reading this:
>
> https://www.jwz.org/blog/2021/01/very-much-do-not-have-my-emacs-setup-just-how-i-like-it/
>
> I was curious how XEmacs did this command, so I installed XEmacs.  If
> there's two (or one) window, the command works the same in Emacs and
> XEmacs, but it's interesting what XEmacs does when there's more than two
> windows: It cycles the windows.

This is how Emacs already works - exactly as XEmacs
it uses get-lru-window that caused a lot of sadness
for many years.  Many times I asked to fix this to
use get-mru-window instead.  Maybe now is a good moment to
finally change this to work differently from XEmacs.

bug#45688: 28.0.50; New action for display-buffer?

[Drew Adams]

> Many times I asked to fix this to
> use get-mru-window instead.  Maybe now is a good moment to
> finally change this to work differently from XEmacs.

If such a change is made, please provide an option for choosing mru or lru.

I suspect that there are a fair number of users and libraries that expect lru to be used as the default (first in order), if only because that's been the case for a long time.

bug#45688: 28.0.50; New action for display-buffer?

[martin rudalics]

 >> I was curious how XEmacs did this command, so I installed XEmacs.  If
 >> there's two (or one) window, the command works the same in Emacs and
 >> XEmacs, but it's interesting what XEmacs does when there's more than two
 >> windows: It cycles the windows.
 >
 > This is how Emacs already works - exactly as XEmacs
 > it uses get-lru-window that caused a lot of sadness
 > for many years.  Many times I asked to fix this to
 > use get-mru-window instead.  Maybe now is a good moment to
 > finally change this to work differently from XEmacs.

Emacs doesn't work like XEmacs here.  When XEmacs has found a window to
display the buffer in, it does

               ;; Bring the window's previous buffer to the top of the
               ;; MRU chain.
               (if (window-buffer window)
                   (save-excursion
                     (save-selected-window
                       (select-window window)
                       (record-buffer (window-buffer window)))))

which selects the window and, as a side effect, updates its use time.
Which makes that window less likely for `display-buffer' to be used
again.

martin