bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[Stephen Berman]

[-- Attachment #1: Type: text/plain, Size: 1896 bytes --]

In GNU Emacs 23.0.60.21 (i686-pc-linux-gnu, GTK+ Version 2.12.9)
 of 2008-12-03 on escher

1. emacs -Q

2. Eval this:
   (define-minor-mode my-mm ()
     "My minor mode."
     :global t
     (if my-mm
         (setq default-header-line-format
   	       '(:eval (propertize "test" 'face '(:overline t))))
       (setq default-header-line-format nil)))

3. Enable my-mm.

4. C-x d foo (where `foo' is some directory with at least three files,
   e.g. emacs/lisp).

5. Mark three files in foo.

6. Type `C'
   ==> The popped up buffer *Marked Files* shows only two of the three
   marked files.

Likewise if the value of the 'face property in my-mm has a non-nil :box
attribute.  In contrast, if the value of the 'face property is
'(:underline t) (but not '(:underline t :overline t)), then in step 6
all three marked files are shown, though with no room to spare.

I guess the basic problem here is that Emacs calculates window-height in
integral equally sized line numbers.  But less fundamentally and more
directly, this problem arises, AFAICT, because dired-pop-to-buffer's
shrink-to-fit code fails to account for header lines.  To fix this it
would suffice to add this sexp above the last sexp in
dired-pop-to-buffer: 
    (with-current-buffer buf 
      (and header-line-format
           (progn (select-window w2) (enlarge-window 1))))

However, unless I'm missing something, fit-window-to-buffer does what
dired-pop-to-buffer's shrink-to-fit code does, and moreover also handles
header lines.  From perusing the change logs, I found several instances
where fit-window-to-buffer replaces library-specific code, and this
appears to be another candidate (dired-pop-to-buffer was introduced many
years before fit-window-to-buffer), in addition to fixing the above bug.
I'm not able to fix the display code to let window-height return
fractional line numbers, so I propose the following patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: dired-pop-to-buffer patch --]
[-- Type: text/x-patch, Size: 2250 bytes --]

*** emacs/lisp/dired.el.~1.414.~	2008-12-03 10:17:51.000000000 +0100
--- emacs/lisp/dired.el			2008-12-04 10:11:05.000000000 +0100
***************
*** 2678,2721 ****
  
  (defun dired-pop-to-buffer (buf)
    ;; Pop up buffer BUF.
    ;; If dired-shrink-to-fit is t, make its window fit its contents.
!   (if (not dired-shrink-to-fit)
!       (pop-to-buffer (get-buffer-create buf))
!     ;; let window shrink to fit:
!     (let ((window (selected-window))
! 	  target-lines w2)
!       (cond ;; if split-height-threshold is enabled, use the largest window
!             ((and (> (window-height (setq w2 (get-largest-window)))
! 		     split-height-threshold)
! 		  (window-full-width-p w2))
! 	     (setq window w2))
! 	    ;; if the least-recently-used window is big enough, use it
! 	    ((and (> (window-height (setq w2 (get-lru-window)))
! 		     (* 2 window-min-height))
! 		  (window-full-width-p w2))
! 	     (setq window w2)))
!       (save-excursion
! 	(set-buffer buf)
! 	(goto-char (point-max))
! 	(skip-chars-backward "\n\r\t ")
! 	(setq target-lines (count-lines (point-min) (point)))
! 	;; Don't forget to count the last line.
! 	(if (not (bolp))
! 	    (setq target-lines (1+ target-lines))))
!       (if (<= (window-height window) (* 2 window-min-height))
! 	  ;; At this point, every window on the frame is too small to split.
! 	  (setq w2 (display-buffer buf))
! 	(setq w2 (split-window window
! 		  (max window-min-height
! 		       (- (window-height window)
! 			  (1+ (max window-min-height target-lines)))))))
!       (set-window-buffer w2 buf)
!       (if (< (1- (window-height w2)) target-lines)
! 	  (progn
! 	    (select-window w2)
! 	    (enlarge-window (- target-lines (1- (window-height w2))))))
!       (set-window-start w2 1)
!       )))
  
  (defcustom dired-no-confirm nil
    "A list of symbols for commands Dired should not confirm.
--- 2678,2687 ----
  
  (defun dired-pop-to-buffer (buf)
    ;; Pop up buffer BUF.
+   (pop-to-buffer (get-buffer-create buf))
    ;; If dired-shrink-to-fit is t, make its window fit its contents.
!   (when dired-shrink-to-fit
!     (fit-window-to-buffer (get-buffer-window buf))))
  
  (defcustom dired-no-confirm nil
    "A list of symbols for commands Dired should not confirm.

[-- Attachment #3: Type: text/plain, Size: 452 bytes --]


(Note that if the frame is shrunk so that not all marked file names fit
in the *Marked Files* window, then without an oversized header line as
is my-mm, dired-pop-to-buffer fails with the error "Attempt to delete
minibuffer or sole ordinary window"; whereas with an oversized header
line, this error is raised only if not even one file name can be shown.
But I see this difference in behavior whether or not the above patch is
applied.)

Steve Berman

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[Drew Adams]

> From: Stephen Berman Sent: Thursday, December 04, 2008 1:44 AM
> In GNU Emacs 23.0.60.21 (i686-pc-linux-gnu, GTK+ Version 2.12.9)
>  of 2008-12-03 on escher
> 1. emacs -Q
> 2. Eval this:
>    (define-minor-mode my-mm () "My minor mode." :global t
>      (if my-mm
>          (setq default-header-line-format
>    	       '(:eval (propertize "test" 'face '(:overline t))))
>        (setq default-header-line-format nil)))
> 3. Enable my-mm.
> 4. C-x d foo (where `foo' is some directory with at least three files,
>    e.g. emacs/lisp).
> 5. Mark three files in foo.
> 6. Type `C'
>    ==> The popped up buffer *Marked Files* shows only two of the three
>    marked files.
> 
> Likewise if the value of the 'face property in my-mm has a 
> non-nil :box attribute.  In contrast, if the value of the 'face
> property is '(:underline t) (but not '(:underline t :overline t)),
> then in step 6 all three marked files are shown, though with no room
> to spare.
> 
> I guess the basic problem here is that Emacs calculates 
> window-height in integral equally sized line numbers.

I think so; yes.

This seems related to this bug:
http://lists.gnu.org/archive/html/emacs-devel/2007-03/msg00765.html.

A face that uses :box, :overline, etc. is a pixel or two taller. And that can
cause code that deals with window size to increase by another line.

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

 > However, unless I'm missing something, fit-window-to-buffer does what
 > dired-pop-to-buffer's shrink-to-fit code does, and moreover also handles
 > header lines.

In addition we have the problem that `split-height-threshold' can be now
nil in

             ((and (> (window-height (setq w2 (get-largest-window)))
		     split-height-threshold)

 > I'm not able to fix the display code to let window-height return
 > fractional line numbers, so I propose the following patch:

If there are no objections, I'll commit that in a couple of days.

 > (Note that if the frame is shrunk so that not all marked file names fit
 > in the *Marked Files* window, then without an oversized header line as
 > is my-mm, dired-pop-to-buffer fails with the error "Attempt to delete
 > minibuffer or sole ordinary window"; whereas with an oversized header
 > line, this error is raised only if not even one file name can be shown.
 > But I see this difference in behavior whether or not the above patch is
 > applied.)

I cannot reproduce that here.  That is, marking in a dired buffer more
files than there fit on the frame and typing C gives me a single-window
frame here with some files not shown - no error.

martin

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

 > I cannot reproduce that here.  That is, marking in a dired buffer more
 > files than there fit on the frame and typing C gives me a single-window
 > frame here with some files not shown - no error.

Ah, I can reproduce that now - but not reliably.

I have to debug this - I even managed to get a frame with no modeline
either :-(

martin

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 226 bytes --]

Stephen,

`fit-window-to-buffer' seems broken in a number of regards.  I'll
eventually put condition-cases around the `enlarge-window' calls.
Could you, so far, try whether the attached patch improves things.

Thanks, martin.

[-- Attachment #2: window.el.diff --]
[-- Type: text/plain, Size: 1299 bytes --]

*** window.el.~1.169.~	2008-11-27 11:14:14.671875000 +0100
--- window.el	2008-12-05 15:22:20.281250000 +0100
***************
*** 1307,1313 ****
    (when (null window)
      (setq window (selected-window)))
    (when (null max-height)
!     (setq max-height (frame-height (window-frame window))))
  
    (let* ((buf
  	  ;; Buffer that is displayed in WINDOW
--- 1307,1313 ----
    (when (null window)
      (setq window (selected-window)))
    (when (null max-height)
!     (setq max-height (- (frame-height (window-frame window)) 1)))
  
    (let* ((buf
  	  ;; Buffer that is displayed in WINDOW
***************
*** 1334,1341 ****
  	 (delta
  	  ;; Calculate how much the window height has to change to show
  	  ;; desired-height lines, constrained by MIN-HEIGHT and MAX-HEIGHT.
! 	  (- (max (min desired-height max-height)
! 		  (or min-height window-min-height))
  	     window-height)))
  
      ;; Don't try to redisplay with the cursor at the end
--- 1334,1341 ----
  	 (delta
  	  ;; Calculate how much the window height has to change to show
  	  ;; desired-height lines, constrained by MIN-HEIGHT and MAX-HEIGHT.
! 	  (- (min max-height
! 		  (max desired-height (or min-height window-min-height)))
  	     window-height)))
  
      ;; Don't try to redisplay with the cursor at the end

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[Stephen Berman]

On Fri, 05 Dec 2008 15:25:04 +0100 martin rudalics <rudalics@gmx.at> wrote:

> `fit-window-to-buffer' seems broken in a number of regards.  I'll
> eventually put condition-cases around the `enlarge-window' calls.
> Could you, so far, try whether the attached patch improves things.

You're referring to the parenthetical note below my patch, right?  So I
tested my patched version of dired-pop-to-buffer after applying your
patch.  The result is that dired-pop-to-buffer does not raise an error
if I shrink the frame to a size too small to show all marked files,
irrespective of the presence or absence of a header line; in both cases
I can shrink the frame until only one file can be shown.  That is, your
patch makes the behavior of dired-pop-to-buffer (with my patch)
uniformly like the behavior with a header line without your patch.  Is
that what you intend?

Steve Berman

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

 > You're referring to the parenthetical note below my patch, right?

Yes.

 > So I
 > tested my patched version of dired-pop-to-buffer after applying your
 > patch.  The result is that dired-pop-to-buffer does not raise an error
 > if I shrink the frame to a size too small to show all marked files,
 > irrespective of the presence or absence of a header line; in both cases
 > I can shrink the frame until only one file can be shown.

Isn't that what you wanted?

 > That is, your
 > patch makes the behavior of dired-pop-to-buffer (with my patch)
 > uniformly like the behavior with a header line without your patch.  Is
 > that what you intend?

I don't understand what you mean here.  What I wanted to resolve in the
first place was to make `fit-window-to-buffer' not issue an error when
the frame is to small.  Is there anything else that doesn't work?

martin

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[Stephen Berman]

On Fri, 05 Dec 2008 18:37:22 +0100 martin rudalics <rudalics@gmx.at> wrote:

>> You're referring to the parenthetical note below my patch, right?
>
> Yes.
>
>> So I
>> tested my patched version of dired-pop-to-buffer after applying your
>> patch.  The result is that dired-pop-to-buffer does not raise an error
>> if I shrink the frame to a size too small to show all marked files,
>> irrespective of the presence or absence of a header line; in both cases
>> I can shrink the frame until only one file can be shown.
>
> Isn't that what you wanted?

I didn't mean to suggest either behavior was better or worse, I just
wanted to report the observation.  But I agree it certainly is better to
have uniform behavior, and no error is fine with me (at least, the
particular error message currently used in the no-header-line case seems
inappropriate).

>> That is, your
>> patch makes the behavior of dired-pop-to-buffer (with my patch)
>> uniformly like the behavior with a header line without your patch.  Is
>> that what you intend?
>
> I don't understand what you mean here.  What I wanted to resolve in the
> first place was to make `fit-window-to-buffer' not issue an error when
> the frame is to small.  

That's exactly what I meant, and my question was just to be sure this
was what you wanted to achieve.

>                         Is there anything else that doesn't work?

Not with fit-window-to-buffer, AFAICT.

Steve Berman

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

[-- Attachment #1: Type: text/plain, Size: 188 bytes --]

Stephen,

`fit-window-to-buffer' had more bugs than I thought.  I rewrote it
completely but you would have to use it for a couple of days to know
whether it DTRT.  Patch attached.

martin

[-- Attachment #2: window.el.diff --]
[-- Type: text/plain, Size: 7566 bytes --]

*** window.el.~1.169.~	2008-11-27 11:14:14.671875000 +0100
--- window.el	2008-12-06 20:20:34.390625000 +0100
***************
*** 1296,1379 ****
    "Adjust height of WINDOW to display its buffer's contents exactly.
  WINDOW defaults to the selected window.
  Optional argument MAX-HEIGHT specifies the maximum height of the
! window and defaults to the height of WINDOW's frame.
  Optional argument MIN-HEIGHT specifies the minimum height of the
  window and defaults to `window-min-height'.
  Both, MAX-HEIGHT and MIN-HEIGHT are specified in lines and
  include the mode line and header line, if any.
  Always return nil."
    (interactive)
! 
!   (when (null window)
!     (setq window (selected-window)))
!   (when (null max-height)
!     (setq max-height (frame-height (window-frame window))))
! 
!   (let* ((buf
! 	  ;; Buffer that is displayed in WINDOW
! 	  (window-buffer window))
! 	 (window-height
! 	  ;; The current height of WINDOW
! 	  (window-height window))
! 	 (desired-height
! 	  ;; The height necessary to show the buffer displayed by WINDOW
! 	  ;; (`count-screen-lines' always works on the current buffer).
! 	  (with-current-buffer buf
! 	    (+ (count-screen-lines)
! 	       ;; If the buffer is empty, (count-screen-lines) is
! 	       ;; zero.  But, even in that case, we need one text line
! 	       ;; for cursor.
! 	       (if (= (point-min) (point-max))
! 		   1 0)
! 	       ;; For non-minibuffers, count the mode-line, if any
! 	       (if (and (not (window-minibuffer-p window))
! 			mode-line-format)
! 		   1 0)
! 	       ;; Count the header-line, if any
! 	       (if header-line-format 1 0))))
! 	 (delta
! 	  ;; Calculate how much the window height has to change to show
! 	  ;; desired-height lines, constrained by MIN-HEIGHT and MAX-HEIGHT.
! 	  (- (max (min desired-height max-height)
! 		  (or min-height window-min-height))
! 	     window-height)))
! 
!     ;; Don't try to redisplay with the cursor at the end
!     ;; on its own line--that would force a scroll and spoil things.
!     (when (with-current-buffer buf
! 	    (and (eobp) (bolp) (not (bobp))))
!       (set-window-point window (1- (window-point window))))
! 
!     (save-selected-window
!       (select-window window 'norecord)
! 
!       ;; Adjust WINDOW to the nominally correct size (which may actually
!       ;; be slightly off because of variable height text, etc).
!       (unless (zerop delta)
! 	(enlarge-window delta))
! 
!       ;; Check if the last line is surely fully visible.  If not,
!       ;; enlarge the window.
!       (let ((end (with-current-buffer buf
! 		   (save-excursion
! 		     (goto-char (point-max))
! 		     (when (and (bolp) (not (bobp)))
! 		       ;; Don't include final newline
! 		       (backward-char 1))
! 		     (when truncate-lines
! 		       ;; If line-wrapping is turned off, test the
! 		       ;; beginning of the last line for visibility
! 		       ;; instead of the end, as the end of the line
! 		       ;; could be invisible by virtue of extending past
! 		       ;; the edge of the window.
! 		       (forward-line 0))
! 		     (point)))))
! 	(set-window-vscroll window 0)
! 	(while (and (< desired-height max-height)
! 		    (= desired-height (window-height window))
! 		    (not (pos-visible-in-window-p end window)))
! 	  (enlarge-window 1)
! 	  (setq desired-height (1+ desired-height)))))))
  
  (defun window-safely-shrinkable-p (&optional window)
    "Return t if WINDOW can be shrunk without shrinking other windows.
--- 1296,1383 ----
    "Adjust height of WINDOW to display its buffer's contents exactly.
  WINDOW defaults to the selected window.
  Optional argument MAX-HEIGHT specifies the maximum height of the
! window and defaults to the maximum permissible height of a window
! on WINDOW's frame.
  Optional argument MIN-HEIGHT specifies the minimum height of the
  window and defaults to `window-min-height'.
  Both, MAX-HEIGHT and MIN-HEIGHT are specified in lines and
  include the mode line and header line, if any.
  Always return nil."
    (interactive)
!   ;; Do all the work in WINDOW and its buffer and restore the selected
!   ;; window and the current buffer when we're done.
!   (save-excursion
!     (with-selected-window (or window (selected-window))
!       (set-buffer (window-buffer))
!       (let* ((desired-height
! 	      ;; The height necessary to show all of WINDOW's buffer.
! 	      ;; For an empty buffer (count-screen-lines) returns zero.
! 	      ;; Even in that case we need one line for the cursor.
! 	      (+ (max (count-screen-lines) 1)
! 		 ;; For non-minibuffers count the mode-line, if any.
! 		 (if (and (not (window-minibuffer-p)) mode-line-format) 1 0)
! 		 ;; Count the header-line, if any.
! 		 (if header-line-format 1 0)))
! 	     ;; MIN-HEIGHT must not be less than 1 and defaults to
! 	     ;; `window-min-height'.
! 	     (min-height (max (or min-height window-min-height) 1))
! 	     (max-window-height
! 	      ;; Maximum height of any window on this frame.
! 	      (min (window-height (frame-root-window)) (frame-height)))
! 	     ;; MAX-HEIGHT must not be larger than max-window-height and
! 	     ;; also defaults to that value.
! 	     (max-height
! 	      (min (or max-height max-window-height) max-window-height))
! 	     (delta
! 	      ;; How much the window height has to change to show
! 	      ;; desired-height lines, constrained by MIN-HEIGHT and
! 	      ;; MAX-HEIGHT.
! 	      (- (min max-height (max desired-height min-height))
! 		 (window-height)))
! 	     ;; Avoid deleting this window if it becomes too small.  As
! 	     ;; a side-effect, this may keep some other windows as well.
! 
! 	     ;; Note: The following was removed by Jan on 2007-07-09 but
! 	     ;; it seems needed to (1) avoid deleting the window we want
! 	     ;; to shrink, and (2) as a consequence of (1) have
! 	     ;; (window-height WINDOW) return the buffer displayed by
! 	     ;; WINDOW which seems like a bug in delete_all_subwindows
! 	     ;; which uses decode_any_window (so it doesn't care wether
! 	     ;; WINDOW is live).
! 	     (window-min-height 1))
! 	;; Don't try to redisplay with the cursor at the end on its own
! 	;; line--that would force a scroll and spoil things.
! 	(when (and (eobp) (bolp) (not (bobp)))
! 	  (set-window-point window (1- (window-point))))
! 	;; Use condition-case to handle any fixed-size windows and other
! 	;; pitfalls nearby.
! 	(condition-case nil
! 	    ;; Adjust WINDOW's height to the nominally correct one
! 	    ;; (which may actually be slightly off because of variable
! 	    ;; height text, etc).
! 	    (unless (zerop delta)
! 	      (enlarge-window delta)
! 	      ;; Check if the last line is surely fully visible.  If
! 	      ;; not, enlarge the window.
! 	      (let ((end (save-excursion
! 			   (goto-char (point-max))
! 			   (when (and (bolp) (not (bobp)))
! 			     ;; Don't include final newline
! 			     (backward-char 1))
! 			   (when truncate-lines
! 			     ;; If line-wrapping is turned off, test the
! 			     ;; beginning of the last line for
! 			     ;; visibility instead of the end, as the
! 			     ;; end of the line could be invisible by
! 			     ;; virtue of extending past the edge of the
! 			     ;; window.
! 			     (forward-line 0))
! 			   (point))))
! 		(set-window-vscroll window 0)
! 		(while (and (< (window-height) max-height)
! 			    (not (pos-visible-in-window-p end)))
! 		  (enlarge-window 1))))
! 	  (error nil))))))
  
  (defun window-safely-shrinkable-p (&optional window)
    "Return t if WINDOW can be shrunk without shrinking other windows.

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[Stephen Berman]

On Sat, 06 Dec 2008 20:25:00 +0100 martin rudalics <rudalics@gmx.at> wrote:

> `fit-window-to-buffer' had more bugs than I thought.  I rewrote it
> completely but you would have to use it for a couple of days to know
> whether it DTRT.  Patch attached.

I finally had time to apply your patch and give it a whirl.  Initial
tests together with my patched dired-pop-to-buffer indicate it DTRT,
i.e., both with and without a header line I can shrink the frame and
invoke dired-pop-to-buffer without signalling an error until only one
dired line is displayable: then shrinking is no longer possible and
dired-pop-to-buffer shows one file (also with signalling an error).
I'll keep using it, maybe I'll hit a corner case, but it seems ok.

Steve Berman

bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer

[martin rudalics]

 > I finally had time to apply your patch and give it a whirl.  Initial
 > tests together with my patched dired-pop-to-buffer indicate it DTRT,
 > i.e., both with and without a header line I can shrink the frame and
 > invoke dired-pop-to-buffer without signalling an error until only one
 > dired line is displayable: then shrinking is no longer possible and
 > dired-pop-to-buffer shows one file (also with signalling an error).
 > I'll keep using it, maybe I'll hit a corner case, but it seems ok.

I checked in a revised version of this.  It's pretty hard to get right,
especially when shrinking non-full-width-windows.  There's still a
corner case I cannot handle.  Anyway, please report immediately when
something appears wrong.

Thanks, martin.

bug#1488: marked as done (23.0.60; dired-pop-to-buffer: use fit-window-to-buffer)

[Emacs bug Tracking System]

[-- Attachment #1: Type: text/plain, Size: 877 bytes --]


Your message dated Thu, 11 Dec 2008 10:56:56 +0100
with message-id <4940E3E8.8050501@gmx.at>
and subject line Re: bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer
has caused the Emacs bug report #1488,
regarding 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact don@donarmstrong.com
immediately.)


-- 
1488: http://emacsbugs.donarmstrong.com/cgi-bin/bugreport.cgi?bug=1488
Emacs Bug Tracking System
Contact don@donarmstrong.com with problems

[-- Attachment #2: Type: message/rfc822, Size: 6922 bytes --]

[-- Attachment #2.1.1: Type: text/plain, Size: 1896 bytes --]

In GNU Emacs 23.0.60.21 (i686-pc-linux-gnu, GTK+ Version 2.12.9)
 of 2008-12-03 on escher

1. emacs -Q

2. Eval this:
   (define-minor-mode my-mm ()
     "My minor mode."
     :global t
     (if my-mm
         (setq default-header-line-format
   	       '(:eval (propertize "test" 'face '(:overline t))))
       (setq default-header-line-format nil)))

3. Enable my-mm.

4. C-x d foo (where `foo' is some directory with at least three files,
   e.g. emacs/lisp).

5. Mark three files in foo.

6. Type `C'
   ==> The popped up buffer *Marked Files* shows only two of the three
   marked files.

Likewise if the value of the 'face property in my-mm has a non-nil :box
attribute.  In contrast, if the value of the 'face property is
'(:underline t) (but not '(:underline t :overline t)), then in step 6
all three marked files are shown, though with no room to spare.

I guess the basic problem here is that Emacs calculates window-height in
integral equally sized line numbers.  But less fundamentally and more
directly, this problem arises, AFAICT, because dired-pop-to-buffer's
shrink-to-fit code fails to account for header lines.  To fix this it
would suffice to add this sexp above the last sexp in
dired-pop-to-buffer: 
    (with-current-buffer buf 
      (and header-line-format
           (progn (select-window w2) (enlarge-window 1))))

However, unless I'm missing something, fit-window-to-buffer does what
dired-pop-to-buffer's shrink-to-fit code does, and moreover also handles
header lines.  From perusing the change logs, I found several instances
where fit-window-to-buffer replaces library-specific code, and this
appears to be another candidate (dired-pop-to-buffer was introduced many
years before fit-window-to-buffer), in addition to fixing the above bug.
I'm not able to fix the display code to let window-height return
fractional line numbers, so I propose the following patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2.1.2: dired-pop-to-buffer patch --]
[-- Type: text/x-patch, Size: 2250 bytes --]

*** emacs/lisp/dired.el.~1.414.~	2008-12-03 10:17:51.000000000 +0100
--- emacs/lisp/dired.el			2008-12-04 10:11:05.000000000 +0100
***************
*** 2678,2721 ****
  
  (defun dired-pop-to-buffer (buf)
    ;; Pop up buffer BUF.
    ;; If dired-shrink-to-fit is t, make its window fit its contents.
!   (if (not dired-shrink-to-fit)
!       (pop-to-buffer (get-buffer-create buf))
!     ;; let window shrink to fit:
!     (let ((window (selected-window))
! 	  target-lines w2)
!       (cond ;; if split-height-threshold is enabled, use the largest window
!             ((and (> (window-height (setq w2 (get-largest-window)))
! 		     split-height-threshold)
! 		  (window-full-width-p w2))
! 	     (setq window w2))
! 	    ;; if the least-recently-used window is big enough, use it
! 	    ((and (> (window-height (setq w2 (get-lru-window)))
! 		     (* 2 window-min-height))
! 		  (window-full-width-p w2))
! 	     (setq window w2)))
!       (save-excursion
! 	(set-buffer buf)
! 	(goto-char (point-max))
! 	(skip-chars-backward "\n\r\t ")
! 	(setq target-lines (count-lines (point-min) (point)))
! 	;; Don't forget to count the last line.
! 	(if (not (bolp))
! 	    (setq target-lines (1+ target-lines))))
!       (if (<= (window-height window) (* 2 window-min-height))
! 	  ;; At this point, every window on the frame is too small to split.
! 	  (setq w2 (display-buffer buf))
! 	(setq w2 (split-window window
! 		  (max window-min-height
! 		       (- (window-height window)
! 			  (1+ (max window-min-height target-lines)))))))
!       (set-window-buffer w2 buf)
!       (if (< (1- (window-height w2)) target-lines)
! 	  (progn
! 	    (select-window w2)
! 	    (enlarge-window (- target-lines (1- (window-height w2))))))
!       (set-window-start w2 1)
!       )))
  
  (defcustom dired-no-confirm nil
    "A list of symbols for commands Dired should not confirm.
--- 2678,2687 ----
  
  (defun dired-pop-to-buffer (buf)
    ;; Pop up buffer BUF.
+   (pop-to-buffer (get-buffer-create buf))
    ;; If dired-shrink-to-fit is t, make its window fit its contents.
!   (when dired-shrink-to-fit
!     (fit-window-to-buffer (get-buffer-window buf))))
  
  (defcustom dired-no-confirm nil
    "A list of symbols for commands Dired should not confirm.

[-- Attachment #2.1.3: Type: text/plain, Size: 452 bytes --]


(Note that if the frame is shrunk so that not all marked file names fit
in the *Marked Files* window, then without an oversized header line as
is my-mm, dired-pop-to-buffer fails with the error "Attempt to delete
minibuffer or sole ordinary window"; whereas with an oversized header
line, this error is raised only if not even one file name can be shown.
But I see this difference in behavior whether or not the above patch is
applied.)

Steve Berman

[-- Attachment #3: Type: message/rfc822, Size: 1817 bytes --]

From: martin rudalics <rudalics@gmx.at>
To: 1488-done@emacsbugs.donarmstrong.com
Subject: Re: bug#1488: 23.0.60; dired-pop-to-buffer: use fit-window-to-buffer
Date: Thu, 11 Dec 2008 10:56:56 +0100
Message-ID: <4940E3E8.8050501@gmx.at>

Fixed as

2008-12-11  Stephen Berman <Stephen.Berman@rub.de>

	* dired.el (dired-pop-to-buffer): Use fit-window-to-buffer when
	dired-shrink-to-fit is non-nil.  (Bug#1488)

Thanks, martin.