unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
@ 2022-03-20 12:06 Augusto Stoffel
  2022-03-20 13:10 ` João Távora
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Augusto Stoffel @ 2022-03-20 12:06 UTC (permalink / raw)
  To: 54473; +Cc: João Távora, andreyk.mad

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

This should solve the Eglot-related problem described in this Github comment:

https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Eldoc-Handle-invisible-text-when-truncating-strings.patch --]
[-- Type: text/x-patch, Size: 2678 bytes --]

From c52b891b4197ee5cf9cec39993117487b0b4015f Mon Sep 17 00:00:00 2001
From: Augusto Stoffel <arstoffel@gmail.com>
Date: Sun, 20 Mar 2022 12:59:14 +0100
Subject: [PATCH] Eldoc: Handle invisible text when truncating strings

* lisp/emacs-lisp/eldoc.el (eldoc--echo-area-substring,
eldoc-display-in-echo-area):  Take invisible text into consideration
when counting lines to crop an echo-area message.
---
 lisp/emacs-lisp/eldoc.el | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lisp/emacs-lisp/eldoc.el b/lisp/emacs-lisp/eldoc.el
index 73713a3dec..16c08e1e10 100644
--- a/lisp/emacs-lisp/eldoc.el
+++ b/lisp/emacs-lisp/eldoc.el
@@ -102,7 +102,7 @@ eldoc-echo-area-use-multiline-p
 directly, while a float specifies the number of lines as a
 proportion of the echo area frame's height.
 
-If value is the symbol `truncate-sym-name-if-fit' t, the part of
+If value is the symbol `truncate-sym-name-if-fit', the part of
 the doc string that represents a symbol's name may be truncated
 if it will enable the rest of the doc string to fit on a single
 line, without resizing the echo area.
@@ -525,7 +525,8 @@ eldoc--echo-area-substring
                         (goto-char (point-min))
                         (skip-chars-forward " \t\n")
                         (point))
-                 (goto-char (line-end-position available))
+                 (forward-visible-line (1- available))
+                 (end-of-visible-line)
                  (skip-chars-backward " \t\n")))
         (truncated (save-excursion
                      (skip-chars-forward " \t\n")
@@ -535,7 +536,8 @@ eldoc--echo-area-substring
           ((and truncated
                 (> available 1)
                 eldoc-echo-area-display-truncation-message)
-           (goto-char (line-end-position 0))
+           (forward-visible-line -1)
+           (end-of-visible-line)
            (concat (buffer-substring start (point))
                    (format
                     "\n(Documentation truncated. Use `%s' to see rest)"
@@ -610,7 +612,8 @@ eldoc-display-in-echo-area
                (let ((string
                       (with-current-buffer (eldoc--format-doc-buffer docs)
                         (buffer-substring (goto-char (point-min))
-                                          (line-end-position 1)))))
+                                          (progn (end-of-visible-line)
+                                                 (point))))))
                  (if (> (length string) width)  ; truncation to happen
                      (unless (eldoc--echo-area-prefer-doc-buffer-p t)
                        (truncate-string-to-width string width))
-- 
2.35.1


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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 12:06 bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings Augusto Stoffel
@ 2022-03-20 13:10 ` João Távora
  2022-03-20 13:52 ` Eli Zaretskii
  2022-03-20 21:24 ` Andrii Kolomoiets
  2 siblings, 0 replies; 9+ messages in thread
From: João Távora @ 2022-03-20 13:10 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 54473, andreyk.mad

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

Looks fine, but would need, I think, a decent amount of testing:

* In eglot.el, being validated by both Andrii and you for your particular
use cases.
* In bare emacs-lisp-mode, with multiple eldoc-documentation-strategy
options.  It should
  behave exactly the same before and after the patch, as elisp doesn't use
invisible stuff.
* In other situations you can think of.  Like variations on
eldoc-area-use-multiline-p and
  eldoc-echo-area-display-truncation-messages.  Again, here the behaviour
should be
  equivalent pre- and post- patch.

Just noting this because in my memory this code is somewhat hairy.  That
said, the patch "looks" really fine :-)

Thanks,
João

On Sun, Mar 20, 2022 at 12:06 PM Augusto Stoffel <arstoffel@gmail.com>
wrote:

> This should solve the Eglot-related problem described in this Github
> comment:
>
> https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845
>
>

-- 
João Távora

[-- Attachment #2: Type: text/html, Size: 1614 bytes --]

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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 12:06 bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings Augusto Stoffel
  2022-03-20 13:10 ` João Távora
@ 2022-03-20 13:52 ` Eli Zaretskii
  2022-03-20 14:03   ` João Távora
  2022-03-20 21:24 ` Andrii Kolomoiets
  2 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2022-03-20 13:52 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 54473, joaotavora, andreyk.mad

> From: Augusto Stoffel <arstoffel@gmail.com>
> Date: Sun, 20 Mar 2022 13:06:56 +0100
> Cc: João Távora <joaotavora@gmail.com>,
>  andreyk.mad@gmail.com
> 
> This should solve the Eglot-related problem described in this Github comment:
> 
> https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845

If this is proposed for the release branch, then can you tell more?
The discussion to which you pointed seems to be caused by some recent
change to Eglot that you installed several days ago, so how does it
affect the current pretest of Emacs 28?

I need to understand this to decide whether to consider the change for
the release branch.

Thanks.





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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 13:52 ` Eli Zaretskii
@ 2022-03-20 14:03   ` João Távora
  2022-03-20 14:34     ` Augusto Stoffel
  0 siblings, 1 reply; 9+ messages in thread
From: João Távora @ 2022-03-20 14:03 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Augusto Stoffel, 54473, Andrii Kolomoiets

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

On Sun, Mar 20, 2022 at 1:52 PM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Augusto Stoffel <arstoffel@gmail.com>
> > Date: Sun, 20 Mar 2022 13:06:56 +0100
> > Cc: João Távora <joaotavora@gmail.com>,
> >  andreyk.mad@gmail.com
> >
> > This should solve the Eglot-related problem described in this Github
> comment:
> >
> > https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845
>
> If this is proposed for the release branch, then can you tell more?
> The discussion to which you pointed seems to be caused by some recent
> change to Eglot that you installed several days ago, so how does it
> affect the current pretest of Emacs 28?
>
> I need to understand this to decide whether to consider the change for
> the release branch.
>

Eglot, the Eldoc user, now simply doesn't strip _any_ properties from the
text
it sends Eldoc.  I think is a conceptually correct change to Eglot. It's
just that
Eldoc just doesn't have a way to deal with that yet.

If the patch is tested correctly, it is effectively fixing a bug in
eldoc.el,
whose current formatting/display code is oblivious to invisible text when
calculating how much actual (read visual) free space it has in the echo
area.
And, presumably, this patch fixes that shortcoming.

However, to err on the safe side, I'd say don't push it to the release
branch. Eldoc
is a "core ELPA package" so it can be distributed to the soon-to-be 28
release,
by simply bumping the ELPA package version and then asking for users
to update their packages.  This is how bugs in flymake.el, eldoc.el,
project.el
have been fixed recently: no need to wait for an Emacs release.

Maybe Augusto can fill in any details, if I haven't been clear.

João

[-- Attachment #2: Type: text/html, Size: 2648 bytes --]

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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 14:03   ` João Távora
@ 2022-03-20 14:34     ` Augusto Stoffel
  2022-03-20 15:57       ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Augusto Stoffel @ 2022-03-20 14:34 UTC (permalink / raw)
  To: João Távora; +Cc: 54473, Andrii Kolomoiets

In short, this should definitely not be merged into Emacs 28.





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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 14:34     ` Augusto Stoffel
@ 2022-03-20 15:57       ` Eli Zaretskii
  0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2022-03-20 15:57 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 54473, joaotavora, andreyk.mad

> From: Augusto Stoffel <arstoffel@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>,  54473@debbugs.gnu.org,  Andrii Kolomoiets
>  <andreyk.mad@gmail.com>
> Date: Sun, 20 Mar 2022 15:34:56 +0100
> 
> In short, this should definitely not be merged into Emacs 28.

OK, thanks to both of you.





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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 12:06 bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings Augusto Stoffel
  2022-03-20 13:10 ` João Távora
  2022-03-20 13:52 ` Eli Zaretskii
@ 2022-03-20 21:24 ` Andrii Kolomoiets
  2022-03-24 15:25   ` João Távora
  2 siblings, 1 reply; 9+ messages in thread
From: Andrii Kolomoiets @ 2022-03-20 21:24 UTC (permalink / raw)
  To: Augusto Stoffel; +Cc: 54473, João Távora

Augusto Stoffel <arstoffel@gmail.com> writes:

> This should solve the Eglot-related problem described in this Github comment:
>
> https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845

I've tested this patch with recent Emacs master and Eglot versions, with
various values for the `eldoc-echo-area-use-multiline-p' variable
(truncate-sym-name-if-fit, 2, 1).

All works as expected.

Thanks Augusto!





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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-20 21:24 ` Andrii Kolomoiets
@ 2022-03-24 15:25   ` João Távora
  2022-06-23 17:24     ` Stefan Kangas
  0 siblings, 1 reply; 9+ messages in thread
From: João Távora @ 2022-03-24 15:25 UTC (permalink / raw)
  To: Andrii Kolomoiets, eliz; +Cc: Augusto Stoffel, 54473

Andrii Kolomoiets <andreyk.mad@gmail.com> writes:

> Augusto Stoffel <arstoffel@gmail.com> writes:
>
>> This should solve the Eglot-related problem described in this Github comment:
>>
>> https://github.com/joaotavora/eglot/issues/865#issuecomment-1065565845
>
> I've tested this patch with recent Emacs master and Eglot versions, with
> various values for the `eldoc-echo-area-use-multiline-p' variable
> (truncate-sym-name-if-fit, 2, 1).
>
> All works as expected.
>
> Thanks Augusto!

I've pushed this change to master and bumped the eldoc.el package
version.

João





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

* bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings
  2022-03-24 15:25   ` João Távora
@ 2022-06-23 17:24     ` Stefan Kangas
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Kangas @ 2022-06-23 17:24 UTC (permalink / raw)
  To: João Távora; +Cc: Augusto Stoffel, eliz, 54473, Andrii Kolomoiets

close 54473 29.1
thanks

João Távora <joaotavora@gmail.com> writes:

> I've pushed this change to master and bumped the eldoc.el package
> version.

This was pushed to master, so I'm closing this bug:

    commit 45978f97be89ae989ecf9e7129b88592e70a1f24
    Author: Augusto Stoffel <arstoffel@gmail.com>
    Date:   Thu Mar 24 15:05:39 2022 +0000

        Handle invisible text in Eldoc when calculating size

        Co-authored-by: João Távora <joaotavora@gmail.com>

        * lisp/emacs-lisp/eldoc.el (eldoc--echo-area-substring,
        eldoc-display-in-echo-area):  Take invisible text into consideration
        when counting lines to crop an echo-area message.
        (Version): Bump.





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

end of thread, other threads:[~2022-06-23 17:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-20 12:06 bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings Augusto Stoffel
2022-03-20 13:10 ` João Távora
2022-03-20 13:52 ` Eli Zaretskii
2022-03-20 14:03   ` João Távora
2022-03-20 14:34     ` Augusto Stoffel
2022-03-20 15:57       ` Eli Zaretskii
2022-03-20 21:24 ` Andrii Kolomoiets
2022-03-24 15:25   ` João Távora
2022-06-23 17:24     ` Stefan Kangas

Code repositories for project(s) associated with this 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).