Global search in a background thread #1543
Conversation
|
It would depend on project size, hard drive speed and CPU performance to notice any difference. I did a test on a 15Gb repo I work on, |
| let show_picker = async move { | ||
| let all_matches: Vec<(usize, PathBuf)> = | ||
| UnboundedReceiverStream::new(all_matches_rx).collect().await; | ||
| let call: job::Callback = | ||
| Box::new(move |editor: &mut Editor, compositor: &mut Compositor| { | ||
| if all_matches.is_empty() { | ||
| editor.set_status("No matches found".to_string()); | ||
| return; | ||
| } | ||
|
|
||
| let picker = FilePicker::new( | ||
| all_matches, | ||
| move |(_line_num, path)| { | ||
| let relative_path = helix_core::path::get_relative_path(path) | ||
| .to_str() | ||
| .unwrap() | ||
| .to_owned(); | ||
| if current_path.as_ref().map(|p| p == path).unwrap_or(false) { | ||
| format!("{} (*)", relative_path).into() | ||
| } else { | ||
| relative_path.into() | ||
| } | ||
| }, | ||
| move |editor: &mut Editor, (line_num, path), action| { | ||
| match editor.open(path.into(), action) { | ||
| Ok(_) => {} | ||
| Err(e) => { | ||
| editor.set_error(format!( | ||
| "Failed to open file '{}': {}", | ||
| path.display(), | ||
| e | ||
| )); | ||
| return; | ||
| } | ||
| } | ||
|
|
||
| let line_num = *line_num; | ||
| let (view, doc) = current!(editor); | ||
| let text = doc.text(); | ||
| let start = text.line_to_char(line_num); | ||
| let end = text.line_to_char((line_num + 1).min(text.len_lines())); | ||
| let regex = regex_rx.recv().await; | ||
| if let Some(regex) = regex { | ||
| if let Ok(matcher) = RegexMatcherBuilder::new() | ||
| .case_smart(smart_case) | ||
| .build(regex.as_str()) | ||
| { | ||
| match global_search_task(file_picker_config, matcher).await { | ||
| Ok(all_matches) => { | ||
| let call: job::Callback = global_search_callback(all_matches, current_path); | ||
| return Ok(call); | ||
| } | ||
| Err(e) => { | ||
| log::error!("Global search error: {}", e); | ||
| } | ||
| }; | ||
| } | ||
| } | ||
|
|
||
| doc.set_selection(view.id, Selection::single(start, end)); | ||
| align_view(doc, view, Align::Center); | ||
| }, | ||
| |_editor, (line_num, path)| Some((path.clone(), Some((*line_num, *line_num)))), | ||
| ); | ||
| compositor.push(Box::new(picker)); | ||
| }); | ||
| Ok(call) | ||
| let empty_call: job::Callback = Box::new(|_, _| {}); | ||
| Ok(empty_call) |
There was a problem hiding this comment.
You probably want
cx.jobs.spawn(future);
That way you're not returning an empty job::Callback
There was a problem hiding this comment.
Sorry it took so long but I'm confused here; With job::Callback I have access to Editor and Compositor so I can set_status or push the picker to the compositor. How should I access Editor and Compositor if I'm using cx.jobs.spawn(future);, perhaps I misunderstand how jobs and everything work here?
| fn global_search(cx: &mut Context) { | ||
| let (all_matches_sx, all_matches_rx) = | ||
| tokio::sync::mpsc::unbounded_channel::<(usize, PathBuf)>(); | ||
| let (regex_sx, mut regex_rx) = tokio::sync::mpsc::channel(1); |
There was a problem hiding this comment.
You want to use tokio::sync::oneshot::channel instead
There was a problem hiding this comment.
regex is sent from the callback of Prompt ( move |_view, _doc, regex, event| { ... regex_sx.send() ...}), hence capturing regex_sx, I have just discovered that oneshot::Sender does not implement Copy trait and compiler would refuse to compile here.
There was a problem hiding this comment.
You can use a move closure to move it into the callback
There was a problem hiding this comment.
I have this error:
error[E0507]: cannot move out of
regex_sx, a captured variable in anFnclosure
I think the compiler cannot guarantee that we would call regex_sx only once, so it refuse to compile here.
There was a problem hiding this comment.
Right, using a channel seems like a hack to me anyway. I'd rather pass cx through the regex_prompt callback as a first parameter and manually call let (doc, view) = current!(cx). Then you can create the search picker instance inside the callback.
There was a problem hiding this comment.
I think having a dedicated FnOnce callback for PromptEvent::Validate might clear things up a bit and we can use oneshot channel, but I really don't want to introduce any breaking change to the API, hence the hack.
About passing cx through regex_prompt, wouldn't it mess up the lifetime of cx because we still need to await the search results (just my naive thought)?
| // Otherwise do nothing | ||
| // log::warn!("Global Search Invalid Pattern") | ||
| } | ||
| match block_on(regex_sx.send(regex)) { |
There was a problem hiding this comment.
If you use oneshot then the sender isn't async and you don't need to block_on here.
There was a problem hiding this comment.
Why is this using a channel rather than creating the picker and the global_search_task here? Is it because you have no access to cx or editor?
There was a problem hiding this comment.
I'll check these out and work on improvements as soon as I can (a bit occupied at the moment:) )
the-mikedavis
added
the
S-waiting-on-review
pickfire
added
S-waiting-on-author
kirawi
added
A-helix-term
Upon
PromptEvent::Validate, the regex will be sent through a channel awaited incx.jobs, after that the search_task is spawned bytokio::task::spawn_blocking, which is again awaited in cx.jobs. After that the rest is mostly the same as before.edit: Also removed unnecessary deps