Hyperlinks from markdown hover text are unclickable #865

astoff · 2022-03-09T20:02:08Z

Server used: Digestif
Emacs version: 28
Operating system: Linux
Eglot version: 1.8
Eglot installation method: ELPA
Using Doom: No

When an Eldoc buffer produced by Eglot contains hyperlinks (because the server sent the hover text in markdown format), the hyperlinks are formatted as such and contain the URL in the help-echo property, but clicking a link doesn't work.

This is because the markdown-mode command that opens the link relies on invisible text around the hyperlink to discover the URL. However, Eglot strips the invisible text in this line:

eglot/eglot.el

Line 1439 in d03235f

(string-trim (filter-buffer-substring (point-min) (point-max))))))

In my opinion, the correct implementation of that function is as follows:

https://github.com/astoff/eglot/blob/b4a416f1c13ce0472944e385b867d79d9277c880/eglot.el#L1439-L1441

Now, the use of filter-buffer-substring originated from #482. The problem was that a specific server would send a hover string that started with "```\n", so when Eldoc would display the first line of the Eldoc buffer in the echo area, the results were poor. Here are my observations:

I can't reproduce the reported behavior with pylsp, perhaps the server changed its behavior.
On the other hand, I don't know if any other server adds hyperlinks. Digestif does that because most TeX documentation is in PDF format, and pointing to a file for the user to look further is about the best I can do in most cases. (This also makes this issue rather low priority, I guess).
Markdown mode is probably not at fault, as discussed here: markdown--filter-visible makes hyperlinks unclickable jrblevin/markdown-mode#693
In the problem reported at Use filter-buffer-substring to get buffer text #482, it's arguably Eldoc who's at fault. It should copy one visual line to the echo area and make sure it retains its essential display properties.

The text was updated successfully, but these errors were encountered:

joaotavora · 2022-03-09T20:09:28Z

I like your reasoning Augusto. Let's fix Eglot and Eldoc. In that order sounds good to me. João

…

On Wed, Mar 9, 2022, 20:02 Augusto Stoffel ***@***.***> wrote: - Server used: Digestif - Emacs version: 28 - Operating system: Linux - Eglot version: 1.8 - Eglot installation method: ELPA - Using Doom: No When an Eldoc buffer produced by Eglot contains hyperlinks (because the server sent the hover text in markdown format), the hyperlinks are formatted as such and contain the URL in the help-echo property, but clicking a link doesn't work. This is because the markdown-mode command that opens the link relies on invisible text around the hyperlink to discover the URL. However, Eglot strips the invisible text in this line: https://github.com/joaotavora/eglot/blob/d03235f39a39bdf2e74571d695163aa42f5b9506/eglot.el#L1439 In my opinion, the correct implementation of that function is as follows: https://github.com/astoff/eglot/blob/b4a416f1c13ce0472944e385b867d79d9277c880/eglot.el#L1439-L1441 Now, the use of filter-buffer-substring originated from #482 <#482>. The problem was that a specific server would send a hover string that started with "```\n", so when Eldoc would display the first line of the Eldoc buffer in the echo area, the results were poor. Here are my observations: - I can't reproduce the reported behavior with pylsp, perhaps the server changed its behavior. - On the other hand, I don't know if any other server adds hyperlinks. Digestif does that because most TeX documentation is in PDF format, and pointing to a file for the user to look further is about the best I can do in most cases. (This also makes this issue rather low priority, I guess). - Markdown mode is probably not at fault, as discussed here: jrblevin/markdown-mode#693 <jrblevin/markdown-mode#693> - In the problem reported at #482 <#482>, it's arguably Eldoc who's at fault. It should copy one visual line to the echo area and make sure it retains its essential display properties. — Reply to this email directly, view it on GitHub <#865>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC6PQYIAAXZWZETDQCUNOTU7D7VBANCNFSM5QKSIR4A> . Triage notifications on the go with GitHub Mobile for iOS <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

This was introduced in joaotavora#482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Fixes joaotavora#865. * eglot.el (eglot--format-markup): Don't use 'filter-buffer-substring'; instead, keep invisible text.

astoff · 2022-03-09T20:20:24Z

Okay, I made a pull request. But please take into account that this order of fixing things may break Eldoc for some users, for some time.

astoff · 2022-03-09T20:22:34Z

By the way, we should mention this to @muffinmad, who introduced the change that led to this.

This was introduced in joaotavora#482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Fixes joaotavora#865. * eglot.el (eglot--format-markup): Don't use 'filter-buffer-substring'; instead, keep invisible text.

This was introduced in #482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Per #866 * eglot.el (eglot--format-markup): Use buffer-string instead of filter-buffer-substring

muffinmad · 2022-03-11T21:52:19Z

Sadly, this breaks my setup. I'm using anakin-language-server wich sends hover info like

(:kind "markdown" :value "```python\nint(x: Union[str, bytes, SupportsInt, _SupportsIndex, _SupportsTrunc]=...)\n...

Setting the eldoc-echo-area-use-multiline-p variable to 1 leads to empty echo area instead of the first line of the hover info.

joaotavora · 2022-03-11T22:45:18Z

I don't understand this. Both pre-and and post-change code seems to yield the same value when applied to your example @muffinmad:

(eglot--format-markup "```python\nint(x: Union[str, bytes, SupportsInt, _SupportsIndex, _SupportsTrunc]=...)\n")

returns:

#("```python\nint(x: Union[str, bytes, SupportsInt, _SupportsIndex, _SupportsTrunc]=...)\n```" 0 3
  (face
   (markdown-markup-face markdown-code-face)
   invisible markdown-markup markdown-gfm-block-begin
   (1 10 1 4 4 4 4 10 nil nil 10 10 #<killed buffer>))
  3 9
  (face
   (markdown-language-keyword-face markdown-code-face)
   invisible markdown-markup markdown-gfm-block-begin
   (1 10 1 4 4 4 4 10 nil nil 10 10 #<killed buffer>))
  9 10
  (invisible markdown-markup face
             (markdown-code-face)
             font-lock-multiline t)
  10 85
  (font-lock-multiline t face
                       (markdown-pre-face markdown-code-face)
                       markdown-gfm-code
                       (11 86))
  85 86
  (font-lock-multiline t face
                       (markdown-markup-face markdown-code-face)
                       invisible markdown-markup markdown-gfm-block-end
                       (86 89 86 89 89 89 #<killed buffer>))
  86 87
  (face
   (markdown-markup-face markdown-code-face)
   invisible markdown-markup markdown-gfm-block-end
   (86 89 86 89 89 89 #<killed buffer>))
  87 88
  (face
   (markdown-markup-face markdown-code-face)
   invisible markdown-markup font-lock-multiline t markdown-gfm-block-end
   (86 89 86 89 89 89 #<killed buffer>)))

in both cases. What am I missing?

astoff · 2022-03-12T10:58:05Z

@muffinmad Of course Eglot should work with all sensible servers, but since you are the author of anakin-language-server, may I ask why you add that specific piece of markup? I suppose you want the server to work well on editors that can't format markdown, or slightly mangle it.

joaotavora · 2022-03-12T11:02:10Z

My two cents on the markdown vs non-markdown debate is that I think it's practically a standard now, there's no point in thinking too much about non-support (only a tiny bit ;-) )

My immediate interest here is to make both of you happy. It seems @astoff that all you were missing was hyperlinks right? Or more? Does digestif emit markdown or not?

astoff · 2022-03-12T11:09:27Z

Of course, we should fix the issue @muffinmad reports. Emacs shouldn't be among the editors that slightly mangle markdown rendering. I was just saying that as a server developer I would play safe with the first line of the hover string specifically.

Now, the place we need to look is when Eldoc selects n lines of the Eldoc buffer to display in the echo area. It should be n visual lines, but currently it picks n actual lines.

muffinmad · 2022-03-12T16:56:32Z

@muffinmad Of course Eglot should work with all sensible servers, but since you are the author of anakin-language-server, may I ask why you add that specific piece of markup? I suppose you want the server to work well on editors that can't format markdown, or slightly mangle it.

In case the client is not reporting support for hover content in markdown mode, the server will respond with plain text:

(:jsonrpc "2.0" :id 12 :result
	  (:contents
	   (:kind "plaintext" :value "foo(bar: str, baz: int) -> bool\n\nSome docstring.\nUsage example:\n\n    if foo(\"\", 0):\n        return")))

The markdown format is sent only when the client reporting markdown support.

The first part of the hover info is usually the function signature, which is valid python code. So, to have syntax hightlight, it is wrapped in python code block.
The second part of the hover info is wrapped by plain code block in order to use fixed font and preserve indentation.

The whole hover info will look like this:

muffinmad · 2022-03-12T17:07:05Z

Also, although the number of visible lines is the same, eldoc desides to truncate the hover info:

The 7 lines of the hover info in plainttext format are fully displayed.

muffinmad · 2022-03-12T17:09:52Z

And here is screenshot of the previous behaviour:

astoff · 2022-03-20T12:08:12Z

@muffinmad I understand the idea and the goal. I was just wondering why you want to do this, given that it will almost surely break some editor, even if we fix Eglot :-).

nemethf · 2022-03-20T16:46:24Z

This is a little bit orthogonal to this issue, but if I understand correctly, @muffinmad's underlying assumption is that "the first part of the hover info is usually the function signature". If eldoc-documentation-functions is just '(eglot-signature-eldoc-function), then eldoc will show just the signature. And it seems anakin-language-server is a signatureHelpProvider. So, what's wrong with this setup?

muffinmad · 2022-03-20T16:55:32Z

@astoff Some other editor is pretty fine with markdown format. Maybe because they do not try to get the first line of the docs :)

muffinmad · 2022-03-20T17:03:21Z

@nemethf In case hover info will not contain signature in the first line, it still be wrapped in code block to preserve indentation

(:jsonrpc "2.0" :id 60 :result
          (:contents
           (:kind "markdown" :value "```\nConcrete date/time and related types.\n\nSee http://www.iana.org/time-zones/repository/tz-link.html for\ntime zone and DST data sources.\n```")))

astoff · 2022-03-20T17:29:26Z

Some other editor is pretty fine with markdown format. Maybe because they do not try to get the first line of the docs :)

I'm sure some other editor will handle that markup just fine. But I'm also sure not every editor will.

In case hover info will not contain signature in the first line, it still be wrapped in code block to preserve indentation

This is clearly a wrong use of a code block, since what's inside the code block is not code. According to the markdown documentation, you can insert a hard break by ending a line with at least two spaces:

$ echo -e 'a\nb  \nc' | pandoc --to html
<p>a b<br />
c</p>

muffinmad · 2022-03-20T18:10:07Z

I'm sure some other editor will handle that markup just fine. But I'm also sure not every editor will.

If the editor enables markdown format support in hover info, then it is the editor task to handle it properly, right?

This is clearly a wrong use of a code block, since what's inside the code block is not code. According to the markdown documentation, you can insert a hard break by ending a line with at least two spaces:

It's not about line breaks, but about docstring formattig using spaces. And the monospaced font. Here is the example from the docstring of import statement:

The "import" statement
**********************

   import_stmt     ::= "import" module ["as" identifier] ("," module ["as" identifier])*
                   | "from" relative_module "import" identifier ["as" identifier]
                   ("," identifier ["as" identifier])*
                   | "from" relative_module "import" "(" identifier ["as" identifier]
                   ("," identifier ["as" identifier])* [","] ")"
                   | "from" relative_module "import" "*"
   module          ::= (identifier ".")* identifier
   relative_module ::= "."* module | "."+

Can you imagine how it will look like without the <pre>? :)

According to https://daringfireball.net/projects/markdown/syntax#precode

Pre-formatted code blocks are used for writing about programming or markup source code

I don't see it says "code blocks is only for code". Can we classify doscstrings as "writing about programming"? :)

joaotavora · 2022-03-20T18:15:20Z

@muffinmad regardless of your arguments, which are interesting (and on which I don't have a very clear position, due mainly to ignorance and lack of testing on these servers), have you tested @astoff 's patch to eldoc.el in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=54473 ?

Can you tell if it solves your problem completely/partially/not at all?

muffinmad · 2022-03-20T18:18:43Z

@joaotavora Not yet. But I will.

joaotavora · 2022-03-20T18:23:17Z

Cool. If possible, please reply to that thread. I think your feedback is more useful there.

This reverts commit 6e59eea. joaotavora/eglot#865 has been fixed.

…hover string This was introduced in joaotavora/eglot#482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Per joaotavora/eglot#866 * eglot.el (eglot--format-markup): Use buffer-string instead of filter-buffer-substring

This was introduced in #482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Per #866 * eglot.el (eglot--format-markup): Use buffer-string instead of filter-buffer-substring #865: joaotavora/eglot#865 #482: joaotavora/eglot#482 #866: joaotavora/eglot#866

This was introduced in joaotavora/eglot#482 due to a bad interaction with a specific server. But this solution makes hyperlinks in Eldoc buffers unclickable, because the markdown-mode function that visits a link relies on the invisible text. Per joaotavora/eglot#866 * eglot.el (eglot--format-markup): Use buffer-string instead of filter-buffer-substring GitHub-reference: fix joaotavora/eglot#865

astoff mentioned this issue Mar 9, 2022

Don't strip invisible text when formatting hover string #866

Merged

joaotavora closed this as completed in #866 Mar 10, 2022

dickmao mentioned this issue Mar 20, 2022

28.0.91; [PATCH] Eldoc: Handle invisible text when truncating strings commercial-emacs/commercial-emacs#300

Closed

mpolden added a commit to mpolden/emacs.d that referenced this issue Apr 5, 2022

Revert "markdown: Add helper for browsing fontified links"

b841892

This reverts commit 6e59eea. joaotavora/eglot#865 has been fixed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hyperlinks from markdown hover text are unclickable #865

Hyperlinks from markdown hover text are unclickable #865

astoff commented Mar 9, 2022

joaotavora commented Mar 9, 2022 via email

astoff commented Mar 9, 2022

astoff commented Mar 9, 2022

muffinmad commented Mar 11, 2022

joaotavora commented Mar 11, 2022

astoff commented Mar 12, 2022

joaotavora commented Mar 12, 2022

astoff commented Mar 12, 2022

muffinmad commented Mar 12, 2022

muffinmad commented Mar 12, 2022

muffinmad commented Mar 12, 2022

astoff commented Mar 20, 2022

nemethf commented Mar 20, 2022

muffinmad commented Mar 20, 2022

muffinmad commented Mar 20, 2022

astoff commented Mar 20, 2022 •

edited

Loading

muffinmad commented Mar 20, 2022 •

edited

Loading

joaotavora commented Mar 20, 2022

muffinmad commented Mar 20, 2022

joaotavora commented Mar 20, 2022

Hyperlinks from markdown hover text are unclickable #865

Hyperlinks from markdown hover text are unclickable #865

Comments

astoff commented Mar 9, 2022

joaotavora commented Mar 9, 2022 via email

astoff commented Mar 9, 2022

astoff commented Mar 9, 2022

muffinmad commented Mar 11, 2022

joaotavora commented Mar 11, 2022

astoff commented Mar 12, 2022

joaotavora commented Mar 12, 2022

astoff commented Mar 12, 2022

muffinmad commented Mar 12, 2022

muffinmad commented Mar 12, 2022

muffinmad commented Mar 12, 2022

astoff commented Mar 20, 2022

nemethf commented Mar 20, 2022

muffinmad commented Mar 20, 2022

muffinmad commented Mar 20, 2022

astoff commented Mar 20, 2022 • edited Loading

muffinmad commented Mar 20, 2022 • edited Loading

joaotavora commented Mar 20, 2022

muffinmad commented Mar 20, 2022

joaotavora commented Mar 20, 2022

astoff commented Mar 20, 2022 •

edited

Loading

muffinmad commented Mar 20, 2022 •

edited

Loading