bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[Drew Adams]

Dunno if there is a bug here, beyond possibly improving the doc
string. In general, I have a question about the code.
 
The code imposes this as one of the conditions for fitting the window
to the buffer:
 
(or (not (frame-parameter frame 'minibuffer))
    (let ((mini-window (minibuffer-window frame)))
      (or (null mini-window)
          (not (eq frame (window-frame mini-window)))
               (< (nth 3 edges) (nth 1 (window-edges mini-window)))
               (> (nth 1 edges) (frame-parameter frame 'menu-bar-lines)))))
 
I don't understand the last condition (>). At the least, this
condition seems to be missing from the doc string:
 
"Do nothing if the buffer contains more lines than the present window
height, or if some of the window's contents are scrolled out of view,
or if shrinking this window would also shrink another window, or if
the window is the only window of its frame."
 
What is that last condition (>) for?  What is the relation between the
number of menu-bar-lines and the `Top' position of the window?
 
And doesn't this code raise an error if the `menu-bar-lines' parameter
is nil? Should it really be doing that?
 
 
 
In GNU Emacs 23.0.94.1 (i386-mingw-nt5.1.2600)
 of 2009-05-24 on SOFT-MJASON
Windowing system distributor `Microsoft Corp.', version 5.1.2600
configured using `configure --with-gcc (3.4)'
 

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[martin rudalics]

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

 > The code imposes this as one of the conditions for fitting the window
 > to the buffer:
 >
 > (or (not (frame-parameter frame 'minibuffer))
 >     (let ((mini-window (minibuffer-window frame)))
 >       (or (null mini-window)
 >           (not (eq frame (window-frame mini-window)))
 >                (< (nth 3 edges) (nth 1 (window-edges mini-window)))
 >                (> (nth 1 edges) (frame-parameter frame 'menu-bar-lines)))))
 >
 > I don't understand the last condition (>). At the least, this
 > condition seems to be missing from the doc string:
 >
 > "Do nothing if the buffer contains more lines than the present window
 > height, or if some of the window's contents are scrolled out of view,
 > or if shrinking this window would also shrink another window, or if
 > the window is the only window of its frame."
 >
 > What is that last condition (>) for?  What is the relation between the
 > number of menu-bar-lines and the `Top' position of the window?

I guess it's a poor man's attempt to make sure that there's some other
window above or below the one to shrink.

 > And doesn't this code raise an error if the `menu-bar-lines' parameter
 > is nil? Should it really be doing that?

Looks like a bug.  Could you try the attached patch?  Since this code
hasn't been touched for quite some time we can't fix it for Emacs 23.1
though.

martin

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

*** window.el.~1.180.~	2009-05-07 11:10:52.000000000 +0200
--- window.el	2009-06-18 11:31:13.234375000 +0200
***************
*** 1520,1527 ****
    (when (null window)
      (setq window (selected-window)))
    (let* ((frame (window-frame window))
! 	 (mini (frame-parameter frame 'minibuffer))
! 	 (edges (window-edges window)))
      (if (and (not (eq window (frame-root-window frame)))
  	     (window-safely-shrinkable-p window)
  	     (pos-visible-in-window-p (point-min) window)
--- 1520,1526 ----
    (when (null window)
      (setq window (selected-window)))
    (let* ((frame (window-frame window))
! 	 (mini (frame-parameter frame 'minibuffer)))
      (if (and (not (eq window (frame-root-window frame)))
  	     (window-safely-shrinkable-p window)
  	     (pos-visible-in-window-p (point-min) window)
***************
*** 1530,1539 ****
  		 (let ((mini-window (minibuffer-window frame)))
  		   (or (null mini-window)
  		       (not (eq frame (window-frame mini-window)))
! 		       (< (nth 3 edges)
! 			  (nth 1 (window-edges mini-window)))
! 		       (> (nth 1 edges)
! 			  (frame-parameter frame 'menu-bar-lines))))))
  	(fit-window-to-buffer window (window-height window)))))
  
  (defun kill-buffer-and-window ()
--- 1529,1536 ----
  		 (let ((mini-window (minibuffer-window frame)))
  		   (or (null mini-window)
  		       (not (eq frame (window-frame mini-window)))
! 		       (/= (window-height window)
! 			   (window-height (frame-root-window frame)))))))
  	(fit-window-to-buffer window (window-height window)))))
  
  (defun kill-buffer-and-window ()

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[Drew Adams]

> I guess it's a poor man's attempt to make sure that there's some other
> window above or below the one to shrink.
> 
>  > And doesn't this code raise an error if the `menu-bar-lines'
>  > parameter is nil? Should it really be doing that?
> 
> Looks like a bug.  Could you try the attached patch?  Since this code
> hasn't been touched for quite some time we can't fix it for Emacs 23.1
> though.

I don't know how to try it. What recipe do you suggest?

I didn't try anything before; I was just perusing the code, and I wondered what
it was trying to do.

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[martin rudalics]

 > I don't know how to try it. What recipe do you suggest?

Apply the patch and either remake emacs or put the code of the patched
`shrink-window-if-larger-than-buffer' into your .emacs and wait till it
starts barking.  And don't forget to tell me if it doesn't bark at you
for a couple of weeks.  Then we can install it.

 > I didn't try anything before; I was just perusing the code, and I wondered what
 > it was trying to do.

The only way to understand such code is by comparing the original code
with the present one and, if they differ, go through the revisions, look
at the ChangeLogs, and maybe scan emacs-devel and friends for errors
reported wrt that code.

martin

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[Drew Adams]

>  > I don't know how to try it. What recipe do you suggest?
> 
> Apply the patch and either remake emacs or put the code of the patched
> `shrink-window-if-larger-than-buffer' into your .emacs and 
> wait till it starts barking.  And don't forget to tell me if it
> doesn't bark at you for a couple of weeks.  Then we can install it.

I think I'll pass. The affected part of the code is something that I will
probably never run into, let alone within a couple of weeks. The use case for
that code path is something that someone else will need to test, if it is tested
at all.

>  > I didn't try anything before; I was just perusing the 
>  > code, and I wondered what it was trying to do.
> 
> The only way to understand such code is by comparing the original code
> with the present one and, if they differ, go through the 
> revisions, look at the ChangeLogs, and maybe scan emacs-devel and
> friends for errors reported wrt that code.

I think you misunderstood me. I was speaking about perusing the original code,
not yours. I was saying that I didn't run into a problem trying to do something.
I didn't run into a problem at all. I was simply browsing the (original) code
and noticed that it seemed strange. That's all.

I have no problem with your fix. (And I personally have no problem if no fix is
applied.)

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[martin rudalics]

 >>  > I didn't try anything before; I was just perusing the
 >>  > code, and I wondered what it was trying to do.
 >>
 >> The only way to understand such code is by comparing the original code
 >> with the present one and, if they differ, go through the
 >> revisions, look at the ChangeLogs, and maybe scan emacs-devel and
 >> friends for errors reported wrt that code.
 >
 > I think you misunderstood me. I was speaking about perusing the original code,
 > not yours. I was saying that I didn't run into a problem trying to do something.
 > I didn't run into a problem at all. I was simply browsing the (original) code
 > and noticed that it seemed strange. That's all.

What's your semantics of the word "peruse"?

martin

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[Lars Magne Ingebrigtsen]

"Drew Adams" <drew.adams@oracle.com> writes:

> I think you misunderstood me. I was speaking about perusing the
> original code, not yours. I was saying that I didn't run into a
> problem trying to do something.  I didn't run into a problem at all. I
> was simply browsing the (original) code and noticed that it seemed
> strange. That's all.

This doesn't seem like a bug, so I'm closing the report.

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#3600: 23.0.94; shrink-window-if-larger-than-buffer

[Drew Adams]

> This doesn't seem like a bug, so I'm closing the report.

Why?  Why does it not seem like a bug to you?  The Emacs windows expert (Martin)
said clearly "Looks like a bug.", and he provided a patch that tries to to fix
it.

I reported it, but I declined to build Emacs and test the proposed fix.  That
does not imply that there is no bug.