search

autozimu / LanguageClient-neovim

Language Server Protocol (LSP) support for vim and neovim.
MIT License
3.55k stars 273 forks source link

Add support for rust-analyzer type & parameter hints #1221

Closed Logarithmus closed 3 years ago

Logarithmus commented 3 years ago

Temporary workaround for https://github.com/autozimu/LanguageClient-neovim/issues/934, until neovim anticonceal https://github.com/neovim/neovim/pull/9496 is finished. ChainingHints - Iterator<u32> TypeHints - : u32 ParameterHints - 𝑓(a, b, c)

Demo:

image

martskins commented 3 years ago

Nice! Very clean and readable! :+1:

martskins commented 3 years ago

I'm not sure the prefixes for the different hints are easy to remember. I was going back and forth between your comment here on this PR and the screenshot to understand what each hint meant. I'm thinking if maybe surrounding the type parameters with parentheses and leaving the others without a prefix would be clearer ? (and keep the pipe in between the hints)

Nice work though, and thank you for contributing!

Logarithmus commented 3 years ago

I'm not sure the prefixes for the different hints are easy to remember. I was going back and forth between your comment here on this PR and the screenshot to understand what each hint meant. I'm thinking if maybe surrounding the type parameters with parentheses and leaving the others without a prefix would be clearer ? (and keep the pipe in between the hints)

The problem is that Github interpreted those arrows as emojis.

My logic behind those prefixes is quite simple (hopefully, it makes sense):

I propose the following alternative scheme: ChainingHints - simply Iterator<u32> TypeHints - : u32 ParameterHints - 𝑓(a, b, c)

What do you think?

martskins commented 3 years ago

I propose the following alternative scheme: ChainingHints - simply Iterator TypeHints - : u32 ParameterHints - 𝑓(a, b, c)

What do you think?

I think that would be better yeah. Will need to try it out myself to see if it's clearer (to me of course) but it does seem sensible at first sight.

Logarithmus commented 3 years ago

@martskins I've performed the changes from https://github.com/autozimu/LanguageClient-neovim/pull/1221#discussion_r629232168

martskins commented 3 years ago

So I've given this a try, there's a couple things that I noticed.

The first one, is that if you have nested function calls, then the parameter hints get confusing. See line 38 in the first screenshot (from vim) here. I'm not sure how I would display this, maybe something like f(reader: f(inner)) ? Not entirely convinced, but something like that.

The other thing is that this seems to be hitting a limit in the number of virtual texts. See line 46 in the first screenshot, that same line has a hint in the second screenshot (from VSCode).

Lastly, I think it would be nice to prefix the type parameters with the name of the variable, much like coc.nvim does. So instead of showing : Sender<Call>, Receiver<Call>, it would be much nicer to show something like this tx: Sender<Call>, rx: Receiver<Call>. I suppose this is something we can get from rust-analyzer too, but I haven't checked.

On your comment about displaying them by default, I agree. If rust-analyzer enables them by default then it makes perfect sense for us to do the same.

Again thank you for working on this, much appreciated.

Screenshot_2021-05-13_09-22-00 Screenshot_2021-05-13_09-22-22

Logarithmus commented 3 years ago

@martskins I'm not planning to work on this PR anymore, because neovim 0.5.0 was finally released and someday (when finally have free time) I'll migrate my config to its built-in language server support & https://github.com/hrsh7th/nvim-compe. If anyone interested in continuing this PR, feel free to open your own one based on mine.