Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

All completion proposals filtered out from YAML language #218

Open
BoykoAlex opened this issue May 14, 2018 · 6 comments
Open

All completion proposals filtered out from YAML language #218

BoykoAlex opened this issue May 14, 2018 · 6 comments

Comments

@BoykoAlex
Copy link
Contributor

BoykoAlex commented May 14, 2018

memory: 1^ - works with atom language client and shows proposals coming from the LS
memory: ^- doesn't show any proposals, but language client receives proposals from the LS

The culprit of the problem is around here: https://github.com/atom/atom-languageclient/blob/master/lib/adapters/autocomplete-adapter.ts#L103-L107

The prefix of the request: ": " but the trigger is "". The LS is unaware of the prefix is not part of LSP. The proposals text does not include ": " I'm guessing this is the reason from 0 score and consequently all proposals from the server are filtered out...

Perhaps this is something special about the YAML grammar definition... wonder if we could put a workaround in the language client to include proposals with 0 score or make the new filtering optional such that it could be turned off by setting a flag in AutoLanguageClient?

@BoykoAlex
Copy link
Contributor Author

Actually, it's not just YAML, it's Java Properties as well... property = ^ doesn't work, but property = t^ works or property=^ also works.
Don't think we need to include prefix into suggestions filtering...

@BoykoAlex
Copy link
Contributor Author

BoykoAlex commented Jun 12, 2018

To reproduce:

  1. Open Atom
  2. Open package installer
  3. Search for cf-manifest-yaml and install the latest version

This package lets you edit manifest.yml files which are application deployment manifests for Cloud Foundry.
(If interested in more details, here are the docs: https://docs.cloudfoundry.org/devguide/deploy-apps/manifest.html)

Create manifest.yml file with the content below. Note that ^ is place where CA is invoked (Cmd+Space or Ctrl+Space)

applications:
- name: app
  memory: ^

Note that there is no CA shown... however if you type a space or 1 and then invoke it then you'll see the proposals.

Roll back to the initial content with no prefix for the memory value (as show above)
Put a break point in autocomplete-adapter.ts from atom-languageclient here https://github.com/atom/atom-languageclient/blob/master/lib/adapters/autocomplete-adapter.ts#L102 and invoke CA.

Note that suggestions variable has all 3 proposals for memory value, but then a call to filter(...) would nuke all proposals (https://github.com/atom/atom-languageclient/blob/master/lib/adapters/autocomplete-adapter.ts#L106)

@Gert-dev
Copy link

Gert-dev commented Sep 7, 2019

I'm experiencing the exact same problem with the Serenata language server; I'm trying to return an intelligent list of results from the server, but the culprit call to fuzzaldrin's filter is not only filtering out valid results returned by the server, it is also re-sorting my list to sort less relevant results on top.

I understand this filtering and ordering may be useful and sometimes even desirable, but in some cases the server knows what's best for the language and should get complete dictatorship over the list. The client may still identify matching characters in the result, of course, but it may not filter or re-sort.

It may be worth looking at the filterSuggestions option that can be passed via provideAutocomplete for this. I used to set this to false before I was using this package to ensure this behavior, so was extra confused that this was somehow still happening. Perhaps if this setting is false, the filter call can always be skipped?

@Aerijo
Copy link
Contributor

Aerijo commented Sep 7, 2019

There have been some changes to autocomplete in the current master branch, so I would like to know how they affect your issue.

I’m not sure what you mean by disallowing filtering and sorting though; do you want the server to recalculate and provide a new list on every character typed? The client will normally only request the completion once, presenting the initial ones ordered via sorttext, and use the filtertext property to sort and filter them as the user types. Requesting each time seems quite wasteful, especially for large amounts of completions.

Finally, to disallow it would need to be in the LSP spec somewhere, which if we are not following would be a legitimate bug. If it is a custom extension, you would first need to get it into the LSP spec.

@Gert-dev
Copy link

Gert-dev commented Sep 9, 2019

Thanks for your reply. I made some new discoveries after I posted, and I'll list them here:

I did not know about the sortText property yet, and it solves my issue partially by having provided it from the server. The client is doing everything right here. Thanks!

Suggesting, as I did, that the client not filter at all, is probably wrong. The filterText property is there for a reason, and that is to let the client filter by it. Nonetheless, due to the current filtering, my results are different as to when it is not filtering, even though the prefix matches the filterText in all of my completions.

This suggests that either filtering is being applied on top of what is returned by the server (which already consists only of filtered matches on the same text, in my case), and this filtering results in different results, or the filtered results are the same, and the ordering is altered afterwards, disregarding the sortText. I can investigate this further, if you like.

For what it's worth, I can currently work around this additional filtering and sorting by doing the following, which may help someone:

getSuggestions(request) {
    request.prefix = '';

    return super.getSuggestions(request);
}

The prefix is not used anywhere else and used to determine whether filtering must be applied.

Finally, your statement do you want the server to recalculate and provide a new list on every character typed? is correct. It may seem wasteful, but it is quite the opposite - believe me, I have exhausted my mind trying to avoid doing it. The LSP allows this via the isIncomplete option, which is already supported.

The reason for recalculating is that, in order for the client to filter based on the prefix, the full set of relevant results must be returned that are relevant in that context. This includes classes, functions, and other relevant structural elements. There are thousands of these in a fairly large code base in my case, which need to be transmitted over the socket (every time a new cursor location is used and the client cannot fall back to its cache), just so the client can apply its filtering. This is not only expensive for the server to send, it is also expensive for the client to process and filter.

Furthermore, this list changes fairly often, as new structural elements are added or renamed, so any client side cache is short-lived. I used this approach in php-ide-serenata before it was a language client, and it was... not great, to say the least, CPU spiked every so often to ship over the new lists.

You cannot limit in a fashion similar to "return 10 arbitrary classes, return 10 arbitrary functions" either, as then the client will not be able to filter out relevant matches once the user starts typing (since they were never returned, to begin with). You then automatically end up at letting the server do a reasonable amount of filtering based on the prefix before sending the results back, to reduce the result count, at which point you're basically already fuzzy matching server side and all filtering performed by the client becomes pretty much unnecessary.

Of course, I may simply have reached the boundary of my abilities, and someone else has a genius suggestion that will fix all my problems 😄 .

@UziTech
Copy link

UziTech commented Oct 19, 2020

Development of atom-languageclient has officially moved to https://github.com/atom-ide-community/atom-languageclient 🎉

If this is still an issue please consider opening an issue on that repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants