elves / elvish

Powerful scripting language & versatile interactive shell
https://elv.sh/
BSD 2-Clause "Simplified" License
5.65k stars 299 forks source link

lsp: provide `doc:show` in hover, if available #1684

Closed sidkshatriya closed 1 year ago

sidkshatriya commented 1 year ago

I am new to elvish. I am enjoying elvish very much, thank you!

The built in LSP feature in elvish is very useful but a bit basic at the moment.

One thing that I am running into while reading or writing elvish scripts in an editor (where I am connected to a elvish -lsp) is that I constantly need to refer to the documentation about a particular function (e.g. printf etc) in order to learn the finer aspects of the function.

This means I need to constantly do doc:show in the shell or refer to the elvish website and this breaks my editing "flow".

This PR automatically shows the doc:show (if available) for anything on the cursor in a hover.

The implementation is quite simple and it could be more sophisticated but it's working quite well for me.

sidkshatriya commented 1 year ago

Here is a sample screenshot of the hover feature in action:

hover_in_action

sidkshatriya commented 1 year ago

@xiaq @krader1961 @zzamboni -- Would greatly appreciate a review and/or feedback on this feature... Thanks :-) !

xiaq commented 1 year ago

Hi, thanks for the contribution. I usually review PRs in a FIFO manner, but here are some high-level feedback:

Feel free to address those before I get back to this again, but this is a useful starting point and I wouldn't mind merging this as-is if you prefer.

sidkshatriya commented 1 year ago

Thanks !

The code to decide whether to look up doc is rather simplistic - for example, if you hover over the printf part of echo printf the doc of printf probably should not pop up.

Yes, this can definitely be improved.

Current algorithm

  1. Find a leaf node that has a range which includes the current cursor position
  2. Does the leaf node text have documentation? If so, simply return the documentation

This approach has some false positives like showing hover for echo printf that you pointed out.

Better Algorithm

  1. Find a leaf node that has a range which includes the current cursor position
  2. Once a leaf node is found, see if it is the correct type of node that could have documentation. We could use the parent property on a node to also traverse the tree upwards if this information lies at a higher level of the tree
  3. If the leaf node is found acceptable, see if there is any documentation. If so, return the documentation

I found the parsing/AST code a bit intimidating and was not sure how I should approach (2).

Can you provide some pointers for (2) ? Is there a pretty printer of some sort for the parse tree to get a feel of it ?

If you think it is too complicated to communicate tips for (2) then you could merge this commit as-is as you mentioned and improve upon this feature as you see fit.

xiaq commented 1 year ago

Can you provide some pointers for (2) ? Is there a pretty printer of some sort for the parse tree to get a feel of it ?

There is already one place that does it, the completer for commands: https://github.com/elves/elvish/blob/66a4c590eb5002ab560f5498e8c6cf1f3d3b401f/pkg/edit/complete/completers.go#L77

The code is very concise thanks to some abstractions defined in node_path.go, but the basic idea is walking upwards from the leaf, and see if (1) we get a sequence of Primary, Indexing, Compound and Form nodes, and (2) the Compound node is equal to the Head field of the Form node.