[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: Wed, 04 Jul 2012 20:12:57 +0400
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1

On 04.07.2012 13:18, martin rudalics wrote:
 >  > Sure.  But as I proposed earlier we could have handled this by
 >  > `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.

I think it's good if all windows of this kind behave similarly.

 >> `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 ;-)

I tried it with just my patch applied, and it didn't work, because in this case the stored height value was of already resized window: `display-buffer-record-window' is called from `window--display-buffer', and `display-buffer-use-some-window' calls `window--even-window-heights' before `window--display-buffer'.

Maybe that's fine, I'll just set the variable to nil.

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

No problem, let's wait and see if/when someone complains.

Until then, the issue is somewhat alleviated by the fact that you can press 'z' or 'C-x k' instead of 'q', and both of these won't trigger height restoration.

 >> Anyway, this would only handle the re-resizing when quitting the
 >> 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'?

The idea was to disable resizing at a lower level. For example, let's call the new variable `dont-resize'. If it's set to t, then `window--resize-this-window' won't do anything. You'd rebind `dont-resize' to nil locally in select cases: in `adjust-window-trailing-edge', when `shrink-buffer-if-...' is called interactively, etc. That's the rough outline. If someone wants `shrink-window-if-...' to have no effect only in `vc-diff', well, that's a different goal.

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

I see.

 > 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

I think it's a little backwards: if a user doesn't enable `temp-buffer-resize-mode' or `even-window-heights', then they probably care about keeping their window configuration the same over time. If some command like `vc-diff' still can resize a window automatically, we should strive to keep that action undo-able.

There's a positive side to that suggestion (if the user sets up the preferred configuration after the temp buffer is displayed), but that doesn't outweigh the drawback.

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

Both 1 and 2 would be fine, I think, although 1 would require some extra work to make it work asynchronously. IIUC, `with-output-to-temp-buffer' waits until the called process completes, and `vc-diff' supports both synchronous and asynchronous execution.

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

-- Dmitry

reply via email to

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