search

emacs-lsp / lsp-ui

UI integrations for lsp-mode
https://emacs-lsp.github.io/lsp-ui
GNU General Public License v3.0
1.04k stars 141 forks source link

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

Closed CSRaghunandan closed 1 year ago

CSRaghunandan commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Could be related to #379

sebastiencs commented 4 years ago

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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

@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 https://github.com/emacs-lsp/lsp-ui/commit/79ba8b2a901d079f5f65f87735329af400dfe692 works for me.

Can you see if it works ?

alanconway commented 4 years ago

@sebastiencs Works perfectly! Thanks for the very rapid fix :+1:

yyoncho commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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

sebastiencs commented 4 years ago

@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 commented 4 years ago

@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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Can we close this issue? 😕

alanconway commented 3 years ago

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 commented 3 years ago

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

wraithm commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

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.