feat: Diff source caching and autodetection

Author: poliorcetics

Cargo.lock

@@ -74,6 +74,7 @@ use std::{
     future::Future,
     io::Read,
     num::NonZeroUsize,
+    sync::Arc,
 };
 
 use std::{
@@ -3292,15 +3293,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");
@@ -3371,17 +3372,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

@@ -1329,7 +1329,7 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh
 
     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() {
@@ -1364,6 +1364,8 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) ->
         })
         .collect();
 
+    cx.editor.diff_providers.reset();
+
     for (doc_id, view_ids) in docs_view_ids {
         let doc = doc_mut!(cx.editor, &doc_id);
 
@@ -1373,7 +1375,7 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) ->
         // 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.workspace = true
 arc-swap = { version = "1.7.1" }
 
-gix = { version = "0.72.1", features = ["attributes", "status"], default-features = false, optional = true }
+gix = { version = "0.72.1", features = ["attributes", "parallel", "status"], default-features = false, optional = true }
 imara-diff = "0.1.8"
 anyhow = "1"
 

helix-vcs/Cargo.toml

@@ -22,22 +22,12 @@ use crate::FileChange;
 #[cfg(test)]
 mod test;
 
-#[inline]
-fn get_repo_dir(file: &Path) -> Result<&Path> {
-    file.parent().context("file has no parent directory")
-}
-
-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());
-    let file = gix::path::realpath(file).context("resolve symlinks")?;
-
-    // TODO cache repository lookup
+    let file = gix::path::realpath(file).context("resolve symlink")?;
 
-    let repo_dir = get_repo_dir(&file)?;
-    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)?;
 
@@ -59,15 +49,8 @@ pub fn get_diff_base(file: &Path) -> Result<Vec<u8>> {
     }
 }
 
-pub fn get_current_head_name(file: &Path) -> Result<Arc<ArcSwap<Box<str>>>> {
-    debug_assert!(!file.exists() || file.is_file());
-    debug_assert!(file.is_absolute());
-    let file = gix::path::realpath(file).context("resolve symlinks")?;
-
-    let repo_dir = get_repo_dir(&file)?;
-    let repo = open_repo(repo_dir)
-        .context("failed to open git repo")?
-        .to_thread_local();
+pub fn get_current_head_name(repo: &ThreadSafeRepository) -> Result<Arc<ArcSwap<Box<str>>>> {
+    let repo = repo.to_thread_local();
     let head_ref = repo.head_ref()?;
     let head_commit = repo.head_commit()?;
 
@@ -79,11 +62,17 @@ 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> {
+    // Ensure the repo itself is an absolute real path, else we'll not match prefixes with
+    // symlink-resolved files in `get_diff_base()` above.
+    let path = gix::path::realpath(path)?;
     // 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_diff_base` resolves symlinks so that the same diff base is
@@ -122,8 +134,9 @@ fn symlink() {
     symlink("file.txt", &file_link).unwrap();
     create_commit(temp_git.path(), true);
 
-    assert_eq!(git::get_diff_base(&file_link).unwrap(), contents);
-    assert_eq!(git::get_diff_base(&file).unwrap(), contents);
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert_eq!(git::get_diff_base(&repo, &file_link).unwrap(), contents);
+    assert_eq!(git::get_diff_base(&repo, &file).unwrap(), contents);
 }
 
 /// Test that `get_diff_base` returns content when the file is a symlink to
@@ -147,6 +160,7 @@ fn symlink_to_git_repo() {
     let file_link = temp_dir.path().join("file_link.txt");
     symlink(&file, &file_link).unwrap();
 
-    assert_eq!(git::get_diff_base(&file_link).unwrap(), contents);
-    assert_eq!(git::get_diff_base(&file).unwrap(), contents);
+    let repo = git::open_repo(temp_git.path()).unwrap();
+    assert_eq!(git::get_diff_base(&repo, &file_link).unwrap(), contents);
+    assert_eq!(git::get_diff_base(&repo, &file).unwrap(), contents);
 }