Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 10, 2022

Conversation

astoff
Copy link
Contributor

@astoff astoff commented Mar 9, 2022

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.

Fixes #865.

  • eglot.el (eglot--format-markup): Don't use
    'filter-buffer-substring'; instead, keep invisible text.

@joaotavora
Copy link
Owner

Why not keep the string-trim and use simply buffer-substring?

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
Copy link
Contributor Author

astoff commented Mar 10, 2022

I seemed a bit more idiomatic to not allocate one or two extra strings, but I change it as you suggested (we can also use trim-left to only allocate 0 or 1 extra strings :-)).

@joaotavora
Copy link
Owner

I see what you mean, yes, we could save a string allocation, but the current solution is "no worse" than what we currently have in that department. And my guess is it won't matter much. If you want to change it to trim-left (and know it works) it's fine, but I think this is also fine.

@joaotavora joaotavora merged commit f006162 into joaotavora:master Mar 10, 2022
@joaotavora
Copy link
Owner

OK, thanks! Now do you think you can look at eldoc.el and more or less pin-point where you think the former pyls problem was happening there? (and maybe propose a fix?)

@astoff
Copy link
Contributor Author

astoff commented Mar 10, 2022

All right, I can say 3 things: (1) passing a string with invisible attributes to message works perfectly fine, (2) if the server passes a hover of the form ```\nrest, it won't display in the echo area, (3) and this is because by the string passed to eldoc--message has already been trimmed to 1 line, hence it sees just ``` with invisible attributes coming from markdown.

I suspect the problem is in the line-counting logic of eldoc--echo-area-substring. There are calls to line-end-position there, probably replacing that with forward-visible-line solves the issue.

But actually, since pylsp apparently changed, it might make sense to wait and see if anyone complains again. I think servers should behave reasonably with editors with zero or crippled markdown rendering capabilities.

bhankas pushed a commit to bhankas/emacs that referenced this pull request Sep 18, 2022
…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
bhankas pushed a commit to bhankas/emacs that referenced this pull request Sep 19, 2022
…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
bhankas pushed a commit to bhankas/emacs that referenced this pull request Sep 19, 2022
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
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this pull request Oct 12, 2022
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
jollaitbot pushed a commit to sailfishos-mirror/emacs that referenced this pull request Oct 20, 2022
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hyperlinks from markdown hover text are unclickable
2 participants