-
Notifications
You must be signed in to change notification settings - Fork 825
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
Specify input text in completion items #648
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:
|
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) |
You can specify a |
@KamasamaK, @mickaelistria how would the server communicate that the string it is matching is not contiguous? Say if the user has typed |
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 |
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 don't think a git would be more descriptive than "it just works" here.
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 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.
|
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: Can you say that your Eclipse LSP server supports this kind of completion?
Can you guarantee that that server is not using any out-of-spec fields? 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 |
Or if you prefer, just explain in plain english the relevant parts of the LSP requests and responses that support this. |
Thank you, it works. |
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
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
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: microsoft/vscode#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. |
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 (This isn't the only issue that arises from client-side filtering, as I tried to explain in #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).
Hmm... but |
@kdvolder Suppose it uses fuzzy matching, OK. When the editor inserts the completion, the text will become |
Not if you specify the edit range correctly to indicate that the completion should replace 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. |
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?)
This means that, in the current state of affairs, the
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. |
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.
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). |
@kdvolder I think this #898 (comment) is a good compelling example. Do you agree? In short: |
@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'. |
I want to let y'all know that I'm encountering this issue in the real world, working on fennel-ls. Emacs+Eglot decides the completion range differently than other editors, which causes them to filter out all the completion candidates. The I'm handling it right now by detecting eglot and providing different completions for it, but the promise of LSP is to avoid client-specific behavior. |
FWIW OTOH it's true that LSP simply doesn't have a way to tell what the thing at point is. |
@joaotavora That's the problem. There's nothing to complain about in fennel-mode, because |
As you see, client thinks input text is
co
, but server knows the really input text iscore.co
.So I hope the server can specify input text in the response of Completion Request.
The text was updated successfully, but these errors were encountered: