bug#50716: 28.0.50; whitespace-report does not use existing windows

bug#50716: 28.0.50; whitespace-report does not use existing windows

[Gabriel]

Description:
The command 'whitespace-report' does not using existing windows, so
every new invocation opens a new window at bottom. This behavior is
different from how other emacs commands handle windows. I expect
'whitespace-report' to behave similarly to other commands (e.g.:
'describe-variable', 'memory-report', etc). The problem seems to be on
'whitespace-display-window'.

Steps to reproduce:
1) 'emacs -Q' from master branch
2) M-x 'whitespace-report'
3) 'C-x o' to move to "*Whitespace Report*" buffer
4) 'C-x k RET' to kill the "*Whitespace Report*" buffer
5) Repeat steps 2-4 multiple times

--
        Gabriel

bug#50716: 28.0.50; whitespace-report does not use existing windows

[Lars Ingebrigtsen]

Gabriel <gabriel376@hotmail.com> writes:

> The command 'whitespace-report' does not using existing windows, so
> every new invocation opens a new window at bottom. This behavior is
> different from how other emacs commands handle windows. I expect
> 'whitespace-report' to behave similarly to other commands (e.g.:
> 'describe-variable', 'memory-report', etc). The problem seems to be on
> 'whitespace-display-window'.

Yup.  Fixed now in Emacs 28.

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

bug#50716: 28.0.50; whitespace-report does not use existing windows

[martin rudalics]

@@ -1839,13 +1839,8 @@ cleaning up these problems."
    "Display BUFFER in a new window."
    (goto-char (point-min))
    (set-buffer-modified-p nil)
-  (when (< (window-height) (* 2 window-min-height))
-    (kill-buffer buffer)
-    (error "Window height is too small; \
-can't split window to display whitespace toggle options"))
-  (let ((win (split-window)))
-    (set-window-buffer win buffer)
-    (shrink-window-if-larger-than-buffer win)))
+  (pop-to-buffer buffer)
+  (shrink-window-if-larger-than-buffer))

I think this needs a new doc-string and the last two lines should
probably become something like

(let ((window (display-buffer
	       buffer
	       `((display-buffer-reuse-window
		  display-buffer-below-selected)))))
   (shrink-window-if-larger-than-buffer window))

since the original code did not select the window and tried to show it
below the selected one.

martin

bug#50716: 28.0.50; whitespace-report does not use existing windows

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> I think this needs a new doc-string and the last two lines should
> probably become something like
>
> (let ((window (display-buffer
> 	       buffer
> 	       `((display-buffer-reuse-window
> 		  display-buffer-below-selected)))))
>   (shrink-window-if-larger-than-buffer window))
>
> since the original code did not select the window and tried to show it
> below the selected one.

Thanks; pushed now to Emacs 28.

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

bug#50716: 28.0.50; whitespace-report does not use existing windows

[martin rudalics]

-  "Display BUFFER in a new window."

Why did you drop the doc-string?

martin

bug#50716: 28.0.50; whitespace-report does not use existing windows

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> -  "Display BUFFER in a new window."
>
> Why did you drop the doc-string?

It's an internal function that has a function name that's descriptive
enough, so the doc string seems superfluous.  If you disagree, feel free
to add a new one.

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

bug#50716: 28.0.50; whitespace-report does not use existing windows

[martin rudalics]

 > It's an internal function that has a function name that's descriptive
 > enough,

Incidentally that function name is quite misleading - we do not display
"windows" but "buffers".

 > so the doc string seems superfluous.  If you disagree, feel free
 > to add a new one.

Done.

martin