[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Dmitry Gutov
Subject: bug#11810: 24.1.50; `vc-diff' shrinks pre-existing window
Date: Tue, 03 Jul 2012 22:56:14 +0400
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1

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

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.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]