git: Fix diff hunks not being removed on restore in remote projects (#54823)
Closes #48032 When restoring a diff hunk, we first unstage it unconditionally. That unstaging operation is a no-op in terms of the index text if the hunk was already not staged, but previously we would still always do `spawn_set_index_text_job` and bump the `hunk_staging_operation_count_as_of_write`. Bumping that count in turn causes us to skip a diff recalculation in response to the change in the buffer's text. That works out fine in the local case, because when the worktree picks up the write to `.git/index` we kick off another diff recalculation which is not skipped. But in the remote case, we don't get an `UpdateDiffBases` proto message if the index text didn't actually change, so there is no subsequent diff calculation to do the cleanup, and we end up with a stale no-op hunk. This PR fixes the issue by skipping the write to the index and the `hunk_staging_operation_count_as_of_write` bump if the new and old index texts are the same. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #ISSUE Release Notes: - Fixed a bug where restoring diff hunks in remote projects would leave stale no-op hunks in the UI.
This commit is contained in:
1 parent
9ca30277f5
commit
ec2c637aaf
4 files changed
+126
-5
No files matched your search
@@ -16,7 +16,7 @@ impl Editor {
|
||||
if !self.lsp_data_enabled() || !self.use_document_folding_ranges {
|
||||
return;
|
||||
}
|
||||
let Some(project) = self.project.clone() else {
|
||||
let Some(project) = self.project.as_ref().map(|p| p.downgrade()) else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -43,7 +43,8 @@ impl Editor {
|
||||
|
||||
let Some(tasks) = editor
|
||||
.update(cx, |_, cx| {
|
||||
project.read(cx).lsp_store().update(cx, |lsp_store, cx| {
|
||||
let project = project.upgrade()?;
|
||||
Some(project.read(cx).lsp_store().update(cx, |lsp_store, cx| {
|
||||
buffers_to_query
|
||||
.into_iter()
|
||||
.map(|buffer| {
|
||||
@@ -52,9 +53,10 @@ impl Editor {
|
||||
async move { (buffer_id, task.await) }
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
}))
|
||||
})
|
||||
.ok()
|
||||
.flatten()
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
||||
@@ -142,7 +142,10 @@ impl Editor {
|
||||
);
|
||||
}
|
||||
|
||||
let Some((sema, project)) = self.semantics_provider.clone().zip(self.project.clone())
|
||||
let Some((sema, project)) = self
|
||||
.semantics_provider
|
||||
.clone()
|
||||
.zip(self.project.as_ref().map(|p| p.downgrade()))
|
||||
else {
|
||||
return;
|
||||
};
|
||||
@@ -283,6 +286,9 @@ impl Editor {
|
||||
.buffer(buffer_id)
|
||||
.and_then(|buf| buf.read(cx).language().map(|l| l.name()));
|
||||
|
||||
let Some(project) = project.upgrade() else {
|
||||
return;
|
||||
};
|
||||
editor.display_map.update(cx, |display_map, cx| {
|
||||
project.read(cx).lsp_store().update(cx, |lsp_store, cx| {
|
||||
let mut token_highlights = Vec::new();
|
||||
|
||||
@@ -1848,6 +1848,10 @@ impl GitStore {
|
||||
if let BufferDiffEvent::HunksStagedOrUnstaged(new_index_text) = event {
|
||||
let buffer_id = diff.read(cx).buffer_id;
|
||||
if let Some(diff_state) = self.diffs.get(&buffer_id) {
|
||||
let new_index_text = new_index_text.as_ref().map(|rope| rope.to_string());
|
||||
if new_index_text.as_deref() == diff_state.read(cx).index_text.as_deref() {
|
||||
return;
|
||||
}
|
||||
let hunk_staging_operation_count = diff_state.update(cx, |diff_state, _| {
|
||||
diff_state.hunk_staging_operation_count += 1;
|
||||
diff_state.hunk_staging_operation_count
|
||||
@@ -1857,7 +1861,7 @@ impl GitStore {
|
||||
log::debug!("hunks changed for {}", path.as_unix_str());
|
||||
repo.spawn_set_index_text_job(
|
||||
path,
|
||||
new_index_text.as_ref().map(|rope| rope.to_string()),
|
||||
new_index_text,
|
||||
Some(hunk_staging_operation_count),
|
||||
cx,
|
||||
)
|
||||
|
||||
@@ -2624,6 +2624,115 @@ async fn test_remote_apply_code_action_skips_unadvertised_command(
|
||||
assert_eq!(transaction.0.len(), 0);
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_remote_restore_unstaged_hunk_clears_diff(
|
||||
cx: &mut TestAppContext,
|
||||
server_cx: &mut TestAppContext,
|
||||
) {
|
||||
cx.update(|cx| {
|
||||
let settings_store = SettingsStore::test(cx);
|
||||
cx.set_global(settings_store);
|
||||
theme_settings::init(theme::LoadThemes::JustBase, cx);
|
||||
release_channel::init(semver::Version::new(0, 0, 0), cx);
|
||||
editor::init(cx);
|
||||
});
|
||||
|
||||
use editor::Editor;
|
||||
use gpui::VisualContext;
|
||||
|
||||
let base_text = "
|
||||
fn one() -> usize {
|
||||
1
|
||||
}
|
||||
"
|
||||
.unindent();
|
||||
let modified_text = "
|
||||
fn one() -> usize {
|
||||
100
|
||||
}
|
||||
"
|
||||
.unindent();
|
||||
|
||||
let fs = FakeFs::new(server_cx.executor());
|
||||
fs.insert_tree(
|
||||
path!("/code"),
|
||||
json!({
|
||||
"project1": {
|
||||
".git": {},
|
||||
"src": {
|
||||
"lib.rs": modified_text
|
||||
},
|
||||
},
|
||||
}),
|
||||
)
|
||||
.await;
|
||||
fs.set_index_for_repo(
|
||||
Path::new(path!("/code/project1/.git")),
|
||||
&[("src/lib.rs", base_text.clone())],
|
||||
);
|
||||
fs.set_head_for_repo(
|
||||
Path::new(path!("/code/project1/.git")),
|
||||
&[("src/lib.rs", base_text.clone())],
|
||||
"deadbeef",
|
||||
);
|
||||
|
||||
let (project, _headless) = init_test(&fs, cx, server_cx).await;
|
||||
let worktree_id = {
|
||||
let (worktree, _) = project
|
||||
.update(cx, |project, cx| {
|
||||
project.find_or_create_worktree(path!("/code/project1"), true, cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
cx.update(|cx| worktree.read(cx).id())
|
||||
};
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
let buffer = project
|
||||
.update(cx, |project, cx| {
|
||||
project.open_buffer((worktree_id, rel_path("src/lib.rs")), cx)
|
||||
})
|
||||
.await
|
||||
.unwrap();
|
||||
|
||||
let cx = cx.add_empty_window();
|
||||
let editor = cx.new_window_entity(|window, cx| {
|
||||
Editor::for_buffer(buffer, Some(project.clone()), window, cx)
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
editor.update_in(cx, |editor, window, cx| {
|
||||
let snapshot = editor.snapshot(window, cx);
|
||||
let hunks: Vec<_> = editor
|
||||
.diff_hunks_in_ranges(
|
||||
&[editor::Anchor::Min..editor::Anchor::Max],
|
||||
&snapshot.buffer_snapshot(),
|
||||
)
|
||||
.collect();
|
||||
assert!(!hunks.is_empty(), "should have diff hunks before restore");
|
||||
});
|
||||
|
||||
cx.update_window_entity(&editor, |editor, window, cx| {
|
||||
editor.select_all(&editor::actions::SelectAll, window, cx);
|
||||
editor.git_restore(&git::Restore, window, cx);
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
editor.update_in(cx, |editor, _window, cx| {
|
||||
let snapshot = editor.buffer().read(cx).snapshot(cx);
|
||||
assert_eq!(
|
||||
snapshot.text(),
|
||||
base_text,
|
||||
"buffer text should match base after restoring all hunks"
|
||||
);
|
||||
|
||||
let hunks: Vec<_> = editor
|
||||
.diff_hunks_in_ranges(&[editor::Anchor::Min..editor::Anchor::Max], &snapshot)
|
||||
.collect();
|
||||
assert!(hunks.is_empty(), "should have no diff hunks after restore");
|
||||
});
|
||||
}
|
||||
|
||||
pub async fn init_test(
|
||||
server_fs: &Arc<FakeFs>,
|
||||
cx: &mut TestAppContext,
|
||||
|
||||
Reference in new issue
Block a user