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

extract signature for rust-analyzer #1740

Closed
wants to merge 1 commit into from
Closed

extract signature for rust-analyzer #1740

wants to merge 1 commit into from

Conversation

scturtle
Copy link
Contributor

@scturtle scturtle commented Jun 1, 2020

Add specific lsp-clients-extract-signature-on-hover function for rust-analyzer.

@yyoncho yyoncho requested a review from brotzeit June 2, 2020 14:28
@brotzeit
Copy link
Member

brotzeit commented Jun 2, 2020

@flodiebold can you help me out ?

@flodiebold
Copy link
Contributor

I don't really know what this does or why it's needed 🤔

@scturtle
Copy link
Contributor Author

scturtle commented Jun 3, 2020

The problem these lines of code want to solve is that hover in lsp-mode hardly show useful signature with rust-analyzer.

截屏2020-06-03 08 12 59

截屏2020-06-03 08 15 13

Yes I know where this struct/function come from, please just show me the definition.

The value in response from rust-analyzer is like:

"value": "```rust\nkecc::opt::simplify_cfg\n```\n\n```rust\npub struct SimplifyCfgConstProp\n```\n___\n\nSimplifies block exits by propagating constants.",

or:

"value": "kecc::opt::simplify_cfg\n___\n\n```rust\npub struct SimplifyCfgConstProp\n```\n\nSimplifies block exits by propagating constants.",

On its first line it often shows where this struct/function come from and the format is casual and different between versions of rust-analyzer.

Yes lsp-eldoc-render-all can be turned on but I just need one line of signature like other lsp clients. Document is quite long for stdlib function.

In lsp-mode, lsp--render-on-hover-content calls lsp--handle-rendered-for-echo-area and use generic function lsp-clients-extract-signature-on-hover to extract one line of signature. If there is no specific function like this clangd one the default function will be use and only show the first rendered line which causes the problem above.

This PR extract the first two markdown code block from the response value and prefer the second one. Yes his heuristic is wield and I hope anyone could come out a better solution.

@flodiebold
Copy link
Contributor

This seems very coupled to the exact layout of the hover response though, which could easily change. Maybe it would be possible to add a setting to RA that makes it only return the signature -- you're not using the full hover response in another place with this, right?

@yyoncho
Copy link
Member

yyoncho commented Jun 4, 2020

FWIW if the server uses MarkedString this issue won't exist. I think it is now deprecated but IMO it is much more useful for clients like emacs which want to show minimal information.

@scturtle
Copy link
Contributor Author

Well another problem: lsp--hover-saved-bounds often caches ridiculous large range wrong result and make hover not usable any more.

@yyoncho
Copy link
Member

yyoncho commented Jun 21, 2020

Well another problem: lsp--hover-saved-bounds often caches ridiculous large range wrong result and make hover not usable any more.

Can you elaborate?

@scturtle
Copy link
Contributor Author

Can you elaborate?

If you query the hover request when rust-analyzer is processing some new changes, it will return ridiculous result like:

[Trace - 01:16:39 PM] Received response 'textDocument/hover - (874)' in 3ms.
Result: {
  "range": {
    "start": {
      "line": 86,
      "character": 66
    },
    "end": {
      "line": 297,
      "character": 5
    }
  },
  "contents": {
    "value": "```rust\nbool\n```",
    "kind": "markdown"
  }
}

And then a large range of source code is shadowed by saved bounds result.

@danielmartin
Copy link
Contributor

What's the status of this PR? It needs a rebase as there are some conflicts. I think it's a good idea to try to extract a Rust signature line for eldoc, but unfortunately we need to rely on some heuristics. Do you know if there's a better way to do this?

FWIW if the server uses MarkedString this issue won't exist. I think it is now deprecated but IMO it is much more useful for clients like emacs which want to show minimal information.

Yes, that's a bad thing. I think LSP should separate signature info from documentation so that they could be shown separately.

@yyoncho
Copy link
Member

yyoncho commented Jun 29, 2020

Do you know if there's a better way to do this?

The alternative approach is to get N symbols from the output by default and let users on their own to customize that with a function(which you already provided).

@scturtle
Copy link
Contributor Author

What's the status of this PR? It needs a rebase as there are some conflicts.

I agree with the maintainer that there need be a better way. This PR is what I can do so far.
I hope to leave this PR open for somebody finding it useful. That function can be put into one's config.el and not necessarily lsp-rust.el.
Along with disabling lsp--hover-saved-bounds (to work around the rust-analyzer bug mentioned above), it works quite good for me at the time.

@jiacai2050
Copy link

jiacai2050 commented Jul 4, 2020

@scturtle I copied your PR to my lsp-rust.el file, and restart Emacs, but nothing changed, is there anything I'm missing?

image


[Trace - 01:12:18 AM] Received response 'textDocument/hover - (19)' in 3ms.
Result: {
  "range": {
    "start": {
      "line": 40,
      "character": 6
    },
    "end": {
      "line": 40,
      "character": 10
    }
  },
  "contents": {
    "value": "```rust\nalloc::vec::Vec\n```\n\n```rust\npub fn push(&mut self, value: T)\n```\n___\n\nAppends an element to the back of a collection.\n\n# Panics\n\nPanics if the number of elements in the vector overflows a `usize`.\n\n# Examples\n\n```rust\nlet mut vec = vec![1, 2];\nvec.push(3);\nassert_eq!(vec, [1, 2, 3]);\n```",
    "kind": "markdown"
  }
}

Test with rust-analyzer ca31b1d

@scturtle
Copy link
Contributor Author

scturtle commented Jul 5, 2020

@jiacai2050 Try deleting lsp-rust.elc or recompile it with emacs --batch --eval '(byte-recompile-directory "." 0)'.

@jiacai2050
Copy link

@scturtle No luck.
I also try build lsp-mode from source, apply your changes, and cask build, but hove message doesn't change at all.

@jiacai2050
Copy link

jiacai2050 commented Jul 5, 2020

image

After set lsp-eldoc-render-all nil, hover message display signature! But minibuffer is still too large, with many blank lines.
I try eval lsp--eldoc-saved-message in current buffer, it return

("pub fn push(&mut self, value: T)" 0 3 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-keyword-face markdown-code-face) markdown-gfm-code (9 42)) 3 4 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 4 6 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-keyword-face markdown-code-face) markdown-gfm-code (9 42)) 6 7 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 7 11 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-function-name-face markdown-code-face) markdown-gfm-code (9 42)) 11 12 (font-lock-multiline t fontified t font-lock-fontified t face (rainbow-delimiters-depth-1-face markdown-code-face) markdown-gfm-code (9 42)) 12 13 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 13 16 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-keyword-face markdown-code-face) markdown-gfm-code (9 42)) 16 17 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 17 21 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-keyword-face markdown-code-face) markdown-gfm-code (9 42)) 21 23 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 23 28 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-variable-name-face markdown-code-face) markdown-gfm-code (9 42)) 28 30 (face (markdown-code-face) font-lock-multiline t fontified t font-lock-fontified t markdown-gfm-code (9 42)) 30 31 (font-lock-multiline t fontified t font-lock-fontified t face (font-lock-type-face markdown-code-face) markdown-gfm-code (9 42)) 31 32 (font-lock-multiline t fontified t font-lock-fontified t face (rainbow-delimiters-depth-1-face markdown-code-face) markdown-gfm-code (9 42)))

Any ideas?

@scturtle
Copy link
Contributor Author

scturtle commented Jul 5, 2020

@jiacai2050 Try replacing (lsp--render-element (concat "```rust\n" sig "\n```")) with just sig?

@jiacai2050
Copy link

@scturtle After pull latest commit(7ce0d78), eldoc minibuffer only display one line! Thanks for your help.

@jiacai2050
Copy link

@brotzeit It's kinds of stale. perhaps we can close this PR, and document this. Let users decide how to extract siganature.
cc @scturtle

@chaoky
Copy link

chaoky commented Dec 28, 2020

did something change? this stopped working for me :\

@scturtle
Copy link
Contributor Author

@chaoky https://github.com/scturtle/dotfiles/blob/f1e087e247876dbae20d56f944a1e96ad6f31e0b/doom_emacs/.doom.d/config.el#L74-L85

@gagbo
Copy link
Contributor

gagbo commented Nov 17, 2022

@chaoky https://github.com/scturtle/dotfiles/blob/f1e087e247876dbae20d56f944a1e96ad6f31e0b/doom_emacs/.doom.d/config.el#L74-L85

Would you mind if we upstreamed this in Doom? That looks like a very very useful snippet!

@scturtle
Copy link
Contributor Author

scturtle commented Nov 18, 2022

disabling lsp--hover-saved-bounds (to work around the rust-analyzer bug mentioned above)

I found a easy way to do this by adding this line in config.el:

(advice-add #'lsp-hover :after (lambda () (setq lsp--hover-saved-bounds nil)))

(advice-add #'lsp-eldoc-function :after (lambda (&rest _) (setq lsp--hover-saved-bounds nil)))

It clears the cached bounds after every call to lsp-hover.

@dgutov
Copy link
Contributor

dgutov commented Oct 23, 2023

Now that this showed up on Reddit, and Rust-Analyzer is the official and preferred LS for Rust (since 2022), maybe it would be a good idea to revisit this PR?

@scturtle
Copy link
Contributor Author

scturtle commented Oct 24, 2023

Just in case anyone need it, here is a revised version that fix issue with lsp-use-plists:

(with-eval-after-load 'lsp-rust
  ;; do not cache the shitty result from rust-analyzer
  (advice-add #'lsp-eldoc-function :after (lambda (&rest _) (setq lsp--hover-saved-bounds nil)))
  ;; extract and show short signature for rust-analyzer
  (cl-defmethod lsp-clients-extract-signature-on-hover (contents (_server-id (eql rust-analyzer)))
    (let* ((value (if lsp-use-plists (plist-get contents :value) (gethash "value" contents)))
           (groups (--partition-by (s-blank? it) (s-lines (s-trim value))))
           (mod-group (cond ((s-equals? "```rust" (car (-fifth-item groups))) (-third-item groups))
                            ((s-equals? "```rust" (car (-third-item groups))) (-first-item groups))
                            (t nil)))
           (cmt (if (null mod-group) "" (concat " // " (cadr mod-group))))
           (sig-group (cond ((s-equals? "```rust" (car (-fifth-item groups))) (-fifth-item groups))
                            ((s-equals? "```rust" (car (-third-item groups))) (-third-item groups))
                            (t (-first-item groups))))
           (sig (->> sig-group
                     (--drop-while (s-starts-with? "```" it))
                     (--take-while (not (s-equals? "```" it)))
                     (--map (s-replace-regexp "//.*" "" it))
                     (--map (s-trim it))
                     (s-join " "))))
      (lsp--render-element (concat "```rust\n" sig cmt "\n```"))))
)

@pallix
Copy link

pallix commented Mar 11, 2024

This last function should be part of the mode? We can see how popular it is with the emojis and I have seen many user being confused by getting only the alignment from the type instead of the type in their mode-line when using Rust and lsp.

slotThe added a commit to slotThe/lsp-haskell that referenced this pull request Aug 17, 2024
This fixes a case where a type signature would be broken up over
multiple lines, and we only display the first of these.

Fixes: emacs-lsp#151
Related: emacs-lsp/lsp-mode#4362
Related: emacs-lsp/lsp-mode#1740
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 this pull request may close these issues.

10 participants