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

From: martin rudalics <[email-masked]>
Date: Wed, 04 Jul 2012 11:18:36 +0200

>  > 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,

... but only for vc-diff buffers ...

 > like you said
 > it shouldn't.

... do that for arbitrary buffers.  If we remove the corresponding check
in `quit-window', all windows that can be quit are affected.

 >> `even-window-heights' is customizable so it's not a primary issue.  But
 >> we could undo the evening in `quit-window'.
 >
 > I agree.

We'll do so when we apply your last patch ;-)

 >> 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.

It's tedious and I wouldn't like to do it.

 > Then yes, both of them, I guess.
 > `enlarge-window' and `enlarge-window-horizontally' could be another
 > candidates.

Yes.

 > Not sure about `delete-window' (when we're deleting one
 > window in a configuration), could be either way.

No (we'd have to care about its clients like `quit-window' too then).

 > 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.

My implicit assumption was that people using `temp-buffer-resize-mode'
want automatic changes like that in vc-diff rescinded when done with the
buffer.

 > 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.

Because vc-diff does something special here.

 > 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.

OK, let's do that.  People had enough time to express their concerns
about it.  If problems come up, we'll hear about them soon enough.

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

`vc-diff' unconditionally resizes the window.  What if someone simply
doesn't want its `shrink-window-if-larger-than-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.

That's what I meant: In that case there would not have been any resizing
unless triggered by `even-window-heights'.

 > 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.

We could make the value at the time the window was reused prevail via
the quit-restore parameter: That is, save the total size of the window
only if either `even-window-heights' or `temp-buffer-resize-mode' are
on.

 >> 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.

... in a limited way, though.

Let's close this thread as follows: Remove the `temp-buffer-resize-mode'
check in `quit-window' and add an integerp check for the third element.
If this has any bad consequences, we can still

(1) have `vc-diff' use something similar to `with-output-to-temp-buffer'
     so that `temp-buffer-resize-mode' is honored,

(2) resize the window only if `temp-buffer-resize-mode' is enabled, or

(3) resize the window unconditionally as now but locally set the
     variable `temp-buffer-resize-mode' to t in the diff buffer.

All these approaches would re-resize the window on quitting provided
`even-window-heights' is nil.

Independently from that we can provide an extra vc-prefixed variable
which controls whether the diff window shall be resiized in the first
place.  As you said, that would be a different bug.

More generally, we could provide an option specifying a function or
regexp for all buffers whose windows should be fit to them and have
`fit-window-to-buffer' (unless called interactively) honor that.  That
option would then override `temp-buffer-resize-mode' as well.

martin