Yehowshua/ZIm
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Clear pending staged/unstaged diff hunks hunks when writing to the git index fails (#26173)

Browse source 
Release Notes:

- Git Beta: Fixed a bug where discarding a hunk in the project diff view
performed two concurrent saves of the buffer.
- Git Beta: Fixed an issue where diff hunks appeared in the wrong state
after failing to write to the git index.
This commit is contained in:
Max Brunsfeld 2025-03-05 18:45:09 -08:00 • committed by GitHub
parent d3c68650c0
commit 314ad5dd5f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
14 changed files with 534 additions and 143 deletions
Download patch file Download diff file Expand all files Collapse all files

4
crates/project/src/buffer_store.rs
View file

@ -339,6 +339,7 @@ enum OpenBuffer {
pub enum BufferStoreEvent {
BufferAdded(Entity<Buffer>),
BufferDiffAdded(Entity<BufferDiff>),
BufferDropped(BufferId),
BufferChangedFilePath {
buffer: Entity<Buffer>,
@ -1522,11 +1523,12 @@ impl BufferStore {
if let Some(OpenBuffer::Complete { diff_state, .. }) =
this.opened_buffers.get_mut(&buffer_id)
{
let diff = cx.new(|cx| BufferDiff::new(&text_snapshot, cx));
cx.emit(BufferStoreEvent::BufferDiffAdded(diff.clone()));
diff_state.update(cx, |diff_state, cx| {
diff_state.language = language;
diff_state.language_registry = language_registry;
let diff = cx.new(|cx| BufferDiff::new(&text_snapshot, cx));
match kind {
DiffKind::Unstaged => diff_state.unstaged_diff = Some(diff.downgrade()),
DiffKind::Uncommitted => {

134
crates/project/src/git.rs
View file

@ -1,24 +1,38 @@
use crate::buffer_store::BufferStore;
use crate::worktree_store::{WorktreeStore, WorktreeStoreEvent};
use crate::{Project, ProjectPath};
use crate::{
buffer_store::{BufferStore, BufferStoreEvent},
worktree_store::{WorktreeStore, WorktreeStoreEvent},
Project, ProjectItem, ProjectPath,
};
use anyhow::{Context as _, Result};
use buffer_diff::BufferDiffEvent;
use client::ProjectId;
use futures::channel::{mpsc, oneshot};
use futures::StreamExt as _;
use git::repository::{Branch, CommitDetails, PushOptions, Remote, RemoteCommandOutput, ResetMode};
use git::repository::{GitRepository, RepoPath};
use futures::{
channel::{mpsc, oneshot},
StreamExt as _,
};
use git::{
repository::{
Branch, CommitDetails, GitRepository, PushOptions, Remote, RemoteCommandOutput, RepoPath,
ResetMode,
},
status::FileStatus,
};
use gpui::{
App, AppContext, AsyncApp, Context, Entity, EventEmitter, SharedString, Subscription, Task,
WeakEntity,
};
use language::{Buffer, LanguageRegistry};
use rpc::proto::{git_reset, ToProto};
use rpc::{proto, AnyProtoClient, TypedEnvelope};
use rpc::{
proto::{self, git_reset, ToProto},
AnyProtoClient, TypedEnvelope,
};
use settings::WorktreeId;
use std::collections::VecDeque;
use std::future::Future;
use std::path::{Path, PathBuf};
use std::sync::Arc;
use std::{
collections::VecDeque,
future::Future,
path::{Path, PathBuf},
sync::Arc,
};
use text::BufferId;
use util::{maybe, ResultExt};
use worktree::{ProjectEntryId, RepositoryEntry, StatusEntry, WorkDirectory};
@ -30,7 +44,7 @@ pub struct GitStore {
repositories: Vec<Entity<Repository>>,
active_index: Option<usize>,
update_sender: mpsc::UnboundedSender<GitJob>,
_subscription: Subscription,
_subscriptions: [Subscription; 2],
}
pub struct Repository {
@ -54,10 +68,12 @@ pub enum GitRepo {
},
}
#[derive(Debug)]
pub enum GitEvent {
ActiveRepositoryChanged,
FileSystemUpdated,
GitStateUpdated,
IndexWriteError(anyhow::Error),
}
struct GitJob {
@ -81,7 +97,10 @@ impl GitStore {
cx: &mut Context<'_, Self>,
) -> Self {
let update_sender = Self::spawn_git_worker(cx);
let _subscription = cx.subscribe(worktree_store, Self::on_worktree_store_event);
let _subscriptions = [
cx.subscribe(worktree_store, Self::on_worktree_store_event),
cx.subscribe(&buffer_store, Self::on_buffer_store_event),
];
GitStore {
project_id,
@ -90,7 +109,7 @@ impl GitStore {
repositories: Vec::new(),
active_index: None,
update_sender,
_subscription,
_subscriptions,
}
}
@ -227,10 +246,82 @@ impl GitStore {
}
}
fn on_buffer_store_event(
&mut self,
_: Entity<BufferStore>,
event: &BufferStoreEvent,
cx: &mut Context<'_, Self>,
) {
if let BufferStoreEvent::BufferDiffAdded(diff) = event {
cx.subscribe(diff, Self::on_buffer_diff_event).detach();
}
}
fn on_buffer_diff_event(
this: &mut GitStore,
diff: Entity<buffer_diff::BufferDiff>,
event: &BufferDiffEvent,
cx: &mut Context<'_, GitStore>,
) {
if let BufferDiffEvent::HunksStagedOrUnstaged(new_index_text) = event {
let buffer_id = diff.read(cx).buffer_id;
if let Some((repo, path)) = this.repository_and_path_for_buffer_id(buffer_id, cx) {
let recv = repo
.read(cx)
.set_index_text(&path, new_index_text.as_ref().map(|rope| rope.to_string()));
let diff = diff.downgrade();
cx.spawn(|this, mut cx| async move {
if let Some(result) = cx.background_spawn(async move { recv.await.ok() }).await
{
if let Err(error) = result {
diff.update(&mut cx, |diff, cx| {
diff.clear_pending_hunks(cx);
})
.ok();
this.update(&mut cx, |_, cx| cx.emit(GitEvent::IndexWriteError(error)))
.ok();
}
}
})
.detach();
}
}
}
pub fn all_repositories(&self) -> Vec<Entity<Repository>> {
self.repositories.clone()
}
pub fn status_for_buffer_id(&self, buffer_id: BufferId, cx: &App) -> Option<FileStatus> {
let (repo, path) = self.repository_and_path_for_buffer_id(buffer_id, cx)?;
let status = repo.read(cx).repository_entry.status_for_path(&path)?;
Some(status.status)
}
fn repository_and_path_for_buffer_id(
&self,
buffer_id: BufferId,
cx: &App,
) -> Option<(Entity<Repository>, RepoPath)> {
let buffer = self.buffer_store.read(cx).get(buffer_id)?;
let path = buffer.read(cx).project_path(cx)?;
let mut result: Option<(Entity<Repository>, RepoPath)> = None;
for repo_handle in &self.repositories {
let repo = repo_handle.read(cx);
if repo.worktree_id == path.worktree_id {
if let Ok(relative_path) = repo.repository_entry.relativize(&path.path) {
if result
.as_ref()
.is_none_or(|(result, _)| !repo.contains_sub_repo(result, cx))
{
result = Some((repo_handle.clone(), relative_path))
}
}
}
}
result
}
fn spawn_git_worker(cx: &mut Context<'_, GitStore>) -> mpsc::UnboundedSender<GitJob> {
let (job_tx, mut job_rx) = mpsc::unbounded::<GitJob>();
@ -658,9 +749,8 @@ impl GitStore {
cx: &mut AsyncApp,
) -> Result<Entity<Repository>> {
this.update(cx, |this, cx| {
let repository_handle = this
.all_repositories()
.into_iter()
this.repositories
.iter()
.find(|repository_handle| {
repository_handle.read(cx).worktree_id == worktree_id
&& repository_handle
@ -669,8 +759,8 @@ impl GitStore {
.work_directory_id()
== work_directory_id
})
.context("missing repository handle")?;
anyhow::Ok(repository_handle)
.context("missing repository handle")
.cloned()
})?
}
}
@ -1297,7 +1387,7 @@ impl Repository {
})
}
pub fn set_index_text(
fn set_index_text(
&self,
path: &RepoPath,
content: Option<String>,

2
crates/project/src/lsp_store.rs
View file

@ -3184,7 +3184,7 @@ impl LspStore {
}
}
}
BufferStoreEvent::BufferDropped(_) => {}
_ => {}
}
}

39
crates/project/src/project.rs
View file

@ -46,11 +46,7 @@ use futures::{
pub use image_store::{ImageItem, ImageStore};
use image_store::{ImageItemEvent, ImageStoreEvent};
use ::git::{
blame::Blame,
repository::{GitRepository, RepoPath},
status::FileStatus,
};
use ::git::{blame::Blame, repository::GitRepository, status::FileStatus};
use gpui::{
AnyEntity, App, AppContext as _, AsyncApp, BorrowAppContext, Context, Entity, EventEmitter,
Hsla, SharedString, Task, WeakEntity, Window,
@ -2276,7 +2272,6 @@ impl Project {
BufferStoreEvent::BufferAdded(buffer) => {
self.register_buffer(buffer, cx).log_err();
}
BufferStoreEvent::BufferChangedFilePath { .. } => {}
BufferStoreEvent::BufferDropped(buffer_id) => {
if let Some(ref ssh_client) = self.ssh_client {
ssh_client
@ -2289,6 +2284,7 @@ impl Project {
.log_err();
}
}
_ => {}
}
}
@ -4336,35 +4332,8 @@ impl Project {
self.git_store.read(cx).all_repositories()
}
pub fn repository_and_path_for_buffer_id(
&self,
buffer_id: BufferId,
cx: &App,
) -> Option<(Entity<Repository>, RepoPath)> {
let path = self
.buffer_for_id(buffer_id, cx)?
.read(cx)
.project_path(cx)?;
let mut found: Option<(Entity<Repository>, RepoPath)> = None;
for repo_handle in self.git_store.read(cx).all_repositories() {
let repo = repo_handle.read(cx);
if repo.worktree_id != path.worktree_id {
continue;
}
let Ok(relative_path) = repo.repository_entry.relativize(&path.path) else {
continue;
};
if found
.as_ref()
.is_some_and(|(found, _)| repo.contains_sub_repo(found, cx))
{
continue;
}
found = Some((repo_handle.clone(), relative_path))
}
found
pub fn status_for_buffer_id(&self, buffer_id: BufferId, cx: &App) -> Option<FileStatus> {
self.git_store.read(cx).status_for_buffer_id(buffer_id, cx)
}
}

273
crates/project/src/project_tests.rs
View file

@ -1,5 +1,7 @@
use crate::{task_inventory::TaskContexts, Event, *};
use buffer_diff::{assert_hunks, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind};
use buffer_diff::{
assert_hunks, BufferDiffEvent, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind,
};
use fs::FakeFs;
use futures::{future, StreamExt};
use gpui::{App, SemanticVersion, UpdateGlobal};
@ -5786,7 +5788,7 @@ async fn test_unstaged_diff_for_buffer(cx: &mut gpui::TestAppContext) {
unstaged_diff.update(cx, |unstaged_diff, cx| {
let snapshot = buffer.read(cx).snapshot();
assert_hunks(
unstaged_diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx),
unstaged_diff.hunks(&snapshot, cx),
&snapshot,
&unstaged_diff.base_text_string().unwrap(),
&[
@ -6008,6 +6010,271 @@ async fn test_uncommitted_diff_for_buffer(cx: &mut gpui::TestAppContext) {
});
}
#[gpui::test]
async fn test_staging_hunks(cx: &mut gpui::TestAppContext) {
use DiffHunkSecondaryStatus::*;
init_test(cx);
let committed_contents = r#"
zero
one
two
three
four
five
"#
.unindent();
let file_contents = r#"
one
TWO
three
FOUR
five
"#
.unindent();
let fs = FakeFs::new(cx.background_executor.clone());
fs.insert_tree(
"/dir",
json!({
".git": {},
"file.txt": file_contents.clone()
}),
)
.await;
fs.set_head_for_repo(
"/dir/.git".as_ref(),
&[("file.txt".into(), committed_contents.clone())],
);
fs.set_index_for_repo(
"/dir/.git".as_ref(),
&[("file.txt".into(), committed_contents.clone())],
);
let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await;
let buffer = project
.update(cx, |project, cx| {
project.open_local_buffer("/dir/file.txt", cx)
})
.await
.unwrap();
let snapshot = buffer.read_with(cx, |buffer, _| buffer.snapshot());
let uncommitted_diff = project
.update(cx, |project, cx| {
project.open_uncommitted_diff(buffer.clone(), cx)
})
.await
.unwrap();
let mut diff_events = cx.events(&uncommitted_diff);
// The hunks are initially unstaged.
uncommitted_diff.read_with(cx, |diff, cx| {
assert_hunks(
diff.hunks(&snapshot, cx),
&snapshot,
&diff.base_text_string().unwrap(),
&[
(
0..0,
"zero\n",
"",
DiffHunkStatus::deleted(HasSecondaryHunk),
),
(
1..2,
"two\n",
"TWO\n",
DiffHunkStatus::modified(HasSecondaryHunk),
),
(
3..4,
"four\n",
"FOUR\n",
DiffHunkStatus::modified(HasSecondaryHunk),
),
],
);
});
// Stage a hunk. It appears as optimistically staged.
uncommitted_diff.update(cx, |diff, cx| {
let range =
snapshot.anchor_before(Point::new(1, 0))..snapshot.anchor_before(Point::new(2, 0));
let hunks = diff
.hunks_intersecting_range(range, &snapshot, cx)
.collect::<Vec<_>>();
diff.stage_or_unstage_hunks(true, &hunks, &snapshot, true, cx);
assert_hunks(
diff.hunks(&snapshot, cx),
&snapshot,
&diff.base_text_string().unwrap(),
&[
(
0..0,
"zero\n",
"",
DiffHunkStatus::deleted(HasSecondaryHunk),
),
(
1..2,
"two\n",
"TWO\n",
DiffHunkStatus::modified(SecondaryHunkRemovalPending),
),
(
3..4,
"four\n",
"FOUR\n",
DiffHunkStatus::modified(HasSecondaryHunk),
),
],
);
});
// The diff emits a change event for the range of the staged hunk.
assert!(matches!(
diff_events.next().await.unwrap(),
BufferDiffEvent::HunksStagedOrUnstaged(_)
));
let event = diff_events.next().await.unwrap();
if let BufferDiffEvent::DiffChanged {
changed_range: Some(changed_range),
} = event
{
let changed_range = changed_range.to_point(&snapshot);
assert_eq!(changed_range, Point::new(1, 0)..Point::new(2, 0));
} else {
panic!("Unexpected event {event:?}");
}
// When the write to the index completes, it appears as staged.
cx.run_until_parked();
uncommitted_diff.update(cx, |diff, cx| {
assert_hunks(
diff.hunks(&snapshot, cx),
&snapshot,
&diff.base_text_string().unwrap(),
&[
(
0..0,
"zero\n",
"",
DiffHunkStatus::deleted(HasSecondaryHunk),
),
(1..2, "two\n", "TWO\n", DiffHunkStatus::modified(None)),
(
3..4,
"four\n",
"FOUR\n",
DiffHunkStatus::modified(HasSecondaryHunk),
),
],
);
});
// The diff emits a change event for the changed index text.
let event = diff_events.next().await.unwrap();
if let BufferDiffEvent::DiffChanged {
changed_range: Some(changed_range),
} = event
{
let changed_range = changed_range.to_point(&snapshot);
assert_eq!(changed_range, Point::new(0, 0)..Point::new(5, 0));
} else {
panic!("Unexpected event {event:?}");
}
// Simulate a problem writing to the git index.
fs.set_error_message_for_index_write(
"/dir/.git".as_ref(),
Some("failed to write git index".into()),
);
// Stage another hunk.
uncommitted_diff.update(cx, |diff, cx| {
let range =
snapshot.anchor_before(Point::new(3, 0))..snapshot.anchor_before(Point::new(4, 0));
let hunks = diff
.hunks_intersecting_range(range, &snapshot, cx)
.collect::<Vec<_>>();
diff.stage_or_unstage_hunks(true, &hunks, &snapshot, true, cx);
assert_hunks(
diff.hunks(&snapshot, cx),
&snapshot,
&diff.base_text_string().unwrap(),
&[
(
0..0,
"zero\n",
"",
DiffHunkStatus::deleted(HasSecondaryHunk),
),
(1..2, "two\n", "TWO\n", DiffHunkStatus::modified(None)),
(
3..4,
"four\n",
"FOUR\n",
DiffHunkStatus::modified(SecondaryHunkRemovalPending),
),
],
);
});
assert!(matches!(
diff_events.next().await.unwrap(),
BufferDiffEvent::HunksStagedOrUnstaged(_)
));
let event = diff_events.next().await.unwrap();
if let BufferDiffEvent::DiffChanged {
changed_range: Some(changed_range),
} = event
{
let changed_range = changed_range.to_point(&snapshot);
assert_eq!(changed_range, Point::new(3, 0)..Point::new(4, 0));
} else {
panic!("Unexpected event {event:?}");
}
// When the write fails, the hunk returns to being unstaged.
cx.run_until_parked();
uncommitted_diff.update(cx, |diff, cx| {
assert_hunks(
diff.hunks(&snapshot, cx),
&snapshot,
&diff.base_text_string().unwrap(),
&[
(
0..0,
"zero\n",
"",
DiffHunkStatus::deleted(HasSecondaryHunk),
),
(1..2, "two\n", "TWO\n", DiffHunkStatus::modified(None)),
(
3..4,
"four\n",
"FOUR\n",
DiffHunkStatus::modified(HasSecondaryHunk),
),
],
);
});
let event = diff_events.next().await.unwrap();
if let BufferDiffEvent::DiffChanged {
changed_range: Some(changed_range),
} = event
{
let changed_range = changed_range.to_point(&snapshot);
assert_eq!(changed_range, Point::new(0, 0)..Point::new(5, 0));
} else {
panic!("Unexpected event {event:?}");
}
}
#[gpui::test]
async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) {
init_test(cx);
@ -6065,7 +6332,7 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) {
uncommitted_diff.update(cx, |uncommitted_diff, cx| {
let snapshot = buffer.read(cx).snapshot();
assert_hunks(
uncommitted_diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx),
uncommitted_diff.hunks(&snapshot, cx),
&snapshot,
&uncommitted_diff.base_text_string().unwrap(),
&[(