Remove git diff base from symlinked files (#10037)

Closes https://github.com/zed-industries/zed/issues/4730

![image](https://github.com/zed-industries/zed/assets/2690773/d3c5317f-8120-45b5-b57c-c0fb5d8c066d)

To the left is a symlink, to the right — the real file.
The issue was due to the fact, that symlinks files contain the file path
to the real file, and git (properly) treats that symlink file contents
as diff base, returning in `load_index_text` (via `let content =
repo.find_blob(oid)?.content().to_owned();`) the contents of that
symlink file — the path.

The fix checks for FS metadata before fetching the git diff base, and
skips it entirely for symlinks: Zed opens the symlink file contents
instead, fully obscuring the git symlink diff hunks.

Interesting, that VSCode behaves as Zed before the fix; while the fix
makes Zed behave like Intellij* IDEs now.

Release Notes:

- Fixed git diff hunks appearing in the symlinked files
([4730](https://github.com/zed-industries/zed/issues/4730))
This commit is contained in:
Kirill Bulatov 2024-04-01 17:22:25 +02:00 committed by GitHub
parent 351693ccdf
commit 63e566e56e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 60 additions and 13 deletions

View File

@ -25,6 +25,9 @@ pub struct Branch {
pub trait GitRepository: Send {
fn reload_index(&self);
/// Loads a git repository entry's contents.
/// Note that for symlink entries, this will return the contents of the symlink, not the target.
fn load_index_text(&self, relative_file_path: &Path) -> Option<String>;
/// Returns the URL of the remote with the given name.

View File

@ -7107,11 +7107,12 @@ impl Project {
.find(|(work_dir, _)| path.starts_with(work_dir))?;
let receiver = receiver.clone();
let path = path.clone();
let abs_path = worktree_handle.read(cx).absolutize(&path).ok()?;
Some(async move {
wait_for_loading_buffer(receiver)
.await
.ok()
.map(|buffer| (buffer, path))
.map(|buffer| (buffer, path, abs_path))
})
})
.collect::<FuturesUnordered<_>>();
@ -7130,7 +7131,7 @@ impl Project {
changed_repos
.iter()
.find(|(work_dir, _)| path.starts_with(work_dir))?;
Some((buffer, path.clone()))
Some((buffer, path.clone(), file.abs_path(cx)))
})
.collect::<Vec<_>>();
@ -7140,6 +7141,7 @@ impl Project {
let remote_id = self.remote_id();
let client = self.client.clone();
let fs = self.fs.clone();
cx.spawn(move |_, mut cx| async move {
// Wait for all of the buffers to load.
let future_buffers = future_buffers.collect::<Vec<_>>().await;
@ -7150,20 +7152,47 @@ impl Project {
let diff_bases_by_buffer = cx
.background_executor()
.spawn(async move {
future_buffers
let mut diff_base_tasks = future_buffers
.into_iter()
.flatten()
.chain(current_buffers)
.filter_map(|(buffer, path)| {
.filter_map(|(buffer, path, abs_path)| {
let (work_directory, repo) =
snapshot.repository_and_work_directory_for_path(&path)?;
let repo_entry = snapshot.get_local_repo(&repo)?;
let relative_path = path.strip_prefix(&work_directory).ok()?;
let base_text = repo_entry.repo().lock().load_index_text(relative_path);
Some((buffer, base_text))
Some((buffer, path, abs_path, work_directory, repo_entry))
})
.collect::<Vec<_>>()
.map(|(buffer, path, abs_path, work_directory, repo_entry)| {
let fs = fs.clone();
async move {
let abs_path_metadata = fs
.metadata(&abs_path)
.await
.with_context(|| {
format!("loading file and FS metadata for {path:?}")
})
.log_err()
.flatten()?;
let base_text = if abs_path_metadata.is_dir
|| abs_path_metadata.is_symlink
{
None
} else {
let relative_path = path.strip_prefix(&work_directory).ok()?;
repo_entry.repo().lock().load_index_text(relative_path)
};
Some((buffer, base_text))
}
})
.collect::<FuturesUnordered<_>>();
let mut diff_bases = Vec::with_capacity(diff_base_tasks.len());
while let Some(diff_base) = diff_base_tasks.next().await {
if let Some(diff_base) = diff_base {
diff_bases.push(diff_base);
}
}
diff_bases
})
.await;

View File

@ -1081,10 +1081,25 @@ impl LocalWorktree {
{
if let Some(git_repo) = snapshot.git_repositories.get(&*repo.work_directory) {
let git_repo = git_repo.repo_ptr.clone();
index_task = Some(
cx.background_executor()
.spawn(async move { git_repo.lock().load_index_text(&repo_path) }),
);
index_task = Some(cx.background_executor().spawn({
let fs = fs.clone();
let abs_path = abs_path.clone();
async move {
let abs_path_metadata = fs
.metadata(&abs_path)
.await
.with_context(|| {
format!("loading file and FS metadata for {abs_path:?}")
})
.log_err()
.flatten()?;
if abs_path_metadata.is_dir || abs_path_metadata.is_symlink {
None
} else {
git_repo.lock().load_index_text(&repo_path)
}
}
}));
}
}
}