GNU bug report logs - #11810
24.1.50; `vc-diff' shrinks pre-existing window

Previous Next

Full log

Message #59 received at 11810 <at> debbugs.gnu.org (full text, mbox):

From: martin rudalics <rudalics <at> gmx.at>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 11810 <at> debbugs.gnu.org
Subject: Re: bug#11810: 24.1.50; `vc-diff' shrinks pre-existing window
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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.