From dae3841a75fc521c9e5d63ff8f3ffa32be4e41e1 Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Thu, 15 Feb 2024 15:13:44 -0500
Subject: [PATCH] Use an AsyncHook for picker preview highlighting
The picker previously used the IdleTimeout event as a trigger for
syntax-highlighting the currently selected document in the preview pane.
This is a bit ad-hoc now that the event system has landed and we can
refactor towards an AsyncHook (like those used for LSP completion and
signature-help). This should resolve some odd scenarios where the
preview did not highlight because of a race between the idle timeout
and items appearing in the picker.
---
helix-term/src/ui/picker.rs | 147 ++++++++-------------------
helix-term/src/ui/picker/handlers.rs | 117 +++++++++++++++++++++
2 files changed, 161 insertions(+), 103 deletions(-)
create mode 100644 helix-term/src/ui/picker/handlers.rs
diff --git a/helix-term/src/ui/picker.rs b/helix-term/src/ui/picker.rs
index b8ec57d5..cc86a4fa 100644
--- a/helix-term/src/ui/picker.rs
+++ b/helix-term/src/ui/picker.rs
@@ -1,3 +1,5 @@
+mod handlers;
+
use crate::{
alt,
compositor::{self, Component, Compositor, Context, Event, EventResult},
@@ -10,9 +12,11 @@ use crate::{
EditorView,
},
};
-use futures_util::{future::BoxFuture, FutureExt};
+use futures_util::future::BoxFuture;
+use helix_event::AsyncHook;
use nucleo::pattern::CaseMatching;
use nucleo::{Config, Nucleo, Utf32String};
+use tokio::sync::mpsc::Sender;
use tui::{
buffer::Buffer as Surface,
layout::Constraint,
@@ -25,7 +29,7 @@ use tui::widgets::Widget;
use std::{
collections::HashMap,
io::Read,
- path::PathBuf,
+ path::{Path, PathBuf},
sync::{
atomic::{self, AtomicBool},
Arc,
@@ -36,7 +40,6 @@ use crate::ui::{Prompt, PromptEvent};
use helix_core::{
char_idx_at_visual_offset, fuzzy::MATCHER, movement::Direction,
text_annotations::TextAnnotations, unicode::segmentation::UnicodeSegmentation, Position,
- Syntax,
};
use helix_view::{
editor::Action,
@@ -46,9 +49,12 @@ use helix_view::{
Document, DocumentId, Editor,
};
-pub const ID: &str = "picker";
+use self::handlers::PreviewHighlightHandler;
+
use super::{menu::Item, overlay::Overlay};
+pub const ID: &str = "picker";
+
pub const MIN_AREA_WIDTH_FOR_PREVIEW: u16 = 72;
/// Biggest file size to preview in bytes
pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024;
@@ -56,14 +62,14 @@ pub const MAX_FILE_SIZE_FOR_PREVIEW: u64 = 10 * 1024 * 1024;
#[derive(PartialEq, Eq, Hash)]
pub enum PathOrId {
Id(DocumentId),
- Path(PathBuf),
+ Path(Arc<Path>),
}
impl PathOrId {
fn get_canonicalized(self) -> Self {
use PathOrId::*;
match self {
- Path(path) => Path(helix_stdx::path::canonicalize(path)),
+ Path(path) => Path(helix_stdx::path::canonicalize(&path).into()),
Id(id) => Id(id),
}
}
@@ -71,7 +77,7 @@ impl PathOrId {
impl From<PathBuf> for PathOrId {
fn from(v: PathBuf) -> Self {
- Self::Path(v)
+ Self::Path(v.as_path().into())
}
}
@@ -197,10 +203,12 @@ pub struct Picker<T: Item> {
pub truncate_start: bool,
/// Caches paths to documents
- preview_cache: HashMap<PathBuf, CachedPreview>,
+ preview_cache: HashMap<Arc<Path>, CachedPreview>,
read_buffer: Vec<u8>,
/// Given an item in the picker, return the file path and line number to display.
file_fn: Option<FileCallback<T>>,
+ /// An event handler for syntax highlighting the currently previewed file.
+ preview_highlight_handler: Sender<Arc<Path>>,
}
impl<T: Item + 'static> Picker<T> {
@@ -280,6 +288,7 @@ impl<T: Item + 'static> Picker<T> {
preview_cache: HashMap::new(),
read_buffer: Vec::with_capacity(1024),
file_fn: None,
+ preview_highlight_handler: PreviewHighlightHandler::<T>::default().spawn(),
}
}
@@ -403,16 +412,26 @@ impl<T: Item + 'static> Picker<T> {
) -> Preview<'picker, 'editor> {
match path_or_id {
PathOrId::Path(path) => {
- let path = &path;
- if let Some(doc) = editor.document_by_path(path) {
+ if let Some(doc) = editor.document_by_path(&path) {
return Preview::EditorDocument(doc);
}
- if self.preview_cache.contains_key(path) {
- return Preview::Cached(&self.preview_cache[path]);
+ if self.preview_cache.contains_key(&path) {
+ let preview = &self.preview_cache[&path];
+ match preview {
+ // If the document isn't highlighted yet, attempt to highlight it.
+ CachedPreview::Document(doc) if doc.language_config().is_none() => {
+ helix_event::send_blocking(
+ &self.preview_highlight_handler,
+ path.clone(),
+ );
+ }
+ _ => (),
+ }
+ return Preview::Cached(preview);
}
- let data = std::fs::File::open(path).and_then(|file| {
+ let data = std::fs::File::open(&path).and_then(|file| {
let metadata = file.metadata()?;
// Read up to 1kb to detect the content type
let n = file.take(1024).read_to_end(&mut self.read_buffer)?;
@@ -427,14 +446,21 @@ impl<T: Item + 'static> Picker<T> {
(size, _) if size > MAX_FILE_SIZE_FOR_PREVIEW => {
CachedPreview::LargeFile
}
- _ => Document::open(path, None, None, editor.config.clone())
- .map(|doc| CachedPreview::Document(Box::new(doc)))
+ _ => Document::open(&path, None, None, editor.config.clone())
+ .map(|doc| {
+ // Asynchronously highlight the new document
+ helix_event::send_blocking(
+ &self.preview_highlight_handler,
+ path.clone(),
+ );
+ CachedPreview::Document(Box::new(doc))
+ })
.unwrap_or(CachedPreview::NotFound),
},
)
.unwrap_or(CachedPreview::NotFound);
- self.preview_cache.insert(path.to_owned(), preview);
- Preview::Cached(&self.preview_cache[path])
+ self.preview_cache.insert(path.clone(), preview);
+ Preview::Cached(&self.preview_cache[&path])
}
PathOrId::Id(id) => {
let doc = editor.documents.get(&id).unwrap();
@@ -443,84 +469,6 @@ impl<T: Item + 'static> Picker<T> {
}
}
- fn handle_idle_timeout(&mut self, cx: &mut Context) -> EventResult {
- let Some((current_file, _)) = self.current_file(cx.editor) else {
- return EventResult::Consumed(None);
- };
-
- // Try to find a document in the cache
- let doc = match ¤t_file {
- PathOrId::Id(doc_id) => doc_mut!(cx.editor, doc_id),
- PathOrId::Path(path) => match self.preview_cache.get_mut(path) {
- Some(CachedPreview::Document(ref mut doc)) => doc,
- _ => return EventResult::Consumed(None),
- },
- };
-
- let mut callback: Option<compositor::Callback> = None;
-
- // Then attempt to highlight it if it has no language set
- if doc.language_config().is_none() {
- if let Some(language_config) = doc.detect_language_config(&cx.editor.syn_loader.load())
- {
- doc.language = Some(language_config.clone());
- let text = doc.text().clone();
- let loader = cx.editor.syn_loader.clone();
- let job = tokio::task::spawn_blocking(move || {
- let syntax = language_config
- .highlight_config(&loader.load().scopes())
- .and_then(|highlight_config| {
- Syntax::new(text.slice(..), highlight_config, loader)
- });
- let callback = move |editor: &mut Editor, compositor: &mut Compositor| {
- let Some(syntax) = syntax else {
- log::info!("highlighting picker item failed");
- return;
- };
- let picker = match compositor.find::<Overlay<Self>>() {
- Some(Overlay { content, .. }) => Some(content),
- None => compositor
- .find::<Overlay<DynamicPicker<T>>>()
- .map(|overlay| &mut overlay.content.file_picker),
- };
- let Some(picker) = picker else {
- log::info!("picker closed before syntax highlighting finished");
- return;
- };
- // Try to find a document in the cache
- let doc = match current_file {
- PathOrId::Id(doc_id) => doc_mut!(editor, &doc_id),
- PathOrId::Path(path) => match picker.preview_cache.get_mut(&path) {
- Some(CachedPreview::Document(ref mut doc)) => {
- let diagnostics = Editor::doc_diagnostics(
- &editor.language_servers,
- &editor.diagnostics,
- doc,
- );
- doc.replace_diagnostics(diagnostics, &[], None);
- doc
- }
- _ => return,
- },
- };
- doc.syntax = Some(syntax);
- };
- Callback::EditorCompositor(Box::new(callback))
- });
- let tmp: compositor::Callback = Box::new(move |_, ctx| {
- ctx.jobs
- .callback(job.map(|res| res.map_err(anyhow::Error::from)))
- });
- callback = Some(Box::new(tmp))
- }
- }
-
- // QUESTION: do we want to compute inlay hints in pickers too ? Probably not for now
- // but it could be interesting in the future
-
- EventResult::Consumed(callback)
- }
-
fn render_picker(&mut self, area: Rect, surface: &mut Surface, cx: &mut Context) {
let status = self.matcher.tick(10);
let snapshot = self.matcher.snapshot();
@@ -826,9 +774,6 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {
}
fn handle_event(&mut self, event: &Event, ctx: &mut Context) -> EventResult {
- if let Event::IdleTimeout = event {
- return self.handle_idle_timeout(ctx);
- }
// TODO: keybinds for scrolling preview
let key_event = match event {
@@ -861,9 +806,6 @@ impl<T: Item + 'static + Send + Sync> Component for Picker<T> {
EventResult::Consumed(Some(callback))
};
- // So that idle timeout retriggers
- ctx.editor.reset_idle_timer();
-
match key_event {
shift!(Tab) | key!(Up) | ctrl!('p') => {
self.move_by(1, Direction::Backward);
@@ -989,7 +931,7 @@ impl<T: Item + Send + Sync + 'static> Component for DynamicPicker<T> {
cx.jobs.callback(async move {
let new_options = new_options.await?;
- let callback = Callback::EditorCompositor(Box::new(move |editor, compositor| {
+ let callback = Callback::EditorCompositor(Box::new(move |_editor, compositor| {
// Wrapping of pickers in overlay is done outside the picker code,
// so this is fragile and will break if wrapped in some other widget.
let picker = match compositor.find_id::<Overlay<DynamicPicker<T>>>(ID) {
@@ -997,7 +939,6 @@ impl<T: Item + Send + Sync + 'static> Component for DynamicPicker<T> {
None => return,
};
picker.set_options(new_options);
- editor.reset_idle_timer();
}));
anyhow::Ok(callback)
});
diff --git a/helix-term/src/ui/picker/handlers.rs b/helix-term/src/ui/picker/handlers.rs
new file mode 100644
index 00000000..7a77efa4
--- /dev/null
+++ b/helix-term/src/ui/picker/handlers.rs
@@ -0,0 +1,117 @@
+use std::{path::Path, sync::Arc, time::Duration};
+
+use helix_event::AsyncHook;
+use tokio::time::Instant;
+
+use crate::{
+ job,
+ ui::{menu::Item, overlay::Overlay},
+};
+
+use super::{CachedPreview, DynamicPicker, Picker};
+
+pub(super) struct PreviewHighlightHandler<T: Item> {
+ trigger: Option<Arc<Path>>,
+ phantom_data: std::marker::PhantomData<T>,
+}
+
+impl<T: Item> Default for PreviewHighlightHandler<T> {
+ fn default() -> Self {
+ Self {
+ trigger: None,
+ phantom_data: Default::default(),
+ }
+ }
+}
+
+impl<T: Item> AsyncHook for PreviewHighlightHandler<T> {
+ type Event = Arc<Path>;
+
+ fn handle_event(
+ &mut self,
+ path: Self::Event,
+ timeout: Option<tokio::time::Instant>,
+ ) -> Option<tokio::time::Instant> {
+ if self
+ .trigger
+ .as_ref()
+ .is_some_and(|trigger| trigger == &path)
+ {
+ // If the path hasn't changed, don't reset the debounce
+ timeout
+ } else {
+ self.trigger = Some(path);
+ Some(Instant::now() + Duration::from_millis(150))
+ }
+ }
+
+ fn finish_debounce(&mut self) {
+ let Some(path) = self.trigger.take() else {
+ return;
+ };
+
+ job::dispatch_blocking(move |editor, compositor| {
+ let picker = match compositor.find::<Overlay<Picker<T>>>() {
+ Some(Overlay { content, .. }) => content,
+ None => match compositor.find::<Overlay<DynamicPicker<T>>>() {
+ Some(Overlay { content, .. }) => &mut content.file_picker,
+ None => return,
+ },
+ };
+
+ let Some(CachedPreview::Document(ref mut doc)) = picker.preview_cache.get_mut(&path)
+ else {
+ return;
+ };
+
+ if doc.language_config().is_some() {
+ return;
+ }
+
+ let Some(language_config) = doc.detect_language_config(&editor.syn_loader.load())
+ else {
+ return;
+ };
+ doc.language = Some(language_config.clone());
+ let text = doc.text().clone();
+ let loader = editor.syn_loader.clone();
+
+ tokio::task::spawn_blocking(move || {
+ let Some(syntax) = language_config
+ .highlight_config(&loader.load().scopes())
+ .and_then(|highlight_config| {
+ helix_core::Syntax::new(text.slice(..), highlight_config, loader)
+ })
+ else {
+ log::info!("highlighting picker item failed");
+ return;
+ };
+
+ job::dispatch_blocking(move |editor, compositor| {
+ let picker = match compositor.find::<Overlay<Picker<T>>>() {
+ Some(Overlay { content, .. }) => Some(content),
+ None => compositor
+ .find::<Overlay<DynamicPicker<T>>>()
+ .map(|overlay| &mut overlay.content.file_picker),
+ };
+ let Some(picker) = picker else {
+ log::info!("picker closed before syntax highlighting finished");
+ return;
+ };
+ let Some(CachedPreview::Document(ref mut doc)) =
+ picker.preview_cache.get_mut(&path)
+ else {
+ return;
+ };
+ let diagnostics = helix_view::Editor::doc_diagnostics(
+ &editor.language_servers,
+ &editor.diagnostics,
+ doc,
+ );
+ doc.replace_diagnostics(diagnostics, &[], None);
+ doc.syntax = Some(syntax);
+ });
+ });
+ });
+ }
+}