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

search buffer contents during global search #5652

Merged
merged 1 commit into from Jul 11, 2023

Conversation

pascalkuthe
Copy link
Member

Followup to #5639, addresses #5604 (comment)

Currently, helix always queries the FS when performing global search.
However, when previewing and opening a file helix automatically uses the buffer if it's already open.
That means that the search would report incorrect results for unsaved buffers (which lead to crashes prior to #5639).

This PR creates am implementation of std::io::Read for Rope, so we can easily reuse the existing API in grep_searcher.

This change has the nice side effect of avoiding IO for already open buffers for global search. The performance difference will rarely matter in practice, but it's a nice boost when a huge file is already open in helix.
A similar mechanism can be used for #5645 so that files that are changed but unsaved also get reported as changed.

@pascalkuthe pascalkuthe added C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer. A-helix-term Area: Helix term improvements labels Jan 23, 2023
@pascalkuthe pascalkuthe mentioned this pull request Jan 23, 2023
@pascalkuthe pascalkuthe added E-medium Call for participation: Experience needed to fix: Medium / intermediate and removed E-easy Call for participation: Experience needed to fix: Easy / not much labels Jan 23, 2023
@pascalkuthe
Copy link
Member Author

Closes #4207

@the-mikedavis the-mikedavis linked an issue Jan 24, 2023 that may be closed by this pull request
Copy link
Member

@the-mikedavis the-mikedavis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar enough with Rust parallelism to review most of this but I think this looks good. I left some small typo fix comments

helix-view/src/editor.rs Outdated Show resolved Hide resolved
helix-view/src/editor.rs Outdated Show resolved Hide resolved
helix-term/src/commands.rs Show resolved Hide resolved
helix-term/src/commands.rs Show resolved Hide resolved
@pascalkuthe pascalkuthe force-pushed the buffer_grep branch 2 times, most recently from bb342b7 to 5a6f91a Compare January 31, 2023 17:40
@zyklotomic
Copy link
Contributor

Slightly unrelated to the PR, but wanted to check if my understanding of the UnboundedReceiverStream is correct.

In the asynchronous callback show_picker, do we have to worry about the UnboundedReceiverStream not collecting any values (because all the all_matches_sx from earlier have been dropped)? In WalkerBuilder.run(), inside the Box, in the definition of sink, there is a reference to all_matches_sx, so it won't be dropped. Is this why we don't have to worry about the UnboundedReceiverStream closing early?

@archseer archseer added this to the next milestone Jun 7, 2023
@pascalkuthe pascalkuthe added A-command Area: Commands and removed A-helix-term Area: Helix term improvements labels Jun 20, 2023
helix-view/src/editor.rs Outdated Show resolved Hide resolved
@archseer archseer merged commit 1adb194 into helix-editor:master Jul 11, 2023
6 checks passed
@useche useche mentioned this pull request Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands C-bug Category: This is a bug E-medium Call for participation: Experience needed to fix: Medium / intermediate S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Global search should search opened documents
6 participants