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

Unable to open hyperlinks in lsp-ui-doc #452

Closed
CSRaghunandan opened this issue Jun 1, 2020 · 22 comments · Fixed by emacs-lsp/lsp-mode#3844
Closed

Unable to open hyperlinks in lsp-ui-doc #452

CSRaghunandan opened this issue Jun 1, 2020 · 22 comments · Fixed by emacs-lsp/lsp-mode#3844
Labels

Comments

@CSRaghunandan
Copy link

When I click on any hyperlink in lsp-ui-doc buffers, I observe this in my messages bufffer:

Mark set
markdown-link-p: Beginning of buffer

And the link does not open in my browser.

Here is my environment info:

**Emacs version: GNU Emacs 28.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.17.3)
 of 2020-05-02, built using commit a941a9e8c226cff8eb77c4f8d7d0d54cb4a36340.

./configure options:
  --with-modules --with-rsvg --with-dbus --with-imagemagick --without-pop --with-xft --with-xml2 --with-libotf --with-mailutils

Features:
  XPM JPEG TIFF GIF PNG RSVG CAIRO IMAGEMAGICK SOUND GPM DBUS GSETTINGS GLIB NOTIFY INOTIFY ACL GNUTLS LIBXML2 FREETYPE HARFBUZZ LIBOTF ZLIB TOOLKIT_SCROLL_BARS GTK3 X11 XDBE XIM MODULES THREADS LIBSYSTEMD JSON PDUMPER LCMS2 GMP
**
@alanconway
Copy link

I've been trying to figure this out.

It appears that the doc frame contains a buffer in markdown mode, but it does not actually contain markdown text. The text has already been forcibly font-locked by lsp--fontlock-with-mode so the original markdown text has been replaced by only the text that is visible in gfm-view mode, with a bunch of font codes. Attempting to click on the link fails because it's looking for markdown link text which doesn't exist, and ends up back at the start of the buffer. The lsp-ui-doc--make-clickable-link code doesn't work because the original https:// link text is gone, it is hidden in gfm-view mode. All that's left is the font-marked anchor text.

Not sure how to fix it though...

@jcrossley3
Copy link

Could be related to #379

@sebastiencs
Copy link
Member

Could you show a screenshot of the documentation and the link ?
What LSP server are you using ?

I can click on a link in the documentation and it opens in my browser:
image

@alanconway
Copy link

alanconway commented Sep 10, 2020

@sebastiencs That works because the raw URL is in the text, so lsp-ui-doc--make-clickable-link can recognize it. The gopls server returns markdown with links like this:

Duration int64

[`time.Duration` on pkg.go.dev](https://pkg.go.dev/time#Duration)

A Duration represents the elapsed time between two instants
as an int64 nanosecond count\. The representation limits the
largest representable duration to approximately 290 years\.

The markdown rendering replaces the markdown link with just the font-locked anchor text, so there's no raw URL left to find. The rendered text isn't markdown at all, there are some font-lock hints left behind so markdown-mode tries to follow on a mouse click, but it searches for the original markdown [ANCHOR](URL) syntax which is no longer there.

@alanconway
Copy link

Here's a workaround:

  (defun markdown-raw-links (&rest ignore)
    "Convert link markup [ANCHOR](URL) to raw URL
     so lsp-ui-doc--make-clickable-link can find it"
    (save-excursion
      (goto-char (point-min))
      (while (re-search-forward markdown-regex-link-inline nil t)
          (replace-match (replace-regexp-in-string "\n" "" (match-string 6))))))
  (advice-add 'lsp--render-markdown :before #'markdown-raw-links)

This replaces the markdown link with the raw URL before rendering, so it gives this:

workaround

It would be better to get the rendering process to respect markdown links properly rather than post-processing in lsp-ui-doc to restore clickability only to raw URLs. Here's what it should look like - but this doesn't work if you click.

pretty

@sebastiencs
Copy link
Member

sebastiencs commented Sep 10, 2020

@alanconway I tried to setup the go environment with lsp-mode, but without success.
So I manually inserted the string you sent to test and the commit 79ba8b2 works for me.

Can you see if it works ?

@alanconway
Copy link

alanconway commented Sep 11, 2020

@sebastiencs Works perfectly! Thanks for the very rapid fix 👍

@yyoncho
Copy link
Member

yyoncho commented Sep 11, 2020

Seems like this should go in lsp-mode to handle lsp-describe-thing-at-point as well. Also, we should find a way to handle custom schemes like jdt:// .

@sebastiencs
Copy link
Member

we should find a way to handle custom schemes like jdt://

My guess would be to use another function than browse-url, but I'm not familiar with jdt scheme. What are they supposed to open ?

@alanconway
Copy link

Uninformed speculation here: lsp-doc-ui is sort-of-but-not-actually using markdown-mode. It's only using it for font locking in the render phase, then it displays a font-locked string in the ui-doc window. Is it possible to change that? Have the ui-doc window display a real markdown-mode buffer with real markdown text? It is a much bigger fix but it has bigger benefits - no more playing catch-up to re-implement lost markdown-mode features (link following may not be the only such problem), and less coupling to the innards of markdown mode (text-property names) so less likely to be broken by future markdown-mode versions. Also that all applies for free to any other display mode you might need to use in future.
No idea if that's feasible or a good idea, just thinking aloud - I'm happy with the proposed fix; it works. The above is more about future maintenance of lsp-mode - which I have an interest in 'cause it's great and I use it every day! Thanks

@yyoncho
Copy link
Member

yyoncho commented Sep 14, 2020

Is it possible to change that?

Yes, we need to introduce a second function in lsp-mode for doing that. The current function does this odd stuff because the overlays do not support handling invisible string so we kind of strip that upfront.

@yyoncho
Copy link
Member

yyoncho commented Sep 14, 2020

Actually, most likely we have to fix what we have how and use current logic only for the sideline.

@sebastiencs
Copy link
Member

@alanconway markdown-mode is used to format the documentation: It sets the colors, text size, font etc.
We then literally copy the text, with all the formatting (the font locking you mention), to our own buffer.
Visually, there should be no differences with a markdown-mode buffer.

Using a markdown-mode buffer will bring some issues:
markdown-mode has it's own keymap and set a lot of different hooks. In addition to the user customization to markdown-mode, it will sure interfere with lsp-ui-doc, which we will have no control.
For each version of markdown-mode, there might be a different keymap, different hooks...

@alanconway
Copy link

@alanconway markdown-mode is used to format the documentation: It sets the colors, text size, font etc.
We then literally copy the text, with all the formatting (the font locking you mention), to our own buffer.
Visually, there should be no differences with a markdown-mode buffer.

Using a markdown-mode buffer will bring some issues:
markdown-mode has it's own keymap and set a lot of different hooks. In addition to the user customization to markdown-mode, it will sure interfere with lsp-ui-doc, which we will have no control.
For each version of markdown-mode, there might be a different keymap, different hooks...

Yep - using the full mode would save you implementing link-following etc. but also drag in a lot of other stuff you don't need, which is risky. Your evaluation of the trade-off is certainly better informed than mine :) Keep up the good work, I depend on this tool to supplement my ageing memory!

Cheers,
Alan.

@wraithm
Copy link

wraithm commented Aug 4, 2021

It's frustrating. I can hover my mouse over a link, and it shows the url in the bottom, but there doesn't seem to be a way to actually browse to the url!

@wraithm
Copy link

wraithm commented Aug 4, 2021

It's frustrating. I can hover my mouse over a link, and it shows the url in the bottom, but there doesn't seem to be a way to actually browse to the url!

Oh! I found that lsp-ui links work great. It's the standard lsp-describe-thing-at-point that doesn't work.

@jcs090218
Copy link
Member

Can we close this issue? 😕

@alanconway
Copy link

Can we close this issue?
Yes - as far as I am concerned. The problem I experienced has been fixed for some time now.
I can't speak for others...

@kiennq
Copy link
Member

kiennq commented Aug 6, 2021

We should probably update lsp-describe-thing-at-point to support mouse click + keyboard follow on link too

@wraithm
Copy link

wraithm commented Aug 6, 2021

We should probably update lsp-describe-thing-at-point to support mouse click + keyboard follow on link too

💯 That would be great. I tend to not use lsp-ui-doc. Does this call for a bug report at the main lsp repo? I would write it, but I feel like others would be better able to provide clarity on how to fix it, given that it's fixed here in lsp-ui.

@kiennq
Copy link
Member

kiennq commented Aug 6, 2021

Does this call for a bug report at the main lsp repo?

Yes, please file a bug/feature request there.

Btw, calling M-x lsp-ui-doc--open-markdown-link on lsp-describe-thing-at-point buffer will also open the links too.

@unhammer
Copy link

Can we close this issue?
Yes - as far as I am concerned. The problem I experienced has been fixed for some time now.
I can't speak for others...

Still an issue for me, on

  • lsp-ui-doc version: 8.0.0, commit: b625f3c
  • GNU Emacs 28.0.50 (build 3, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw3d scroll bars) of 2021-05-26

Maybe it's because the link itself defines a keymap (at least in lsp-haskell). If I focus the frame and C-h c I get a text property keymap which binds mouse-2 to markdown-follow-link-at-point, which seems not overridden by remapping.

yyoncho pushed a commit to emacs-lsp/lsp-mode that referenced this issue Dec 16, 2022
… buffers (#3844)

* Fix markdown link navigation in *lsp-help* buffers

  Fix emacs-lsp/lsp-ui#452

* Treat all markdown link types the same

  - markdown-link-face, markdown-url-face and markdown-plain-url-face

* Inhibit read-only in lsp--fix-markdown-links

  for Emacs27

* Use markdown-find-next-prop instead of text-property-search-forward

 which is missing on Emacs26

* Start searching from point-min in lsp--fix-markdown-links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants