Request workspace symbols on idle-timeout #4
Conversation
the-mikedavis
force-pushed
the
md-dyn-picker-handle-idle-timeout
branch
from
c14bb6e
to
2e8bd9d
Compare
Now that the idle-timeout is an event handled by components, we can trigger a workspace symbol request on the idle-timeout as a sort of debounce.
the-mikedavis
force-pushed
the
md-dyn-picker-handle-idle-timeout
branch
from
2e8bd9d
to
0987b5e
Compare
|
cc @sudormrfbin (sorry I don't mean to ping, I just noticed you aren't watching this repo so you may not have seen this. no rush!) |
|
I indeed missed the notification (the title is similar to the PR open in the upstream repo too) so thanks for the ping 😄. I'm away for the weekend though, so it might be a few days till I get to properly test and review the code :) |
We need to compare the current query in the file picker to the query that we recorded on the last completion request. If we don't, we won't be able to use the arrow keys: using the arrow keys will trigger another idle timeout, so if we scroll through the options we end up re-requesting the workspace symbols which resets our position in the options, making it impossible to select anything but the first result.
There was a problem hiding this comment.
Sorry for the wait, I'm back! The current approach has the disadvantage of the latency being dependent on a config value that seems unrelated to this -- it might be assumed that the idle timeout config is only used for showing completion popup, so changing it to say 1 second will only update the dynamic picker every second. I suppose we need internal idle timer(s) that are not configurable, but there's no rush since this is seems like a good initial take 👍🏾.
helix-term/src/ui/picker.rs
Outdated
| // Try to find a document in the cache | ||
| let doc = self | ||
| .current_file(cx.editor) | ||
| .current_file(editor) | ||
| .and_then(|(path, _range)| self.preview_cache.get_mut(&path)) | ||
| .and_then(|cache| match cache { | ||
| CachedPreview::Document(doc) => Some(doc), |
There was a problem hiding this comment.
We could use an early return here and drop the Option wrapping
helix-term/src/ui/picker.rs
Outdated
| // This callback is triggered on the idle timeout, so we simulate the | ||
| // file-picker's handle_idle_timeout behavior by highlighting the | ||
| // currently previewed file. | ||
| file_picker.highlight_current_file(editor); |
There was a problem hiding this comment.
Is this needed here? Isn't the highlighting already handled by calling file_picker.handle_event() at the beginning of the function, which calls handle_idle_timeout()?
There was a problem hiding this comment.
If the new symbols response triggered by the idle timeout selects a new file then it won't be highlighted. I just noticed that it doesn't work anyways so I'll remove this 😅
There was a problem hiding this comment.
(for example searching for something specific like "flip" in the helix repo)
|
Closing this since upstream PR has been superseded and another one merged; thanks for driving it to completion :) |
Hey sudormrfbin 😀
This is a change on top of helix-editor#3110 that uses the idle-timeout as a debounce.
The call to highlight the current file feels slightly hacky to me. What do you think?