Re-request workspace symbols dynamically #5055
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most language servers limit the number of workspace symbols that are returned with an empty query even though all symbols are supposed to be returned, according to the spec (for perfomance reasons). This patch adds a workspace symbol picker based on a dynamic picker that allows re-requesting the symbols on every keypress (i.e. when the picker query text changes). The old behavior has been completely replaced, and I have only tested with rust-analyzer so far.
This change uses the idle-timeout event to trigger fetching new results in the DynamicPicker, so idle-timeout becomes a sort of debounce. This prevents querying the language server overly aggressively.
If the new results shown by the picker select a file that hasn't been previewed before, the idle timeout would not trigger highlighting on that file. With this change, we reset the idle timeout and allow that file to be highlighted on the next idle timeout event.
the-mikedavis
added
C-enhancement
A language server might send None as the response to workspace symbols. We should treat this as the empty Vec rather than the server sending an error status. This fixes the interaction with gopls which uses None to mean no matching symbols.
This was referenced Dec 30, 2022
Philipp-M
added a commit
to Philipp-M/helix
that referenced
this pull request
Feb 16, 2023
…ith multiple different item sources (async, sync, refetch) This PR unifies the functionality of the `Picker` and the `Menu` in the struct `OptionsManager` (i.e. matching/filtering, sorting, cursor management etc.), and extends the logic of `score` by being able to retain the cursor position (which is useful, if new entries are added async). The main reason for this PR though is to have multiple option `ItemSource`s, which can be async and which may be refetched (see helix-editor#5055) to provide the options for the `OptionsManager`. It currently allows 3 different input sources: synchronous data, asynchronous data, and asynchronous functions which provide the data, which are reexecuted after an `IdleTimeout` occured (similar as helix-editor#5055). This PR is or will be a requirement for helix-editor#2507 (currently provided by helix-editor#3082), which I will rebase onto this PR soon. Noticeable effects right now: * The `Menu` now has the more sophisticated logic of the picker (mostly better caching while changing the query) * The `Menu` currently *doesn't* reset the cursor position, if `score()` is called (effectively different pattern query), I'm not sure if we should keep this behavior, I guess it needs some testing and feedback. I have kept the original behavior of the Picker (everytime `score()` changes it resets the cursor) This PR is an alternative to helix-editor#3082, which I wasn't completely happy with (adding options async was/is kind of a hack to keep it simple) thus this PR will supersede it. It was originally motivated by helix-editor#5055 and how to integrate that logic into helix-editor#2507 (which is not done yet). The PR unfortunately grew a little bit more than I've anticipated, but in general I think it's the cleaner and more future proof way to be able to have async item sources, which may change the options overtime. For example the same logic of helix-editor#5055 could be simply implemented for the completion menu with different multiple completion sources, whether it makes sense or not (though also one of the motivations of this PR to have this possibility). This should also cleanup and greatly (hopefully) simplify helix-editor#2507. To make reviewing easier (I hope) I provide a technical summary of this PR: * `OptionsManager` contains all logic that is necessary to manage options (d'oh), that means: * Filtering/matching of options with `score()` which takes a few more parameters now: * *Optional* pattern to be able to recalculate the score without providing a pattern (necessary for async addition of the options) * Boolean flag to retain the cursor, since it will likely be annoying, if an item source takes so long that the user is already selecting options of a different item source and it gets resetted because score has to be called again for matches that respect the new option source). * force_recalculation, kind of an optimization, if nothing external has changed (except new async options) * The `OptionsManager` is fed by `ItemSource`s * Fetching of items from the item source is started by the creation of the `OptionsManager` (`create_from_item_sources`), and as soon one item source provides some items a construction callback is executed (see below) * Async fetching is done by looping over `FuturesUnordered` async requests and adding the options via an `mpsc`, I wanted to avoid any `Mutex` or something alike to keep the rest of the logic as is (and likely to be more performant as well). Thus instead the editor `redraw_handle` is notified when new options are available and in the render functions new options are polled (via `poll_for_new_options`). Although not optimal, it was IMHO the best tradeoff (I tried different solutions, and came to that conclusion). * Options are stored with an additional index to the item source to be able to easily change the options on the fly, it provides some iterator wrapper functions to access the options/matches outside as usual. * When using the `OptionManager` with async sources, I have decided (to mostly mirror current logic) to use a "constructor callback" in the `create_from_item_sources` for creating e.g. the actual `Menu` or `Picker` when at least one item is available (this callback is only then executed), otherwise an optional other callback can show e.g. editor status messages, when there was no item source provided any data. * The `(File-)Picker` and `Menu` now takes the `OptionsManager` instead of the items or editor data (this is now provided in the `ItemSource`) * For sync data it's also possible to directly create the `OptionsManager` without a callback using `create_from_items` but only with this function. * I've imported the `CompletionItem` from helix-editor#2507 to provide additional information where the item came from (which language server id and offset encoding) when multiple language servers are used as Item source, it's currently an enum to avoid unnecessary refactoring if other completion sources are added (such as helix-editor#2608 or helix-editor#1015), though it's not really relevant for this PR currently (as the doc has just one language server anyway), but I think it may be easier to review it here separated from helix-editor#2507 and it's also the (only) dependency of helix-editor#2507 for helix-editor#2608 * The `DynamicPicker` is removed as this behavior can now be modeled with a `ItemSource::AsyncRefetchOnIdleTimeoutWithPattern` which is done for the `workspace_symbol_picker`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #3110. Many language servers don't send all symbols when the client sends an empty query (typically for performance reasons). This change adds a DynamicPicker component that re-requests workspace symbols when the query changes.
See sudormrfbin's before & after gifs:
This version uses the idle-timeout event as a sort of debounce which prevents querying the language server too aggressively.
Fixes #1437
Fixes #3619
Fixes #4910
Closes #3110