bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Augusto Stoffel]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[João Távora]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Eli Zaretskii]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[João Távora]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Augusto Stoffel]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Eli Zaretskii]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Andrii Kolomoiets]

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!

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[João Távora]

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

bug#54473: 28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings

[Stefan Kangas]

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.