Skip to content
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

Feature: allow disabling of autocompletion-related limits via configuration #13564

Closed
danielhuang opened this issue Nov 6, 2022 · 3 comments
Closed
Labels
A-completion autocompletion C-support Category: support questions

Comments

@danielhuang
Copy link

danielhuang commented Nov 6, 2022

I currently maintain a branch that has the autocompletion-related limits disabled. See https://github.com/danielhuang/rust-analyzer/tree/completion2.

Specifically, the changes are:

  • Change DEFAULT_QUERY_SEARCH_LIMIT to 400000 (from the original 40)
  • Remove the limit for fuzzy_name_length (automatic imports will be found immediately, even without 3 characters already typed)
  • Set the LSP response's is_incomplete to false to prevent rust-analyzer from recomputing the completions when typing another character

Note: The branch also contains #11667 (which was closed).

Overall, these changes provide a more consistent experience for those using the forked branch, by removing the surprise factor when new items suddenly appear when typing that were previously shown to not be available, and removes the delay before the new autocompletion popup appears.

The default behavior (in a large project):

Screencast.from.2022-11-06.16-07-04.mov

With changes (in the same project):

Screencast.from.2022-11-06.16-11-05.mov

It would be nice if this was configurable without using the fork.

@Veykril
Copy link
Member

Veykril commented Nov 6, 2022

  • Change DEFAULT_QUERY_SEARCH_LIMIT to 400000 (from the original 40)

I can certainly see us have a setting that reflects the limit, allowing users to bump it up if they so desire/

Remove the limit for fuzzy_name_length (automatic imports will be found immediately, even without 3 characters already typed)

This limit is in place because calculating import completions for less characters is incredibly expensive and finds way too many results which isn't too useful. I am doubtful that removing that limit via a config will benefit anyone.

Set the LSP response's is_incomplete to false to prevent rust-analyzer from recomputing the completions when typing another character.

This will cause completions to be missed out in certain scenarios. For one assuming the fuzzy_name_length limit is still in place, the import completions won't be visibile if the completion was requested before the number of charactesr of the limit were typed out. But even without that, requesting completions for an empty identifier vs a non empty one yield different results due to limitations with salsa (that is requesting completions in let x = $0 vs let x = a$0 where $0 is the cursor yield different results).
A second reason why is_incomplete is required is because of how the LSP works in regards to server side sorting. We need this to be set to false so that we can dynamically sort the results depending on what was typed.

@Veykril Veykril added A-completion autocompletion C-support Category: support questions labels Nov 7, 2022
@danielhuang
Copy link
Author

This limit is in place because calculating import completions for less characters is incredibly expensive and finds way too many results which isn't too useful. I am doubtful that removing that limit via a config will benefit anyone.

I don't know how common situations like this occur for everyone, but as an example, I had to search the documentation instead of viewing the autocomplete.

Specifically, when implementing error handling using color-eyre, typing . after a color_eyre::Report does not show any useful methods for attaching additional information to the error report. Eventually, I found Section::section which was a trait method that required an additional import. If the import completions appeared immediately, I would have found section without searching directly via docs.rs.

@Veykril
Copy link
Member

Veykril commented Apr 18, 2024

My previous points still stand, we can't lift the current limits as rust-analyzer is generally too slow for these. I've also changed my mind on the config for the limit as the same applies there, we can bump the limit incrementally once we made progress with the perf of completions, but until then I personally don't want to change it.

@Veykril Veykril closed this as completed Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-completion autocompletion C-support Category: support questions
Projects
None yet
2 participants