Language-server-protocol: Specify input text in completion items

Created on 28 Dec 2018  Â·  26Comments  Â·  Source: microsoft/language-server-protocol

image

As you see, client thinks input text is co, but server knows the really input text is core.co.
So I hope the server can specify input text in the response of Completion Request.

clarification completion feature-request

Most helpful comment

So, while in general I am really a proponent of making the 'input text range' explicit and _separate_ from the completion's 'edit range'... it is actually quite hard to find good compelling examples for this.

@kdvolder I think this https://github.com/microsoft/language-server-protocol/issues/898#issuecomment-594063359 is a good compelling example. Do you agree?

In short: foo.ba is getting completed to foo["bar bar"]. The "input text" does not include the dot, but the "edit range" does

All 26 comments

+1

In ycmd's protocol, the server tells the client the column at which completion should start (as Vim and can't support arbitrary text edits for completion). For the YCM LSP implementation, our LSP client has to try and guess, as the behaviour of overlapping text is pretty much unspecified in the protocol:

The insertText is subject to interpretation by the client side.
* Some tools might not take the string literally. For example
* VS Code when code complete is requested in this example con<cursor position>
* and a completion item with an insertText of console is provided it
* will only insert sole.

Interesting. Just realized this is actually noticeable in the example given for Microsoft/vscode-docker#277 although not really related to the fix itself.

Just realized this is closely related to #651. In that issue I provide an alternative way to fix this, where the server says what characters of the completion it is actually matching.

I don't think it's a protocol issue; as it seems to me that for other clients (such as Eclipse IDE for instance) core.co would be highlighted as match. The client already has everything necessary to improve this, and I don't think the protocol is to blame here, it seems more like a plain bug in VSCode that should better detect the string overlap between proposed edit and current code.

You can specify a textEdit in completion items, and its range should be be sufficient to determine "input text". This can be necessary when including word boundaries in completion items. I too do not believe this is an issue with the protocol.

@KamasamaK, @mickaelistria how would the server communicate that the string it is matching is not contiguous? Say if the user has typed co.let to match core.completion. Is that possible with text edits in completion responses too?

Is that possible with text edits in completion responses too?

At least Eclipse IDE support for LSP is able to do it without any protocol extension. Try Eclipse Wild Web Developer and open a .json file which has a json schema containing something like last.name and you'll see la.na shows last.name as proposal and highlight la.na in the proposal.

Try Eclipse Wild Web Developer and open a .json file

I'm unable to do that now. Can you perhaps post a small gif recording of that interaction here? I am also interested in the traffic of LSP messages that enable this, if you can capture them. Eclipse JDT server is known for doing some out-of-spec things. But if it's within the current spec, I'd love to know how.

I'm unable to do that now. Can you perhaps post a small gif recording of that interaction here?

I don't think a git would be more descriptive than "it just works" here.

I am also interested in the traffic of LSP messages that enable this, if you can capture them.

it's a completion request/completion response, just like VSCode or any LSP client. The Language Server that's used is the JSon language server from VSCode.

Eclipse JDT server is known for doing some out-of-spec things

Eclipse JDT is not involved here, it's only the client part that implements substring detection better than VSCode apparently; but again, only relying on things that are already available.

But if it's within the current spec, I'd love to know how.

https://git.eclipse.org/c/lsp4e/lsp4e.git/tree/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSIncompleteCompletionProposal.java#n209

I don't think a git would be more descriptive than "it just works" here.

You suggested I install in my system a large piece of software with lots of depedencies, isn't it simpler just to record a simple gif so we can ensure we're talking about the same thing.

But OK, if that's too hard, here's a GIF from an IDE that does what I'm looking for:

image

Can you say that your Eclipse LSP server supports this kind of completion?

it's a completion request/completion response, just like VSCode or any LSP client. The Language Server that's used is the JSon language server from VSCode.

Can you guarantee that that server is not using any out-of-spec fields?

https://git.eclipse.org/c/lsp4e/lsp4e.git/tree/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSIncompleteCompletionProposal.java#n209

Don't make me go read your Java code (I'm not telling you to go read Emacs-lisp either), there are many abstractions and data types outside that function that I need to understand before I can make sense out of the function. LSP is language-agnostic: if you're arguing LSP is sufficient here please show the LSP traffic for your last.name example.

please show the LSP traffic for your last.name example.

Or if you prefer, just explain in plain english the relevant parts of the LSP requests and responses that support this.

You can specify a textEdit in completion items, and its range should be be sufficient to determine "input text". This can be necessary when including word boundaries in completion items. I too do not believe this is an issue with the protocol.

Thank you, it works.

You can specify a textEdit in completion items, and its range should be be sufficient to determine

This is just a 'happy' coincidence but there really isn't any fundamental reason why this should be the input text.

Say I type 'foo' and the completion is 'foobar'. It could be represented equivalently by a server as

  • "insert 'bar' after 'foo'.
  • "replace 'foo' with 'foobar'.

Both these edits will result in the same end result. However the range in the first completion is empty (so input text is empty) and the range in the second one is 'foo'.

So just to add my '+1' and explanation why I think it is best to have the server explitly indicate the 'input text'.

In my opinion the arguments on not doing / needing this information explicitly basically boil down saying that the client can 'guess' the input text from available information. (E.g. the range if the main edit, as vscode does or some fancy text matching as eclipse does).

And it is true this works fine in most cases, however

1) it remains a heuristic. The client is only guessing what part of the document the server took to be as 'input text' and it is certainly possible the client's heuristics gets it wrong.

2) the heuristics are or can be non trivial and so are a burden to implement

3) the heuristics are not spec-ed and so not consistently implemented the same on different clients.

Re point 1), besides the 'foobar' example above, I have another real example of this. Our language server intenionally included some spaces in front of the input text in the main edit range. The purpose of this was to 'de-dent' a proposal (by deleting some spaces in front of the input text). However the spaces really are not part of the 'input text' (just stuff in front of the input text that we want to delete). These proposals threw off vscode's 'guess' of the input text (it included the spaces) which in turn messed up sorting of proposals.

See: https://github.com/Microsoft/vscode/issues/26096 for details.

Vscode resolved this issue by adding some special case rules around (not) including spaces at the start of the input text.

So to summarise: My point is, sure... you can 'guess' the input text correctly in most/many cases. But why guess at all? When it would typically be very easy for a server to explicitly indicate what section of the document it actually used as 'input text'.

I do agree that this needs to be clarified. However assuming that we find a solution for the unintended problem I think specifying this via the range of a text edit or via an additional property has the same expressiveness.

I think specifying this via the range of a text edit or via an additional property has the same expressiveness

Only if you assume the range of the text edit matches the input text. This is not guaranteed and you cannot always make it so. This is because the 'text edit range' and the 'input text' are different things that have different meaning.

The range of the text edit is the text you want to replace, whereas the 'input text' is the text you use for filtering matches. Now, yes, it is true these are usually the same, but that is really just a coincidence. It all depends on what effect you want to have when a completion is applied, whether you can make the edit range be equal to the text you range you used for filtering completions.

Notice another consequence of this issue: If the user wants to complete

require 'core.fo|'

(with the cursor indicated by |) and the server returns core.foobar as a candidate, the editor will filter it out, since the presumed input text, fo, doesn't match core.foobar.

(This isn't the only issue that arises from client-side filtering, as I tried to explain in https://github.com/microsoft/language-server-protocol/issues/898)

@astoff don't you have a typo there? Or am I missing something? Also, where is the user's cursor in that moment?

@joaotavora Should be correct now :-)

In general I am in agreement, however I'm not sure this example @astoff is a good one (or perhaps I am misunderstanding it).

the presumed input text, fo, doesn't match core.foobar.

Hmm... but fo does match with core.foobar since fo is a substring of 'core.foobar'. At least in vscode, I think that counts as a match.

@kdvolder Suppose it uses fuzzy matching, OK.

When the editor inserts the completion, the text will become core.core.foobar.

When the editor inserts the completion, the text will become core.core.foobar.

Not if you specify the edit range correctly to indicate that the completion should replace core.fo -> core.foobar.

So, while in general I am really a proponent of making the 'input text range' explicit and separate from the completion's 'edit range'... it is actually quite hard to find good compelling examples for this.

but fo does match with core.foobar since fo is a substring of 'core.foobar'. At least in vscode

Fine. Is there any genuine reason to do this kind of substring matching, or is it just a workaround for cases like the above?

If it's a workaround, then it has a simple solution: provide the completion prefix/bounds/subject in the response.

If it is a genuine feature, that's OK. But other editors may have a different idea. For instance, if you allow substring matches, sorting becomes more complicated (what sorts higher as a completion of foo: afoo or foobar?)

When the editor inserts the completion, the text will become core.core.foobar.

This means that, in the current state of affairs, the textEdit field of a completion item is essentially mandatory to ensure correct operation.

it is actually quite hard to find good compelling examples for this

If the server could convey to the client that no filtering should happen, then I agree that the only reason to give the completion subject is cosmetic — so the editor can place a popup box right below the completion subject, highlight the match etc. If there is client-side filtering, the prefix information is essential.

Fine. Is there any genuine reason to do this kind of substring matching

I don't know about you but I like the fuzzy matches. I can type some memorable part of the string and not bother typing the sometimes very long common prefix that is shared by a lot of other things in the same package/namespace.

This means that, in the current state of affairs, the textEdit field of a completion item is essentially mandatory to ensure correct operation.

Agreed. And our servers always include them precisely because just using insertText alone just seems too ambiguous to me as to what that means (both in terms of the 'effect' that happens when the completion is invoked, as well as it's 'filtering' semantics).

So in that sense, I should change my statement that its not a good example. I think now... it is a good example... if we explicitly assume the completion uses simple 'insertText' rather than a textEdit with a range.

I think fuzzy matching is good, but we can't rely on it (there is a certain fraction of users who avoid it).

So, while in general I am really a proponent of making the 'input text range' explicit and _separate_ from the completion's 'edit range'... it is actually quite hard to find good compelling examples for this.

@kdvolder I think this https://github.com/microsoft/language-server-protocol/issues/898#issuecomment-594063359 is a good compelling example. Do you agree?

In short: foo.ba is getting completed to foo["bar bar"]. The "input text" does not include the dot, but the "edit range" does

@astoff Yes I agree. It is in some ways also quite similar to our own examples which I have presented a long while ago (sorry no link... its burried somewhere in the long threads of multiple similar issues raised around this, so its kind of hard to find it now). But in short our example is for a yaml editor and it is similar to the 'delete the .' example... in that it ours wants to delete some spaces in front of the 'input text' to make sure the completion is inserted with a correct indentation.

I think in both cases what we have is a desire to adjust some 'punctuation' characters near the 'input text', to ensure 'correct' syntactic structure after the expansion. But these characters are not considered as part of the 'input text'.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

pajatopmr picture pajatopmr  Â·  3Comments

marekdedic picture marekdedic  Â·  5Comments

ljw1004 picture ljw1004  Â·  3Comments

lanza picture lanza  Â·  6Comments

kaloyan-raev picture kaloyan-raev  Â·  6Comments