feat: Diff source autodetection and caching

helix-term/src/commands.rs

@@ -79,6 +79,7 @@ use std::{
     future::Future,
     io::Read,
     num::NonZeroUsize,
+    sync::Arc,
 };
 
 use std::{
@@ -3481,15 +3482,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");
@@ -3568,17 +3569,24 @@ fn changed_file_picker(cx: &mut Context) {
             helix_loader::workspace_trust::TrustQuery::Git,
         )
         .is_trusted();
-    cx.editor
-        .diff_providers
-        .clone()
-        .for_each_changed_file(cwd, trust_full, 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, trust_full);
+    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/typed.rs

@@ -1593,7 +1593,7 @@ fn reload(cx: &mut compositor::Context, _args: Args, event: PromptEvent) -> anyh
     let scrolloff = cx.editor.config().scrolloff;
     let trust_full = doc_trust_full(cx.editor);
     let (view, doc) = current!(cx.editor);
-    doc.reload(view, &cx.editor.diff_providers, trust_full)
+    doc.reload(view, &mut cx.editor.diff_providers, trust_full)
         .map(|_| {
             view.ensure_cursor_in_view(doc, scrolloff);
         })?;
@@ -1629,6 +1629,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);
 
@@ -1647,7 +1649,7 @@ fn reload_all(cx: &mut compositor::Context, _args: Args, event: PromptEvent) ->
                 helix_loader::workspace_trust::TrustQuery::Git,
             )
             .is_trusted();
-        if let Err(error) = doc.reload(view, &cx.editor.diff_providers, trust_full) {
+        if let Err(error) = doc.reload(view, &mut cx.editor.diff_providers, trust_full) {
             cx.editor.set_error(format!("{}", error));
             continue;
         }

helix-vcs/Cargo.toml

@@ -17,10 +17,9 @@ tokio = { version = "1", features = ["rt", "rt-multi-thread", "time", "sync", "p
 parking_lot.workspace = true
 arc-swap.workspace = true
 
-gix = { version = "0.85.0", features = ["attributes", "status", "max-performance", "sha1"], default-features = false, optional = true }
+gix = { version = "0.85.0", features = ["attributes", "parallel", "status", "max-performance", "sha1"], default-features = false, optional = true }
 imara-diff =  "0.2.0"
 anyhow = "1"
-
 log = "0.4"
 
 [features]

helix-vcs/src/git.rs

@@ -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, trust_full: bool) -> Result<Vec<u8>> {
+pub(super) 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 repo_dir = get_repo_dir(&file)?;
-    let repo = open_repo(repo_dir, trust_full)
-        .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)?;
 
@@ -65,15 +55,8 @@ pub fn get_diff_base(file: &Path, trust_full: bool) -> Result<Vec<u8>> {
     }
 }
 
-pub fn get_current_head_name(file: &Path, trust_full: bool) -> 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, trust_full)
-        .context("failed to open git repo")?
-        .to_thread_local();
+pub(super) 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()?;
 
@@ -85,15 +68,14 @@ pub fn get_current_head_name(file: &Path, trust_full: bool) -> Result<Arc<ArcSwa
     Ok(Arc::new(ArcSwap::from_pointee(name.into_boxed_str())))
 }
 
-pub fn for_each_changed_file(
-    cwd: &Path,
-    trust_full: bool,
+pub(super) fn for_each_changed_file(
+    repo: &ThreadSafeRepository,
     f: impl Fn(Result<FileChange>) -> bool,
 ) -> Result<()> {
-    status(&open_repo(cwd, trust_full)?.to_thread_local(), f)
+    status(&repo.to_thread_local(), f)
 }
 
-fn open_repo(path: &Path, trust_full: bool) -> Result<ThreadSafeRepository> {
+pub(super) fn open_repo(path: &Path, trust_full: bool) -> Result<ThreadSafeRepository> {
     // `trust_full` is the workspace-trust decision made by the caller, and it must be the
     // authority on the gix trust level. gix's own discovery (`discover_*`) ignores a
     // caller-supplied trust level: it always re-derives trust from `.git` ownership, so a malicious
@@ -110,6 +92,10 @@ fn open_repo(path: &Path, trust_full: bool) -> Result<ThreadSafeRepository> {
         gix::sec::Trust::Reduced
     };
 
+    // 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)?;
+
     // On Windows various configuration options are bundled as part of the git installation. The
     // lookup is expensive; only do it there.
     let config = gix::open::permissions::Config {
@@ -130,7 +116,7 @@ fn open_repo(path: &Path, trust_full: bool) -> Result<ThreadSafeRepository> {
         dot_git_only: true,
         ..Default::default()
     };
-    let (repo_path, _trust_from_ownership) = gix::discover::upwards_opts(path, discover_options)
+    let (repo_path, _trust_from_ownership) = gix::discover::upwards_opts(&path, discover_options)
         .context("failed to discover git repo")?;
     let (git_dir, _work_dir) = repo_path.into_repository_and_work_tree_directories();
 

helix-vcs/src/git/test.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, true).is_err());
+    let repo = git::open_repo(temp_git.path(), true).unwrap();
+    assert!(git::get_diff_base(&repo, &file).is_err());
 }
 
 #[test]
@@ -64,8 +65,10 @@ fn unmodified_file() {
     let contents = b"foo".as_slice();
     File::create(&file).unwrap().write_all(contents).unwrap();
     create_commit(temp_git.path(), true);
+
+    let repo = git::open_repo(temp_git.path(), true).unwrap();
     assert_eq!(
-        git::get_diff_base(&file, true).unwrap(),
+        git::get_diff_base(&repo, &file).unwrap(),
         Vec::from(contents)
     );
 }
@@ -79,8 +82,9 @@ fn modified_file() {
     create_commit(temp_git.path(), true);
     File::create(&file).unwrap().write_all(b"bar").unwrap();
 
+    let repo = git::open_repo(temp_git.path(), true).unwrap();
     assert_eq!(
-        git::get_diff_base(&file, true).unwrap(),
+        git::get_diff_base(&repo, &file).unwrap(),
         Vec::from(contents)
     );
 }
@@ -101,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, true).is_err());
+
+    let repo = git::open_repo(temp_git.path(), true).unwrap();
+    assert!(git::get_diff_base(&repo, &dir).is_err());
 }
 
 /// Test that `get_diff_base` resolves symlinks so that the same diff base is
@@ -128,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, true).unwrap(), contents);
-    assert_eq!(git::get_diff_base(&file, true).unwrap(), contents);
+    let repo = git::open_repo(temp_git.path(), true).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
@@ -153,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, true).unwrap(), contents);
-    assert_eq!(git::get_diff_base(&file, true).unwrap(), contents);
+    let repo = git::open_repo(temp_git.path(), true).unwrap();
+    assert_eq!(git::get_diff_base(&repo, &file_link).unwrap(), contents);
+    assert_eq!(git::get_diff_base(&repo, &file).unwrap(), contents);
 }