bug#11810: 24.1.50; `vc-diff' shrinks pre-existing window

[Dmitry Gutov]

On 03.07.2012 20:40, martin rudalics wrote:
> > Because of `shrink-window-if-larger-than-buffer'.
>
> Sure.  But as I proposed earlier we could have handled this by setting
> `temp-buffer-resize-mode' to t in the diff-buffer.

This will re-introduce the issue, the one you said 
`temp-buffer-resize-mode' check was guarding from.
Namely, if I run `vc-diff', it reuses some window that has a neighbor 
vertically, I drag its window border to resize, then go back to the 
window and press 'q', it will restore the original height, like you said 
it shouldn't.

>  > `even-window-heights' could have contributed in another case, if my
>  > windows were not of equal height already.
>
> `even-window-heights' is customizable so it's not a primary issue.  But
> we could undo the evening in `quit-window'.

I agree.

>  >> to handle when quitting a window.  Maybe the best solution is to set
> the
>  >> 3rd element of the quit-restore parameter iff either
>  >>
>  >> (1) `temp-buffer-resize-mode' is non-nil, or
>  >
>  > Maybe just when `fit-window-to-buffer' is called? That would account for
>  > `shrink-window-if-larger-than-buffer', too.
>
> Unless it's an interactive call of
> `shrink-window-if-larger-than-buffer' probably.

Well, yes. I think that's the hard part - deciding on the set of 
functions and if we need to do the check with `called-interactively-p' 
in some of them.

>  >> `adjust-window-trailing-edge' would be an obvious candidate.  But which
>  >> window's parameter would you clear?  Both?
>  >
>  > What's the second one? Please keep in mind that I don't know the
>  > window.el codebase, I'm just reading the code along the discussion.
>
> `adjust-window-trailing-edge' drags an edge between two windows and
> usually resizes the two windows adjacent to the edge, for example, when
> mouse-dragging the mode-line.  Hence we have two candidates for clearing
> a quit-restore parameter's size element.

Then yes, both of them, I guess.
`enlarge-window' and `enlarge-window-horizontally' could be another 
candidates. Not sure about `delete-window' (when we're deleting one 
window in a configuration), could be either way.

>  > If `temp-buffer-resize-mode' were on, that wouldn't have made a
>  > difference, would it?
>  > You'd have to locally set `temp-buffer-resize-mode' to nil in all cases
>  > when you don't want the window size to be restored afterwards, which is
>  > the same amount of work as clearing 'quit-window parameter.
>
> Why?  I set the buffer-local value only when `temp-buffer-resize-mode'
> is off.  When it's on I don't assign a buffer-local value.  The
> re-resize code in `quit-window' would trigger when either the local or
> the global value is t.

There are cases when we don't want it to be triggered, right? (See 
example at the top of this email). And when `temp-buffer-resize-mode' is 
t locally or globally, re-resizing code will always be triggered.

Hence, the `temp-buffer-resize-mode' check in `quit-window' does not 
really serve the purpose you ascribe to it. Or doesn't serve it well enough.

So I think the first thing to do is replace that check with (integerp 
...), like I suggested, and consider the question of when not to restore 
window height a separate issue (maybe file a separate bug, maybe not), 
because the issue is already there when `temp-buffer-resize-mode' is on.

> Anyway, this would only handle the re-resizing when quitting the vc-diff
> buffer.  It would not handle the case where people don't want any
> resizing.  Maybe vc-diff should shrink iff `temp-buffer-resize-mode' is
> on.

Maybe so. I think this is also a separate issue, but it's closely 
related to identifying the set of functions after which we don't restore 
window sizes, because just as we might want not to restore the height 
after `adjust-window-trailing-edge' was called, or 
`shrink-buffer-if-...' was called interactively, we similarly might want 
to resize the windows *only* when one of those functions is called (and 
only when interactively, in `shrink-buffer-if...' case).

>  >>  > Would a window that displayed a normal buffer previously but
> which now
>  >>  > is displaying a temporary buffer be considered a "window of
>  >>  > non-temporary buffer"?
>  >>
>  >> Only after it displays the normal buffer again.  Why did you ask?
>  >
>  > Because that's what at issue here. I think the main two cases of
>  > automatic shrinking are:
>  > 1) "temp buffer" in a window that was created for it,
>  > 2) "temp buffer" in a reused window,
>  > and 2) is the reason I filed this bug.
>
> Remember that the vc-diff buffer is not a temporary buffer.  Currently,

Yes, hence the quotes around "temp buffer".

> in order to trigger automatic resizing of temporary buffer windows you
> have to use `with-output-to-temp-buffer'.  If vc-diff had used that
> macro, the entire resizing issue would have been handled already.

It wouldn't, because `temp-buffer-resize-mode' is off by default. Or 
even if it were on by default, just because it could be switched on and 
off, the logic there can't depend on it.

> Another way is to explicitly call `resize-temp-buffer-window' and set
> `temp-buffer-resize-mode' t in that buffer.

I think that's the only way that restoring `vc-diff' window height could 
have worked with current `quit-window' code.
But like I said above (see the top of this email), it fixes one problem 
by introducing another.