feat: Diff source caching and autodetection

Author: poliorcetics

Cargo.lock

@@ -71,6 +71,7 @@ use std::{
     future::Future,
     io::Read,
     num::NonZeroUsize,
+    sync::Arc,
 };
 
 use std::{
@@ -3027,15 +3028,15 @@ fn jumplist_picker(cx: &mut Context) {
 
 fn changed_file_picker(cx: &mut Context) {
     pub struct FileChangeData {
-        cwd: PathBuf,
+        cwd: Arc<Path>,
         style_untracked: Style,
         style_modified: Style,
         style_conflict: Style,
         style_deleted: Style,
         style_renamed: Style,
     }
 
-    let cwd = helix_stdx::env::current_working_dir();
+    let cwd: Arc<Path> = Arc::from(helix_stdx::env::current_working_dir().as_path());
     if !cwd.exists() {
         cx.editor
             .set_error("Current working directory does not exist");
@@ -3106,17 +3107,24 @@ fn changed_file_picker(cx: &mut Context) {
     .with_preview(|_editor, meta| Some((meta.path().into(), None)));
     let injector = picker.injector();
 
-    cx.editor
-        .diff_providers
-        .clone()
-        .for_each_changed_file(cwd, move |change| match change {
+    // Helix can be launched without arguments, in which case no diff provider will be loaded since
+    // there is no file to provide infos for.
+    //
+    // This ensures we have one to work with for cwd (and as a bonus it means any file opened
+    // from this picker will have its diff provider already in cache).
+    cx.editor.diff_providers.add(&cwd);
+    cx.editor.diff_providers.clone().for_each_changed_file(
+        cwd.clone(),
+        move |change| match change {
             Ok(change) => injector.push(change).is_ok(),
             Err(err) => {
                 status::report_blocking(err);
                 true
             }
-        });
+        },
+    );
     cx.push_layer(Box::new(overlaid(picker)));
+    cx.editor.diff_providers.remove(&cwd);
 }
 
 pub fn command_palette(cx: &mut Context) {

helix-term/src/commands.rs

@@ -1279,7 +1279,7 @@ fn reload(
 
     let scrolloff = cx.editor.config().scrolloff;
     let (view, doc) = current!(cx.editor);
-    doc.reload(view, &cx.editor.diff_providers).map(|_| {
+    doc.reload(view, &mut cx.editor.diff_providers).map(|_| {
         view.ensure_cursor_in_view(doc, scrolloff);
     })?;
     if let Some(path) = doc.path() {
@@ -1318,6 +1318,8 @@ fn reload_all(
         })
         .collect();
 
+    cx.editor.diff_providers.reset();
+
     for (doc_id, view_ids) in docs_view_ids {
         let doc = doc_mut!(cx.editor, &doc_id);
 
@@ -1327,7 +1329,7 @@ fn reload_all(
         // Ensure that the view is synced with the document's history.
         view.sync_changes(doc);
 
-        if let Err(error) = doc.reload(view, &cx.editor.diff_providers) {
+        if let Err(error) = doc.reload(view, &mut cx.editor.diff_providers) {
             cx.editor.set_error(format!("{}", error));
             continue;
         }

helix-term/src/commands/typed.rs

@@ -19,7 +19,7 @@ tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "p
 parking_lot = "0.12"
 arc-swap = { version = "1.7.1" }
 
-gix = { version = "0.64.0", features = ["attributes", "status"], default-features = false, optional = true }
+gix = { version = "0.64.0", features = ["attributes", "parallel", "status"], default-features = false, optional = true }
 imara-diff = "0.1.7"
 anyhow = "1"
 

helix-vcs/Cargo.toml

@@ -22,16 +22,11 @@ use crate::FileChange;
 #[cfg(test)]
 mod test;
 
-pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> {
+pub fn get_diff_base(repo: &ThreadSafeRepository, file: &Path) -> Result<Vec<u8>> {
     debug_assert!(!file.exists() || file.is_file());
     debug_assert!(file.is_absolute());
 
-    // TODO cache repository lookup
-
-    let repo_dir = file.parent().context("file has no parent directory")?;
-    let repo = open_repo(repo_dir)
-        .context("failed to open git repo")?
-        .to_thread_local();
+    let repo = repo.to_thread_local();
     let head = repo.head_commit()?;
     let file_oid = find_file_in_commit(&repo, &head, file)?;
 
@@ -53,13 +48,14 @@ pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> {
     }
 }
 
-pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
+pub fn get_current_head_name(
+    repo: &ThreadSafeRepository,
+    file: &Path,
+) -> Result<Arc<ArcSwap<Box<str>>>> {
     debug_assert!(!file.exists() || file.is_file());
     debug_assert!(file.is_absolute());
-    let repo_dir = file.parent().context("file has no parent directory")?;
-    let repo = open_repo(repo_dir)
-        .context("failed to open git repo")?
-        .to_thread_local();
+
+    let repo = repo.to_thread_local();
     let head_ref = repo.head_ref()?;
     let head_commit = repo.head_commit()?;
 
@@ -71,11 +67,14 @@ pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
     Ok(Arc::new(ArcSwap::from_pointee(name.into_boxed_str())))
 }
 
-pub fn for_each_changed_file(cwd: &Path, f: impl Fn(Result<FileChange>) -> bool) -> Result<()> {
-    status(&open_repo(cwd)?.to_thread_local(), f)
+pub fn for_each_changed_file(
+    repo: &ThreadSafeRepository,
+    f: impl Fn(Result<FileChange>) -> bool,
+) -> Result<()> {
+    status(&repo.to_thread_local(), f)
 }
 
-fn open_repo(path: &Path) -> Result<ThreadSafeRepository> {
+pub(super) fn open_repo(path: &Path) -> Result<ThreadSafeRepository> {
     // custom open options
     let mut git_open_opts_map = gix::sec::trust::Mapping::<gix::open::Options>::default();
 

helix-vcs/src/git.rs

@@ -54,7 +54,8 @@ fn missing_file() {
     let file = temp_git.path().join("file.txt");
     File::create(&file).unwrap().write_all(b"foo").unwrap();
 
-    assert!(git::get_diff_base(&file).is_err());
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert!(git::get_diff_base(&repo, &file).is_err());
 }
 
 #[test]
@@ -64,7 +65,12 @@ fn unmodified_file() {
     let contents = b"foo".as_slice();
     File::create(&file).unwrap().write_all(contents).unwrap();
     create_commit(temp_git.path(), true);
-    assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents));
+
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert_eq!(
+        git::get_diff_base(&repo, &file).unwrap(),
+        Vec::from(contents)
+    );
 }
 
 #[test]
@@ -76,7 +82,11 @@ fn modified_file() {
     create_commit(temp_git.path(), true);
     File::create(&file).unwrap().write_all(b"bar").unwrap();
 
-    assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents));
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert_eq!(
+        git::get_diff_base(&repo, &file).unwrap(),
+        Vec::from(contents)
+    );
 }
 
 /// Test that `get_file_head` does not return content for a directory.
@@ -95,7 +105,9 @@ fn directory() {
 
     std::fs::remove_dir_all(&dir).unwrap();
     File::create(&dir).unwrap().write_all(b"bar").unwrap();
-    assert!(git::get_diff_base(&dir).is_err());
+
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert!(git::get_diff_base(&repo, &dir).is_err());
 }
 
 /// Test that `get_file_head` does not return content for a symlink.
@@ -116,6 +128,11 @@ fn symlink() {
     symlink("file.txt", &file_link).unwrap();
 
     create_commit(temp_git.path(), true);
-    assert!(git::get_diff_base(&file_link).is_err());
-    assert_eq!(git::get_diff_base(&file).unwrap(), Vec::from(contents));
+
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert!(git::get_diff_base(&repo, &file_link).is_err());
+    assert_eq!(
+        git::get_diff_base(&repo, &file).unwrap(),
+        Vec::from(contents)
+    );
 }