shrink-window-if-larger-than-buffer in VC-diff

shrink-window-if-larger-than-buffer in VC-diff

[Chong Yidong]

When there are no changes, `C-x v D' does something annoying: it shrinks
the window showing the *vc-diff* buffer to three lines tall.  This makes
the window useless for displaying other buffers; you have to resize it
or delete it.

I think the shrink-window-if-larger-than-buffer call in vc-diff-finish
shouldn't be there in the first place.  It's well-intentioned, but even
in cases less extreme that the above, it can be annoying to resize the
user's windows.  Any objection to removing this call?

Re: shrink-window-if-larger-than-buffer in VC-diff

[Miles Bader]

Chong Yidong <cyd@stupidchicken.com> writes:
> I think the shrink-window-if-larger-than-buffer call in vc-diff-finish
> shouldn't be there in the first place.  It's well-intentioned, but even
> in cases less extreme that the above, it can be annoying to resize the
> user's windows.  Any objection to removing this call?

Isn't this same objection also applicable to _all_ uses of
shrink-window-if-larger-than-buffer?

[Sometimes I like the effect of such shrinking, sometimes I'm annoyed by
it, but I don't see a really good way to decide between these cases
mechanically...  well except that obviously for "transient" windows --
e.g. completion from the minibuffer -- shrinking is almost always good.]

-Miles

-- 
The car has become... an article of dress without which we feel uncertain,
unclad, and incomplete.  [Marshall McLuhan, Understanding Media, 1964]

Re: shrink-window-if-larger-than-buffer in VC-diff

[Andreas Schwab]

Chong Yidong <cyd@stupidchicken.com> writes:

> I think the shrink-window-if-larger-than-buffer call in vc-diff-finish
> shouldn't be there in the first place.  It's well-intentioned, but even
> in cases less extreme that the above, it can be annoying to resize the
> user's windows.  Any objection to removing this call?

At least it should be made optional.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

annoyances [Was: shrink-window-if-larger-than-buffer in VC-diff]

[Uday S Reddy]

On 8/15/2010 12:15 AM, Chong Yidong wrote:
> When there are no changes, `C-x v D' does something annoying: it shrinks
> the window showing the *vc-diff* buffer to three lines tall.  This makes
> the window useless for displaying other buffers; you have to resize it
> or delete it.

I find it annoying too, and I will be glad if it is gone.

While we are on the matter of annoyances, I am also quite annoyed by Emacs 
switching windows without a definitive reason.

For instance, `o' in a dired window switches to the other window.  And, I have 
to switch back to dired in order to go and view another file.

C-x` in a diff window switches to the other window.  And, I have to switch back 
to diff in order to even view the hunk in its entirety.  (This problem might 
also be shared by all other uses of C-x`)

In general, I think that, whenever there is a listing in one window and stuff 
in another window, it is a good idea for Emacs to assume that both are 
important and avoid switching windows unnecessarily.

Cheers,
Uday

Re: annoyances

[David Kastrup]

Uday S Reddy <u.s.reddy@cs.bham.ac.uk> writes:

> On 8/15/2010 12:15 AM, Chong Yidong wrote:
>> When there are no changes, `C-x v D' does something annoying: it shrinks
>> the window showing the *vc-diff* buffer to three lines tall.  This makes
>> the window useless for displaying other buffers; you have to resize it
>> or delete it.
>
> I find it annoying too, and I will be glad if it is gone.
>
> While we are on the matter of annoyances, I am also quite annoyed by
> Emacs switching windows without a definitive reason.
>
> For instance, `o' in a dired window switches to the other window.
> And, I have to switch back to dired in order to go and view another
> file.

Well, don't use `o' then.  Duh.  That's what C-o is for.

Unfortunately, C-o has the side effect of loading the file into a
buffer.  And the buffer then stays around.  It would be nice to have
dired record when a buffer only becomes populated by C-o and kill it
when the window gets deleted or reassociated without the buffer ever
becoming the current buffer.

`o' currently has the advantage that you can move back into dired using
C-x 4 0 (why doesn't C-x 4 k work?  That binding is quite more mnemonic)
and get rid of the buffer that way.

-- 
David Kastrup

Re: annoyances

[Uday S Reddy]

On 8/15/2010 11:06 AM, David Kastrup wrote:

> Well, don't use `o' then.  Duh.  That's what C-o is for.

Ok, I either didn't know about C-o or forgot all about it.  I will go rebind it 
to `o' before I forget again.

But, I believe that this is a more widespread problem than dired.

> Unfortunately, C-o has the side effect of loading the file into a
> buffer.  And the buffer then stays around.  It would be nice to have
> dired record when a buffer only becomes populated by C-o and kill it
> when the window gets deleted or reassociated without the buffer ever
> becoming the current buffer.
>
> `o' currently has the advantage that you can move back into dired using
> C-x 4 0 (why doesn't C-x 4 k work?  That binding is quite more mnemonic)
> and get rid of the buffer that way.

Yes, good ideas indeed.

Cheers,
Uday

annoyances [Was: shrink-window-if-larger-than-buffer in VC-diff]

[Stephen J. Turnbull]

Uday S Reddy writes:

 > C-x` in a diff window switches to the other window.  And, I have to
 > switch back to diff in order to even view the hunk in its entirety.
 > (This problem might also be shared by all other uses of C-x`)

Doesn't C-M-v (scroll-other-window) mostly alleviate this problem?

Re: annoyances [Was: shrink-window-if-larger-than-buffer in VC-diff]

[Uday S Reddy]

On 8/15/2010 3:12 PM, Stephen J. Turnbull wrote:
> Uday S Reddy writes:
>
>   >  C-x` in a diff window switches to the other window.  And, I have to
>   >  switch back to diff in order to even view the hunk in its entirety.
>   >  (This problem might also be shared by all other uses of C-x`)
>
> Doesn't C-M-v (scroll-other-window) mostly alleviate this problem?

Thanks.  It helps.  (I normally avoid such complex key combinations because of 
my RSI history.  But I found that there are nicer key bindings M-<next> and 
M-<prior> which will suit me.)

But the more general point is about Emacs taking liberties with switching 
windows.  I suppose you understand that.

Cheers,
Uday

Re: annoyances [Was: shrink-window-if-larger-than-buffer in VC-diff]

[Stephen J. Turnbull]

Uday S Reddy writes:

 > But the more general point is about Emacs taking liberties with
 > switching windows.  I suppose you understand that.

I understand, but don't necessarily sympathize.  For example, with
find-file, it's pretty clear that automatically switching windows is a
good thing.  Maybe in the case of C-x ` (is that `next-error' in your
context, as it is for me?) it should be optional.  I admit I'm an
infrequent user of next-error, but I think it's quite natural to
switch windows since it's documented as visiting the error buffer and
the corresponding source file.  This is almost always what I want.  If
you don't like that behavior, you need to write your own function that
implements the workflow you want.  As usual.[1]

It's true that many third-party modes do things like switching windows
that I find really annoying.  However, most of that has long since
been beaten out of modes and functions that are part of core Emacs.

Footnotes: 
[1]  If the basic functionally you need is all hard-coded into
next-error, making this an annoying process of copying many large
sections of code, I would consider that a design bug.  But if
next-error is well-factored into an error parser, a source finder, and
a window popper-upper, it should be easy to write your alternative
workflow, perhaps with some minor refactoring.

Re: annoyances [Was: shrink-window-if-larger-than-buffer in VC-diff]

[Uday S Reddy]

On 8/16/2010 2:45 AM, Stephen J. Turnbull wrote:

> I understand, but don't necessarily sympathize.  For example, with
> find-file, it's pretty clear that automatically switching windows is a
> good thing.

As you can tell, it is not "clear" to me that it is a good thing.  When the 
user is displaying two windows, Emacs doesn't really know which window the user 
wants to be in.  The best thing to do would be to make no assumption.  The 
documentation of dired-find-file-other-window doesn't say that it is going to 
switch windows.  You might say that that is included in the semantics of 
"find-file".  But, then, would the semantics of "next-error" include putting 
focus on the error message?

These switching actions are an example of over-enthusiasm on the part of Emacs, 
just as s-w-i-l-t-b is.  Whoever wrote these functions was thinking of one 
particular workflow and forgetting that less can often be more.

   Maybe in the case of C-x ` (is that `next-error' in your
> context, as it is for me?) it should be optional.  I admit I'm an
> infrequent user of next-error, but I think it's quite natural to
> switch windows since it's documented as visiting the error buffer and
> the corresponding source file.  This is almost always what I want.  If
> you don't like that behavior, you need to write your own function that
> implements the workflow you want.  As usual.[1]

I suppose you are right.  I am a bit spoilt by VM, which seems to know how to 
do just the right thing in terms of the user interface.  One of the ideas I 
have is to write a generic "browser" mode, using VM ideas, which can be 
inherited by all modes like dired, diff, buffer-menu, VM, Gnus etc. and provide 
a uniform user interface to all of them.  This should also address some of the 
key binding issues we were talking about in the other thread.  I don't yet know 
when I will get time to do it.

Cheers,
Uday

Re: shrink-window-if-larger-than-buffer in VC-diff

[Juanma Barranquero]

On Sun, Aug 15, 2010 at 01:15, Chong Yidong <cyd@stupidchicken.com> wrote:

> I think the shrink-window-if-larger-than-buffer call in vc-diff-finish
> shouldn't be there in the first place.  It's well-intentioned, but even
> in cases less extreme that the above, it can be annoying to resize the
> user's windows.  Any objection to removing this call?

IMHO, such cases should be made optional (perhaps a global variable
covering such uses in many/most/all modes?) instead of removed. I
usually *always* want the shrinking behavior, and rarely reuse the
window.

    Juanma

Re: shrink-window-if-larger-than-buffer in VC-diff

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

>> I think the shrink-window-if-larger-than-buffer call in vc-diff-finish
>> shouldn't be there in the first place.  It's well-intentioned, but even
>> in cases less extreme that the above, it can be annoying to resize the
>> user's windows.  Any objection to removing this call?
>
> IMHO, such cases should be made optional (perhaps a global variable
> covering such uses in many/most/all modes?) instead of removed. I
> usually *always* want the shrinking behavior, and rarely reuse the
> window.

As far as I can tell, the only really good uses of s-w-i-l-t-b occur
when the window in question is electric.  For permanent windows, the
behavior seems poor.  Here is another example of it misbehaving, in
finder:

1. M-x finder-by-keyword RET
2. C-x 1
3. Go to "tex" and type RET
4. Go to "tex-mode.el" and RET

   => The upper window, now showing *Finder-package* with an empty
   commentary, is 4 lines tall (due to s-w-i-l-t-b).

5. C-x o
6. Go to "reftex.el" and RET

   => The upper window now shows *Finder-package* with a long verbose
   commentary.  But it is still 4 lines tall, and therefore useless.


I think the existing non-electric uses of s-w-i-l-t-b need to be
reconsidered.  Maybe we should change `pop-to-buffer' so that it accepts
an option to both shrink *and* grow windows.  Then people who want their
windows to resize automagically can use this option, and have it work
more reliably than it does now.  (The default should be to avoid
resizing at all.)

Re: shrink-window-if-larger-than-buffer in VC-diff

[Juanma Barranquero]

On Mon, Aug 16, 2010 at 05:10, Chong Yidong <cyd@stupidchicken.com> wrote:

> As far as I can tell, the only really good uses of s-w-i-l-t-b occur
> when the window in question is electric.

Well, yes. I tend to use electric windows as much as possible.

> Maybe we should change `pop-to-buffer' so that it accepts
> an option to both shrink *and* grow windows.  Then people who want their
> windows to resize automagically can use this option, and have it work
> more reliably than it does now.  (The default should be to avoid
> resizing at all.)

I'm OK with that.

    Juanma

Re: shrink-window-if-larger-than-buffer in VC-diff

[Chong Yidong]

Juanma Barranquero <lekktu@gmail.com> writes:

>> Maybe we should change `pop-to-buffer' so that it accepts
>> an option to both shrink *and* grow windows.  Then people who want their
>> windows to resize automagically can use this option, and have it work
>> more reliably than it does now.  (The default should be to avoid
>> resizing at all.)
>
> I'm OK with that.

Here's a possible implementation, which replaces even-window-heights
with a new option `resize-windows-for-display'.  The value `fit' says to
resize the displayed/popped windows.  What do you think?


=== modified file 'lisp/window.el'
*** lisp/window.el	2010-06-07 18:28:02 +0000
--- lisp/window.el	2010-08-18 21:46:44 +0000
***************
*** 989,1025 ****
                         ))
  	frame))))
  
! (defcustom even-window-heights t
!   "If non-nil `display-buffer' will try to even window heights.
! Otherwise `display-buffer' will leave the window configuration
! alone.  Heights are evened only when `display-buffer' chooses a
! window that appears above or below the selected window."
!   :type 'boolean
!   :group 'windows)
  
! (defun window--even-window-heights (window)
!   "Even heights of WINDOW and selected window.
! Do this only if these windows are vertically adjacent to each
! other, `even-window-heights' is non-nil, and the selected window
! is higher than WINDOW."
!   (when (and even-window-heights
! 	     (not (eq window (selected-window)))
! 	     ;; Don't resize minibuffer windows.
! 	     (not (window-minibuffer-p (selected-window)))
! 	     (> (window-height (selected-window)) (window-height window))
! 	     (eq (window-frame window) (window-frame (selected-window)))
! 	     (let ((sel-edges (window-edges (selected-window)))
! 		   (win-edges (window-edges window)))
! 	       (and (= (nth 0 sel-edges) (nth 0 win-edges))
! 		    (= (nth 2 sel-edges) (nth 2 win-edges))
! 		    (or (= (nth 1 sel-edges) (nth 3 win-edges))
! 			(= (nth 3 sel-edges) (nth 1 win-edges))))))
!     (let ((window-min-height 1))
!       ;; Don't throw an error if we can't even window heights for
!       ;; whatever reason.
!       (condition-case nil
! 	  (enlarge-window (/ (- (window-height window) (window-height)) 2))
! 	(error nil)))))
  
  (defun window--display-buffer-1 (window)
    "Raise the frame containing WINDOW.
--- 989,1030 ----
                         ))
  	frame))))
  
! (defcustom resize-windows-for-display t
!   "Whether `display-buffer' should resize windows.
! If nil, leave the window configuration alone.
! If `fit', try to shrink the chosen window if its buffer does not
! need so many lines; otherwise, try to even the window heights.
! For any other non-nil value, try to even the window heights if
! the chosen window appears above or below the selected window."
!   :type '(choice (const :tag "Equalize window heights" t)
!                  (const :tag "Best fit" 'fit)
!                  (const :tag "No resize" nil))
!   :group 'windows
!   :version "24.1")
  
! (define-obsolete-variable-alias 'even-window-heights 'resize-windows-for-display)
! 
! (defun window--adjust-window-heights (window)
!   "Adjust height of WINDOW according to `resize-windows-for-display'."
!   ;; Try shrinking window if `resize-windows-for-display' is `fit'.
!   (and resize-windows-for-display
!        (not (and (eq resize-windows-for-display 'fit)
! 		 (shrink-window-if-larger-than-buffer window)))
!        (not (eq window (selected-window)))
!        (not (window-minibuffer-p (selected-window)))
!        (eq (window-frame window) (window-frame (selected-window)))
!        (let ((sel-edges (window-edges (selected-window)))
! 	     (win-edges (window-edges window)))
! 	 (and (= (nth 0 sel-edges) (nth 0 win-edges))
! 	      (= (nth 2 sel-edges) (nth 2 win-edges))
! 	      (or (= (nth 1 sel-edges) (nth 3 win-edges))
! 		  (= (nth 3 sel-edges) (nth 1 win-edges)))))
!        (let ((window-min-height 1))
! 	 ;; Don't throw an error if we can't even window heights for
! 	 ;; whatever reason.
! 	 (condition-case nil
! 	     (enlarge-window (/ (- (window-height window) (window-height)) 2))
! 	   (error nil)))))
  
  (defun window--display-buffer-1 (window)
    "Raise the frame containing WINDOW.
***************
*** 1036,1047 ****
        (raise-frame frame))
      window))
  
! (defun window--display-buffer-2 (buffer window &optional dedicated)
    "Display BUFFER in WINDOW and make its frame visible.
! Set `window-dedicated-p' to DEDICATED if non-nil.
  Return WINDOW."
    (when (and (buffer-live-p buffer) (window-live-p window))
      (set-window-buffer window buffer)
      (when dedicated
        (set-window-dedicated-p window dedicated))
      (window--display-buffer-1 window)))
--- 1041,1056 ----
        (raise-frame frame))
      window))
  
! (defun window--display-buffer-2 (buffer window &optional dedicated adjust-size)
    "Display BUFFER in WINDOW and make its frame visible.
! If DEDICATED is non-nil, set `window-dedicated-p'.
! If ADJUST-SIZE is non-nil, resize WINDOW according to
! `resize-windows-for-display'.
  Return WINDOW."
    (when (and (buffer-live-p buffer) (window-live-p window))
      (set-window-buffer window buffer)
+     (when adjust-size
+       (window--adjust-window-heights window))
      (when dedicated
        (set-window-dedicated-p window dedicated))
      (window--display-buffer-1 window)))
***************
*** 1160,1166 ****
  		     (window--try-to-split-window
  		      (get-lru-window frame-to-use t)))))
        (window--display-buffer-2 buffer window-to-use
!                                 display-buffer-mark-dedicated))
       ((let ((window-to-undedicate
  	     ;; When NOT-THIS-WINDOW is non-nil, temporarily dedicate
  	     ;; the selected window to its buffer, to avoid that some of
--- 1169,1175 ----
  		     (window--try-to-split-window
  		      (get-lru-window frame-to-use t)))))
        (window--display-buffer-2 buffer window-to-use
!                                 display-buffer-mark-dedicated t))
       ((let ((window-to-undedicate
  	     ;; When NOT-THIS-WINDOW is non-nil, temporarily dedicate
  	     ;; the selected window to its buffer, to avoid that some of
***************
*** 1186,1193 ****
  	  (when (window-live-p window-to-undedicate)
  	    ;; Restore dedicated status of selected window.
  	    (set-window-dedicated-p window-to-undedicate nil))))
!       (window--even-window-heights window-to-use)
!       (window--display-buffer-2 buffer window-to-use)))))
  
  (defun pop-to-buffer (buffer-or-name &optional other-window norecord)
    "Select buffer BUFFER-OR-NAME in some window, preferably a different one.
--- 1195,1201 ----
  	  (when (window-live-p window-to-undedicate)
  	    ;; Restore dedicated status of selected window.
  	    (set-window-dedicated-p window-to-undedicate nil))))
!       (window--display-buffer-2 buffer window-to-use nil t)))))
  
  (defun pop-to-buffer (buffer-or-name &optional other-window norecord)
    "Select buffer BUFFER-OR-NAME in some window, preferably a different one.

*** lisp/vc/vc.el	2010-07-16 15:42:15 +0000
--- lisp/vc/vc.el	2010-08-18 21:50:15 +0000
***************
*** 1511,1517 ****
                 (message "%s" (cdr messages))))
          (goto-char (point-min))
          (when window
!           (shrink-window-if-larger-than-buffer window)))
        (when (and messages (not emptyp))
          (message "%sdone" (car messages))))))
  
--- 1511,1517 ----
                 (message "%s" (cdr messages))))
          (goto-char (point-min))
          (when window
! 	  (window--adjust-window-heights window)))
        (when (and messages (not emptyp))
          (message "%sdone" (car messages))))))
  
***************
*** 1583,1589 ****
        (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
                                                              messages)))
        ;; Display the buffer, but at the end because it can change point.
!       (pop-to-buffer (current-buffer))
        ;; In the async case, we return t even if there are no differences
        ;; because we don't know that yet.
        t)))
--- 1583,1590 ----
        (vc-exec-after `(vc-diff-finish ,(current-buffer) ',(when verbose
                                                              messages)))
        ;; Display the buffer, but at the end because it can change point.
!       (let ((resize-windows-for-display nil))
! 	(pop-to-buffer (current-buffer)))
        ;; In the async case, we return t even if there are no differences
        ;; because we don't know that yet.
        t)))

Re: shrink-window-if-larger-than-buffer in VC-diff

[martin rudalics]

 > Here's a possible implementation, which replaces even-window-heights
 > with a new option `resize-windows-for-display'.  The value `fit' says to
 > resize the displayed/popped windows.  What do you think?

How is this option supposed to interact with `temp-buffer-resize-mode'?
The latter, if enabled, will probably always override the option so such
interaction should be somehow described in the doc-string(s).

But I'm not quite sure whether using two similar approaches for the same
problem is a good idea in the first place.  I'd call functions like
`fit-window-to-buffer' and `shrink-window-if-larger-than-buffer' only
internally.  The user interface should be provided by one mode or buffer
local variable which also makes sure that any window displaying such a
buffer gets adjusted when other windows or the frame change size.

 > !       (let ((resize-windows-for-display nil))
 > ! 	(pop-to-buffer (current-buffer)))

Here you explicitly override the user option - is that intentional?

martin

Re: shrink-window-if-larger-than-buffer in VC-diff

[martin rudalics]

 > Maybe we should change `pop-to-buffer' so that it accepts
 > an option to both shrink *and* grow windows.  Then people who want their
 > windows to resize automagically can use this option, and have it work
 > more reliably than it does now.

This is now possible using the `window-height' and `window-width' alist
entries.  There are currently two major problems with this:

(1) `temp-buffer-resize-mode' overrides it.  This is no problem for
`with-output-to-temp-buffer' since that macro encapsulates the
`display-buffer' call.  But `with-temp-buffer-window' has an ACTION
argument which can be now overridden and manually activating
`temp-buffer-resize-mode' in a buffer will accomplish the same.

(2) I'm not sure what to do with `fit-window-to-buffer' and
`shrink-window-if-larger-than-buffer' calls that happen after
`display-buffer' like in `vc-root-diff'.  In principle, I could have
`display-buffer' add a window parameter or a buffer-local variable to
handle such cases.  But neither of them look very attractive to me.

Any suggestions?

 > (The default should be to avoid
 > resizing at all.)

I disagree here.  We should try to preserve the previous behavior.

martin