bug#6000: describe-text-sexp does not know window-width

bug#6000: describe-text-sexp does not know window-width

[Lennart Borgman]

In describe-text-sexp there is a call to window-width. I believe this
gives unreliable results because the help-window might not be select
at that point always.

This shows up in the display of (what-cursor-position t) where the
sexp are sometimes unnecessary hidden behind "[Show]".

A possible good enough cure is to surround the call to window-width
with something like this

   (with-selected-window (or (get-buffer-window "*Help*") (selected-window))
     (window-width))

bug#6000: describe-text-sexp does not know window-width

[Juri Linkov]

> In describe-text-sexp there is a call to window-width. I believe this
> gives unreliable results because the help-window might not be select
> at that point always.
>
> This shows up in the display of (what-cursor-position t) where the
> sexp are sometimes unnecessary hidden behind "[Show]".
>
> A possible good enough cure is to surround the call to window-width
> with something like this
>
>    (with-selected-window (or (get-buffer-window "*Help*") (selected-window))
>      (window-width))

Martin referred to this bug report from bug#17831, and I noticed
that instead of hard-coding the "*Help*" buffer name, better would be
to use (current-buffer) because the formatted buffer is current:

  (- (if (window-live-p (get-buffer-window (current-buffer) t))
         (with-selected-window (get-buffer-window (current-buffer) t)
           (window-width))
       (window-width))
     (current-column))

bug#6000: describe-text-sexp does not know window-width

[Lars Ingebrigtsen]

Juri Linkov <juri@jurta.org> writes:

>>    (with-selected-window (or (get-buffer-window "*Help*") (selected-window))
>>      (window-width))
>
> Martin referred to this bug report from bug#17831, and I noticed
> that instead of hard-coding the "*Help*" buffer name, better would be
> to use (current-buffer) because the formatted buffer is current:
>
>   (- (if (window-live-p (get-buffer-window (current-buffer) t))
>          (with-selected-window (get-buffer-window (current-buffer) t)
>            (window-width))
>        (window-width))
>      (current-column))

This sounds like a good idea to me, but there wasn't a recipe to
reproduce whatever error this bug report was talking about, so I can't
test.

Does anybody have a test case that displays the problem (in case it's
still in place after all these years)?

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

bug#6000: describe-text-sexp does not know window-width

[Juri Linkov]

> This sounds like a good idea to me, but there wasn't a recipe to
> reproduce whatever error this bug report was talking about, so I can't
> test.
>
> Does anybody have a test case that displays the problem (in case it's
> still in place after all these years)?

Indeed a test case would be needed, but I have none.

bug#6000: describe-text-sexp does not know window-width

[martin rudalics]

 >> This sounds like a good idea to me, but there wasn't a recipe to
 >> reproduce whatever error this bug report was talking about, so I can't
 >> test.
 >>
 >> Does anybody have a test case that displays the problem (in case it's
 >> still in place after all these years)?
 >
 > Indeed a test case would be needed, but I have none.

There's no realistic test case for this.  You can do

(defun foo ()
   (interactive)
   (with-current-buffer (get-buffer-create "*foo*")
     (describe-text-sexp 'foooooooooooooooooooooooooooooooooo)))

and M-x foo in some window.  Dependent on the width of that window,
`describe-text-sexp' will decide whether to insert the expression in
*foo* or display a "Show" button instead.  When you eventually display
*foo* you will see the result.  I think a simple check like

     (if (and (not (string-match-p "\n" pp))
              (eq (window-buffer) (current-buffer))
              (<= (length pp) (- (window-width) (current-column))))
	(insert pp)

should work.

martin

bug#6000: describe-text-sexp does not know window-width

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> and M-x foo in some window.  Dependent on the width of that window,
> `describe-text-sexp' will decide whether to insert the expression in
> *foo* or display a "Show" button instead.  When you eventually display
> *foo* you will see the result.  I think a simple check like
>
>     (if (and (not (string-match-p "\n" pp))
>              (eq (window-buffer) (current-buffer))
>              (<= (length pp) (- (window-width) (current-column))))
> 	(insert pp)
>
> should work.

Hm...  this will always insert the [Show] button if the *foo* buffer
isn't the current buffer?  Is that what we want here?

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

bug#6000: describe-text-sexp does not know window-width

[martin rudalics]

 >> and M-x foo in some window.  Dependent on the width of that window,
 >> `describe-text-sexp' will decide whether to insert the expression in
 >> *foo* or display a "Show" button instead.  When you eventually display
 >> *foo* you will see the result.  I think a simple check like
 >>
 >>      (if (and (not (string-match-p "\n" pp))
 >>               (eq (window-buffer) (current-buffer))
 >>               (<= (length pp) (- (window-width) (current-column))))
 >> 	(insert pp)
 >>
 >> should work.
 >
 > Hm...  this will always insert the [Show] button if the *foo* buffer
 > isn't the current buffer?  Is that what we want here?

`describe-text-sexp' always works on the current buffer

(defun describe-text-sexp (sexp)
   "Insert a short description of SEXP in the current buffer."

so the added line will avoid the [Show] button iff the selected window
doesn't show the current buffer which should be rare enough.

martin

bug#6000: describe-text-sexp does not know window-width

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> `describe-text-sexp' always works on the current buffer
>
> (defun describe-text-sexp (sexp)
>   "Insert a short description of SEXP in the current buffer."
>
> so the added line will avoid the [Show] button iff the selected window
> doesn't show the current buffer which should be rare enough.

That's what I thought should happen, but your proposed change is the
patch below, which seems to do the opposite?  I.e., it'll always do the
[Show] button if the selected window doesn't show the current buffer...

diff --git a/lisp/descr-text.el b/lisp/descr-text.el
index 85017de5d5..f92f37d53d 100644
--- a/lisp/descr-text.el
+++ b/lisp/descr-text.el
@@ -51,7 +51,8 @@ describe-text-sexp
       (setq pp (substring pp 0 (1- (length pp)))))
 
     (if (and (not (string-match-p "\n" pp))
-    	     (<= (length pp) (- (window-width) (current-column))))
+             (eq (window-buffer) (current-buffer))
+             (<= (length pp) (- (window-width) (current-column))))
 	(insert pp)
       (insert-text-button
        "[Show]"


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

bug#6000: describe-text-sexp does not know window-width

[martin rudalics]

 >> Does anybody have a test case that displays the problem (in case it's
 >> still in place after all these years)?
 >
 > Indeed a test case would be needed, but I have none.

With emacs -Q evaluate

(progn
   (split-window nil -20 t)
   (put-text-property 1 2 'foo "This is a very long text property")
   (describe-char 1))

or

(progn
   (setq split-height-threshold nil)
   (setq split-width-threshold 20)
   (put-text-property 1 2 'foo "This is a very long text property")
   (describe-char 1))

Here these show a *Help* window on the right where not the entire text
property string is visible.  Depending on your settings it won't matter
much because the rest of that buffer is only partially visible too.

To show the inverse effect, again with emacs -Q, evaluate

(progn
   (split-window nil 20 t)
   (put-text-property 1 2 'foo t)
   (describe-char 1))

Here this shows *Help* in the window on the right with a line

   foo                  [Show]

where clicking on [Show] will show "t" in the window on the left.
This is embarrassingly silly.

Obviously, there are many more scenarios but most of them require to
customize `display-buffer-alist'.  Presumably, most people don't notice
the behavior - they just got used to it.

We could

- Do nothing: If, in the earlier scenarios, we'd insert a "Show" button
   instead of the text, clicking that button might again pop up a too
   narrow *Pp Eval Output* window and nothing has been gained.  As for
   the last scenario, we could tell users that they are on their own when
   they invoke `describe-char' in a too narrow window.

- Skip the `window-width' check in `describe-text-sexp' and maybe
   recommend using `temp-buffer-resize-mode' with a few tweaks like

(progn
   (temp-buffer-resize-mode)
   (setq temp-buffer-max-width 100)
   (setq fit-window-to-buffer-horizontally t)
   (setq split-height-threshold nil)
   (setq split-width-threshold 20)
   (put-text-property 1 2 'foo "This is a very long text property")
   (describe-char 1))

   But some people don't like `temp-buffer-resize-mode'.

- Calculate the maximum width of text in the buffer preceding the line
   where to insert the property with a function like

(defun max-column-in-frame ()
   "Return maximum column of current buffer in selected frame.
The return value is the longest column from the beginning of the
buffer to the line specified by the selected frame's height."
   (save-excursion
     (goto-char (point-min))
     (let ((width (frame-width))
	  (height (frame-height))
	  (line 0)
	  (max-column 0)
	  column)
       (while (and (< line height) (< (point) (point-max)))
	(setq column (move-to-column width))
	(when (> column max-column)
	  (setq max-column column))
	(forward-line 1)
	(setq line (1+ line)))
       max-column)))

and use that instead of `window-width' in `describe-text-sexp' as

     	     (<= (length pp) (- (max-column-in-frame) (current-column))))

This will delegate the problem to those who inserted text earlier into
that buffer.  If they were right, `describe-text-sexp' won't do anything
wrong.

martin