microsoft / language-server-protocol

Defines a common protocol for language servers.
https://microsoft.github.io/language-server-protocol/
Creative Commons Attribution 4.0 International
10.91k stars 764 forks source link

Pass the current selection for a Hover Request #377

Open alanz opened 6 years ago

alanz commented 6 years ago

In some languages, such as Haskell, it is meaningful to show the type of an expression, rather than just an identifier.

To allow this, extend the Hover Request to also include a Range if the selection is active.

At the moment in the haskell vscode plugin this is done in a roundabout way using workspace/executeCommand, which means it has to be separately implemented in each client.

rictic commented 6 years ago

+1, I've also wanted this functionality in TypeScript. The current workaround I've used is to declare a local const with the expression to get the type.

rcjsuen commented 6 years ago

Can someone explain this feature to me with a TypeScript/JavaScript/Java example?

Also, what happens if the hover is not over the selection in the editor?

rictic commented 6 years ago

Suppose a user wonders what the type of 'foo' + 'bar' is in typescript. Is it 'foobar' or is it string?

One option is that the user can do: const tempVar = 'foo' + 'bar'; and then hover over tempVar. Under this proposal, the user could select the text 'foo' + 'bar' in their editor, then mouse over any part of the selection. The selection would then be sent up to the typescript LSP server as part of the hover request, allowing it to return the type of 'foo' + 'bar' as a whole.

Other motivating expressions:

await Promise.all([foo, bar, baz])
foo.then((f) => f.bar())
foo === undefined ? null : bar
vavans commented 6 years ago

I had to implement this in the haskero plugin too. (As it communicates throughout the language server protocol). In many languages this feature would be great.

@alanz i should try to tweak my haskero plugin to use your language server, it could be nice

dbaeumer commented 6 years ago

It sounds reasonable to me to include the range in the Hover request. Anyone willing to work on a PR to add this. If so please also check if this can be added to the VS Code client LSP library (https://github.com/Microsoft/vscode-languageserver-node)

rcjsuen commented 6 years ago

@dbaeumer What would be added to Microsoft/vscode-languageserver-node? You mean update the protocol to reflect the new additional parameter in the request? It's not possible to make any client changes unless VS Code supports this given that HoverProvider needs to be augmented to include selection information.

/**
 * The hover provider interface defines the contract between extensions and
 * the [hover](https://code.visualstudio.com/docs/editor/intellisense)-feature.
 */
export interface HoverProvider {

    /**
     * Provide a hover for the given position and document. Multiple hovers at the same
     * position will be merged by the editor. A hover can have a range which defaults
     * to the word range at the position when omitted.
     *
     * @param document The document in which the command was invoked.
     * @param position The position at which the command was invoked.
     * @param token A cancellation token.
     * @return A hover or a thenable that resolves to such. The lack of a result can be
     * signaled by returning `undefined` or `null`.
     */
    provideHover(document: TextDocument, position: Position, token: CancellationToken): ProviderResult<Hover>;
}

I think to move forward with this we need to decide on three things:

  1. What do we call the new request's parameter's interface? Currently, it's a TextDocumentPositionParams but there will be new selection information. When textDocument/completion was changed CompletionParams extends TextDocumentPositionParam was added. Do we just call it HoverParams? It seems more generic than that though. I don't think we want to just add a new parameter to TextDocumentPositionParams anyway...or do we?

  2. What do we call the new parameter? range or selection or activeSelection or something else?

  3. In the capabilities, a new clientCapabilities.textDocument.completion.context was added for context support in textDocument/completion. Is something like this needed for this new hover "capabilitiy"? If yes, what do we call it?

jdneo commented 4 years ago

I hope the CompletionParams can also have the selection information.

TonalidadeHidrica commented 3 years ago

Any updates on this? I want the "get type of the expression" for rust-analyzer, but this issue seems to be blocking it.

rgrinberg commented 3 years ago

This would be quite useful for ocaml-lsp-server as well.

matklad commented 2 years ago

rust-analyzer adds this as an extension:

Hover Range

Experimental Server Capability: { "hoverRange": boolean }

This extension allows passing a Range as a position field of HoverParams. The primary use-case is to use the hover request to show the type of the expression currently selected.

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Range | Position;
}

Whenever the client sends a Range, it is understood as the current selection and any hover included in the range will show the type of the expression if possible.

https://github.com/rust-analyzer/rust-analyzer/blob/406eeb39bec3d81f924e31d33d6d4a40ae670f30/docs/dev/lsp-extensions.md#hover-range

Turns out, it's very straightforward to just support this in VS Code as well:

https://github.com/rust-analyzer/rust-analyzer/pull/9693/files#diff-bca0ed357f47caaadd008edc9b2df19f36869583b78c2c0265b9f0db3650df2eR59

rchl commented 2 years ago

What does the client send as position when a range is selected but cursor is not on the range? Just a Position, I presume?

flodiebold commented 2 years ago

The current implementation just sends the position, yes.

dbaeumer commented 2 years ago

An implementation IMO must look like this:

predragnikolic commented 2 years ago

FWIW, I wouldn't mix the Range type with the position:

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Range | Position;
}

instead I would add a optional range property:

interface HoverParams extends WorkDoneProgressParams {
    textDocument: TextDocumentIdentifier;
    position: Position;
    range?: Range;
}

The server can then return whatever it wants based if the range is present or not.

santiweight commented 1 year ago

I don't have experience with LSP nor TS enough to contribute in the near term, but this submission would be of great benefit to Haskell's LSP server!