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

From: Dmitry Gutov <[email-masked]>
Date: Tue, 03 Jul 2012 00:39:17 +0400

On 02.07.2012 20:32, martin rudalics wrote:
>  > Note that if the widow does get split, `vc-diff' behaves okay, because
>  > `q' deletes the used window, and the configuration becomes as it was (at
>  > least with `window-combination-resize' nil).
>
> Not necessarily: When you have two windows one above the other and
> `display-buffer' splits the upper one (thresholds permitting) and then
> `vc-diff' temporarily resizes the middle one, it steals space from the
> lower window.  When quitting the middle window, that space is returned
> to the upper one.

I guess that's true, my height threshold is just too high for that.
A bit inconsistent, though, isn't it? Stealing space from the lower, 
returning to the upper.

>  >> (1) When `temp-buffer-resize-mode' is non-nil and the window size has
>  >>      changed, chances are that the change was caused by
>  >>      `temp-buffer-resize-mode' so it seems pretty safe to rescind that
>  >>      change.
>  >
>  > I don't think so. Like I said, the value saved in 'quit-restore
>  > parameter is the same in this case, whether `temp-buffer-resize-mode' is
>  > on or not.
>  > So even if `temp-buffer-resize-mode' is on, we can't confidently decide
>  > that the value was set by it.
>
> Sure, but ...
>
>  >> (2) When `temp-buffer-resize-mode' is nil and the window size has
>  >>      changed, the change was probably due to some manual intervention
>  >>      probably needed to see more of a buffer originally present and it
>  >>      seems better to leave that change alone.
>
> ... in this case we can be sure that the user changed the size so we
> probably should leave it alone.

That depends on the definition of "the user". In our case, *I* didn't 
explicitly resize the window, `vc-diff' did.

>  > Would it be harder to *not set* the 'quit-restore parameter in cases
>  > when we don't want the configuration restored, instead?
>
> The whole idea of quitting is to get as much as possible of the state
> that existed before some temporal change without, however, rescinding
> changes introduced by user.

How about clearing (or changing) the 'quit-window parameter in each 
command when we're sure that the user won't want to have the size 
restored afterwards?
There would be a set of "user-facing" functions that would do that.

>  > But as it is, I'm not sure what's the problem with always using its
>  > value when present. I've been running Emacs with the tiny patch I posted
>  > for a couple of days, and it seems fine.
>  >
>  > Could you describe the scenario with "seeing more of a buffer originally
>  > present"?
>
> When watching diffs I usually very soon want to concentrate on the
> modified file and its changes.  For that reason, I sometimes want to
> enlarge its window and not have `quit-window' shrink it to what it was
> before invoking vc-diff.  But this use case might be sufficiently exotic
> to dismiss it.

How will the temp-buffer-resize-mode null check help in this case?
It's a global minor mode, so it's either t in all buffers, or nil in all 
of them.

>  >>  > Also, (nth 3 quad) is integer at that point even
>  >> temp-buffer-resize-mode
>  >>  > is disabled, so the mode check doesn't make sense.
>  >>  > To be safe, though, we could replace it with `integerp' call.
>  >>
>  >> Yes, but the `condition-case' should handle any problems with it.
>  >
>  > Don't we want to execute the code following (when resize ...) either
> way?
>
> Don't we?  How would the (when resize ...) check affect the rest?  And
> keep in mind that any resize operation has to take into account that the
> frame configuration or size has changed in the meantime.

Eh, I meant the comparison will blow up, it's above the condition-case:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p nil)
   /=(nil 42)
   eval((/= nil 42) nil)

>  > I'm fine with either behavior (not resizing or properly restoring), but
>  > `vc-diff' is not the only culprit. `vc-log-print' also does the
>  > shrinking, although it's harder to observe.
>
> What and where is `vc-log-print'?

Sorry, `vc-print-log'. C-x v l.

>  > If there will be a variable, I don't see why it should be local to VC.
>  > Are there users who would want windows shrunk by VC, but not other
>  > packages, or vice versa?
>
> I don't know.  I think a vc-diff buffer should be considerded a
> temporary buffer, subject to `temp-buffer-resize-mode'.  Ideally,
> windows of non-temporary buffers are never shrunk automatically.

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

>  >> (2) provide yet another variable which, when set, causes `quit-window'
>  >>      to re-resize the window (we could misuse `even-window-heights' for
>  >>      this purpose), or
>  >
>  > That won't do if the original windows sizes were not even, like in the
>  > SO question linked to previously.
>
> I miss you here.  The information is there (otherwise your patch
> couldn't use it) and I'd just use it in the additional case where
> `even-window-heights' is set.

I misunderstood. Re-resizing to the original height would work, of course.