unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#6000: describe-text-sexp does not know window-width
@ 2010-04-22 12:15 Lennart Borgman
  2014-07-01 23:37 ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Lennart Borgman @ 2010-04-22 12:15 UTC (permalink / raw)
  To: 6000

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))







^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2010-04-22 12:15 bug#6000: describe-text-sexp does not know window-width Lennart Borgman
@ 2014-07-01 23:37 ` Juri Linkov
  2021-06-03  9:07   ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: Juri Linkov @ 2014-07-01 23:37 UTC (permalink / raw)
  To: Lennart Borgman; +Cc: 6000

> 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))





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2014-07-01 23:37 ` Juri Linkov
@ 2021-06-03  9:07   ` Lars Ingebrigtsen
  2021-06-03 20:25     ` Juri Linkov
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-06-03  9:07 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 6000, Lennart Borgman

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-03  9:07   ` Lars Ingebrigtsen
@ 2021-06-03 20:25     ` Juri Linkov
  2021-06-04  9:18       ` martin rudalics
  2021-06-06  7:42       ` martin rudalics
  0 siblings, 2 replies; 9+ messages in thread
From: Juri Linkov @ 2021-06-03 20:25 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 6000, Lennart Borgman

> 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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-03 20:25     ` Juri Linkov
@ 2021-06-04  9:18       ` martin rudalics
  2021-06-04  9:49         ` Lars Ingebrigtsen
  2021-06-06  7:42       ` martin rudalics
  1 sibling, 1 reply; 9+ messages in thread
From: martin rudalics @ 2021-06-04  9:18 UTC (permalink / raw)
  To: Juri Linkov, Lars Ingebrigtsen; +Cc: Lennart Borgman, 6000

 >> 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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-04  9:18       ` martin rudalics
@ 2021-06-04  9:49         ` Lars Ingebrigtsen
  2021-06-04 12:44           ` martin rudalics
  0 siblings, 1 reply; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-06-04  9:49 UTC (permalink / raw)
  To: martin rudalics; +Cc: 6000, Lennart Borgman

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-04  9:49         ` Lars Ingebrigtsen
@ 2021-06-04 12:44           ` martin rudalics
  2021-06-06  9:03             ` Lars Ingebrigtsen
  0 siblings, 1 reply; 9+ messages in thread
From: martin rudalics @ 2021-06-04 12:44 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: 6000, Lennart Borgman

 >> 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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-03 20:25     ` Juri Linkov
  2021-06-04  9:18       ` martin rudalics
@ 2021-06-06  7:42       ` martin rudalics
  1 sibling, 0 replies; 9+ messages in thread
From: martin rudalics @ 2021-06-06  7:42 UTC (permalink / raw)
  To: Juri Linkov, Lars Ingebrigtsen; +Cc: Lennart Borgman, 6000

 >> 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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#6000: describe-text-sexp does not know window-width
  2021-06-04 12:44           ` martin rudalics
@ 2021-06-06  9:03             ` Lars Ingebrigtsen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars Ingebrigtsen @ 2021-06-06  9:03 UTC (permalink / raw)
  To: martin rudalics; +Cc: 6000, Lennart Borgman

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





^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-06  9:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-22 12:15 bug#6000: describe-text-sexp does not know window-width Lennart Borgman
2014-07-01 23:37 ` Juri Linkov
2021-06-03  9:07   ` Lars Ingebrigtsen
2021-06-03 20:25     ` Juri Linkov
2021-06-04  9:18       ` martin rudalics
2021-06-04  9:49         ` Lars Ingebrigtsen
2021-06-04 12:44           ` martin rudalics
2021-06-06  9:03             ` Lars Ingebrigtsen
2021-06-06  7:42       ` martin rudalics

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).