eclipselabs/eclipse-language-service

[Completion] Filter completion items result according current word.

Closed this issue · 24 comments

It seems that CSS server doesn't filter completion items by using current word. The LSContentAssistProcessor.toProposals should filter items.

csscompeltionpb

I get the same thing with VSCode, so it's an issue in the language server (VSCode).

Are you really sure? VSCode displays :default because it contains a but NOT :checked.

Ok, so in VSCode, completion for a doesn't show :checked whereas completion for a: shows checked. There seems to be some filtering we could use there, however if the necessary data is not part of the language server responses, it's not something we can use here.

I think we must have the same thing thatn VSCode, in other words a LanguageConfiguration https://github.com/Microsoft/vscode/blob/master/src/vs/vscode.d.ts#L2585 for client side to support wordPattern (usefull here in this issue to retrieve the word) and after support indentation, bracket, etc

But perhaps LanguageConfiguration should be a part of GenericEditor?

The Generic editor should allow provide similar featurs to LanguageConfiguration. However, I don't see how this is related to the issue about completion results and filtering here.

To filter items, we need to retrieve the word which depends on the language. For CSS if you open completion after :

  • a => here word is a
  • a: => here word is blank

To manage that, I think VSCode uses LanguageConfiguration#wordPattern.

and then what do we do once we have the word? Does the language server offer a way to filter completion results according to it? Are you sure the filtering isn't something we can improve directly in the language server integration with what the Completion operation already provides?

An other solution to retrieve word according language is that org.eclipse.languageserver.operations.completion.LSContentAssistProcessor provide a default method getWord and after for CSS, we create a new class which extends org.eclipse.languageserver.operations.completion.LSContentAssistProcessor to override getWord.

What do you think about that?

If we want the language server integration to be generic, it means that we should avoid as much as possible having people subclassing it. Subclassing is the opposite of what we want to achieve with generic editors and language servers, which are based on composition.

Ok I don't know very well specification of Language Server but I can tell you about tsserver from TypeScript and it seems Language Server of CSS has the same behaviour than tsserver.

Takes a sample with typescript.java which consumes tsserver:

var s = "";
s.ch // Ctrl+Space to open completion

When you open completion, typescript.java shows the whole methods of string type which contains ch:

typescriptcompletion

But in reallity, tsserver returns the whole method of string even if it doesn't contain ch like (concat, indexOf, etc). So typescript.java needs to filter thoses methods.

Why this behaviour?

It's because you let the client to manage filter like you wish, you can filter with:

  • starts with word. In this sample, only charAt and charCodeAt will be displayed.
  • or contains exactly the word.
  • or (a feature that I love) which contains some characters of the word like

typescriptcompletionadvanced

So client can manage several preferences filter with the same tsserver.

To do that we need to retrieve the word from the content:

s.ch // here TypeScript word is ch

Word depends on the language.

and then what do we do once we have the word?

Like I have tried to explain you, word is used to filter items according a filter strategy (in typescript.java, I have called that matcher).

Does the language server offer a way to filter completion results according to it?

If it's like tsserver, I think no.

Are you sure the filtering isn't something we can improve directly in the language server integration with what the Completion operation already provides?

If it's like tsserver, I think no.

Just for reference, you can look at my attempt to resolve the same filtering issue in Che: eclipse-che/che#2658

If we want the language server integration to be generic, it means that we should avoid as much as possible having people subclassing it. Subclassing is the opposite of what we want to achieve with generic editors and language servers, which are based on composition.

Ok but we need an extension point for LanguageSpecification on client side to retrieve the word. Perhaps we could start to override LSContentAssistProcessor like I suggest to support filter. What do you think about that

The completion filtering issue is not specific to completion provided by language server, as it can be associated directly to a file content-type and apply to whatever completion provider. So it seems to me it's more a feature to target in the Generic Editor.
As mentioned already, subclassing or overriding should be the very last solution considered. Here I believe that before going in that sub-optimal direction and start piling layers, we need to consider adding support for completion filters in the Generic Editor.
Can you please open a bug request for that?

Just for reference, you can look at my attempt to resolve the same filtering issue in Che: eclipse-che/che#2658

Thanks @kaloyan-raev for your feedback. If I see
https://github.com/eclipse/che/pull/2658/files#diff-d146d4a6b2abdb2d09237da9fc038467R114

You retrieve the word with getCurrentIdentifier (but it's generic and not depends on languages)

private String getCurrentIdentifier(String text, int offset) {
 +        int i = offset - 1;
 +        while (i >= 0 && isIdentifierChar(text.charAt(i))) {
 +            i--;
 +        }
 +        return text.substring(i + 1, offset);
 +    }
 +    
 +    private boolean isIdentifierChar(char c) {
 +        return c >= 'a' && c <= 'z' ||
 +            c >= 'A' && c <= 'Z' ||
 +            c >= '0' && c <= '9' ||
 +            c >= '\u007f' && c <= '\u00ff' ||
 +            c == '$' ||
 +            c == '_' ||
 +            c == '-';
 +    }

Right. It's quite generic for now. It should be improved in future.

Can you please open a bug request for that?

Done https://bugs.eclipse.org/bugs/show_bug.cgi?id=504042

Although it seems clear that we need support for filters in generic editor, and that the Eclipse-LSP integration should be able to contribute filters like it contributes completion, I'm still missing a link in that chain.
Where does VSCode get the information that :checked should be filtered out after a? Here it's not only about adding support for words, there is also somewhere where the authorized/unauthorized mappings should be defined to implement such filter. And apparently, it doesn't seem to be in the language server itself.
@kaloyan-raev Do you know more about VSCode filtering?

It doesn't seem to be in the language server itself.

Exactly it's not defined. It must be defined on client side. VSCode uses for that LanguageConfiguration to configure comments, bracket, word pattern, etc

I have tried to suggest this idea at https://bugs.eclipse.org/bugs/show_bug.cgi?id=502565

Where does VSCode get the information that :checkedshould be filtered out after a?

Retrieve of the word is managed with getWordRangeAtPosition

/**
 * Get a word-range at the given position. By default words are defined by
 * common separators, like space, -, _, etc. In addition, per languge custom
 * [word definitions](#LanguageConfiguration.wordPattern) can be defined.
 *
 * The position will be [adjusted](#TextDocument.validatePosition).
 *
 * @param position A position.
 * @return A range spanning a word, or `undefined`.
 */
getWordRangeAtPosition(position: Position): Range;

In the case of CSS, LanguageConfiguration wordPattern is defined like this:

languages.setLanguageConfiguration('css', {
    wordPattern: /(#?-?\d*\.\d\w*%?)|(::?[\w-]*(?=[^,{;]*[,{]))|(([@#.!])?[\w-?]+%?|[@#!.])/g
});

See https://github.com/Microsoft/vscode/blob/master/extensions/css/client/src/cssMain.ts#L59

VSCode can use this word range for completion, hover. You can see that
https://github.com/Microsoft/vscode/search?utf8=%E2%9C%93&q=getWordRangeAtPosition

Still, that doesn't give the missing link. Ok, VSCode has the word, OK it gets completion results from language server. How does it manage the filtering? Where is the operation that takes as input a word and a set of completion proposals and returns the filtered set of proposals?

@kaloyan-raev In Che intergation, how do you proceed for this issue? You retrieve all results from language server and then filter them? Did you reimplement rules for filtering? I'm surprised this filtering doesn't happen on the language server side...

@mickaelistria Yes, I do the filtering on the Che side. I used the existing FuzzyMatches class that was implemented for filtering the results from the document/workspace symbol requests. It worked excellent for filtering the code completion proposals too.

I was also surprised the filtering does not happen on the server side. I guess in VSCode it was decided to give more freedom to the client to decide on filtering and sorting the results. I also noticed that when you start typing a word in the editor, VSCode only sends a single completion request to the language server. Any additional key strokes within the same word do not generate new completion requests, the editor is just filtering the existing result list. Perhaps, this is the VSCode vision for optimal code completion.

However, the protocol does not prevent (for now) the server side to do the filtering. Perhaps, another approach for optimization would be if the client configures the server with the filtering rule (start with matching, substring matching, fuzzy matching, etc.) and then the server does the filtering. Then the client can still process the result list to compute the highlight regions. But then you need to send a new completion request on every key stroke. So, it's a trade off...

I've opened microsoft/vscode#13926 . IMO, filtering is more something to be provided by the language server as there is no value in having it returning erroneous results and no value if requiring all clients to reimplement it.