From 0c2bbb3aa94373b8800bef6ea4a12146dc0b6844 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 28 Feb 2025 12:55:29 -0800 Subject: [PATCH] Optimistically update hunk states when staging and unstaging hunks (#25687) This PR adds an optimistic update when staging or unstaging diff hunks. In the process, I've also refactored the logic for staging and unstaging hunks, to consolidate more of it in the `buffer_diff` crate. I've also changed the way that we treat untracked files. Previously, we maintained an empty diff for them, so as not to show unwanted entire-file diff hunks in a regular editor. But then in the project diff view, we had to account for this, and replace these empty diffs with entire-file diffs. This form of state management made it more difficult to store the pending hunks, so now we always use the same `BufferDiff`/`BufferDiffSnapshot` for untracked files (with a single hunk spanning the entire buffer), but we just have a special case in regular buffers, that avoids showing that entire-file hunk. * [x] Avoid creating a long queue of `set_index` operations when staging/unstaging rapidly * [x] Keep pending hunks when diff is recalculated without base text changes * [x] Be optimistic even when staging the single hunk in added/deleted files * Testing Release Notes: - N/A --------- Co-authored-by: Cole Miller --- Cargo.lock | 1 + crates/buffer_diff/Cargo.toml | 1 + crates/buffer_diff/src/buffer_diff.rs | 576 ++++++++++-------- crates/editor/src/editor.rs | 70 +-- crates/editor/src/editor_tests.rs | 64 +- crates/editor/src/element.rs | 141 +++-- crates/editor/src/proposed_changes_editor.rs | 2 +- crates/git_ui/src/project_diff.rs | 42 +- crates/multi_buffer/src/anchor.rs | 6 +- crates/multi_buffer/src/multi_buffer.rs | 99 ++- crates/multi_buffer/src/multi_buffer_tests.rs | 7 +- crates/project/src/buffer_store.rs | 101 +-- crates/project/src/git.rs | 108 +++- crates/project/src/project_tests.rs | 138 +++-- 14 files changed, 756 insertions(+), 600 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 821a8f9e476b2ca7d1c6c5e957aaccc1473ccce0..9a11f1606b14612f6c72e9c1e3f1f825f3a5d3d1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2076,6 +2076,7 @@ name = "buffer_diff" version = "0.1.0" dependencies = [ "anyhow", + "clock", "ctor", "env_logger 0.11.6", "futures 0.3.31", diff --git a/crates/buffer_diff/Cargo.toml b/crates/buffer_diff/Cargo.toml index 9d4691afd25644d09da8038e16fa21614e5e36dc..c92688698a6bc0634786387074cd414786748808 100644 --- a/crates/buffer_diff/Cargo.toml +++ b/crates/buffer_diff/Cargo.toml @@ -16,6 +16,7 @@ test-support = [] [dependencies] anyhow.workspace = true +clock.workspace = true futures.workspace = true git2.workspace = true gpui.workspace = true diff --git a/crates/buffer_diff/src/buffer_diff.rs b/crates/buffer_diff/src/buffer_diff.rs index 5a3264bb989d3e7748505c0a0ec9cab44f6e6323..84423255358ffe261e877124c573630b878ff9fe 100644 --- a/crates/buffer_diff/src/buffer_diff.rs +++ b/crates/buffer_diff/src/buffer_diff.rs @@ -1,11 +1,12 @@ -use futures::{channel::oneshot, future::OptionFuture}; +use futures::channel::oneshot; use git2::{DiffLineType as GitDiffLineType, DiffOptions as GitOptions, Patch as GitPatch}; -use gpui::{App, AppContext as _, AsyncApp, Context, Entity, EventEmitter}; +use gpui::{App, AppContext as _, AsyncApp, Context, Entity, EventEmitter, Task}; use language::{Language, LanguageRegistry}; use rope::Rope; use std::cmp::Ordering; +use std::mem; use std::{future::Future, iter, ops::Range, sync::Arc}; -use sum_tree::SumTree; +use sum_tree::{SumTree, TreeMap}; use text::ToOffset as _; use text::{Anchor, Bias, BufferId, OffsetRangeExt, Point}; use util::ResultExt; @@ -20,13 +21,14 @@ pub struct BufferDiff { pub struct BufferDiffSnapshot { inner: BufferDiffInner, secondary_diff: Option>, - pub is_single_insertion: bool, } #[derive(Clone)] struct BufferDiffInner { hunks: SumTree, - base_text: Option, + pending_hunks: TreeMap, + base_text: language::BufferSnapshot, + base_text_exists: bool, } #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -47,16 +49,8 @@ pub enum DiffHunkSecondaryStatus { HasSecondaryHunk, OverlapsWithSecondaryHunk, None, -} - -impl DiffHunkSecondaryStatus { - pub fn is_secondary(&self) -> bool { - match self { - DiffHunkSecondaryStatus::HasSecondaryHunk => true, - DiffHunkSecondaryStatus::OverlapsWithSecondaryHunk => true, - DiffHunkSecondaryStatus::None => false, - } - } + SecondaryHunkAdditionPending, + SecondaryHunkRemovalPending, } /// A diff hunk resolved to rows in the buffer. @@ -78,6 +72,17 @@ struct InternalDiffHunk { diff_base_byte_range: Range, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct PendingHunk { + buffer_version: clock::Global, + new_status: DiffHunkSecondaryStatus, +} + +#[derive(Debug, Default, Clone)] +pub struct DiffHunkSummary { + buffer_range: Range, +} + impl sum_tree::Item for InternalDiffHunk { type Summary = DiffHunkSummary; @@ -88,11 +93,6 @@ impl sum_tree::Item for InternalDiffHunk { } } -#[derive(Debug, Default, Clone)] -pub struct DiffHunkSummary { - buffer_range: Range, -} - impl sum_tree::Summary for DiffHunkSummary { type Context = text::BufferSnapshot; @@ -159,131 +159,166 @@ impl BufferDiffSnapshot { self.inner.hunks_intersecting_range_rev(range, buffer) } - pub fn base_text(&self) -> Option<&language::BufferSnapshot> { - self.inner.base_text.as_ref() + pub fn base_text(&self) -> &language::BufferSnapshot { + &self.inner.base_text } pub fn base_texts_eq(&self, other: &Self) -> bool { - match (other.base_text(), self.base_text()) { - (None, None) => true, - (None, Some(_)) => false, - (Some(_), None) => false, - (Some(old), Some(new)) => { - let (old_id, old_empty) = (old.remote_id(), old.is_empty()); - let (new_id, new_empty) = (new.remote_id(), new.is_empty()); - new_id == old_id || (new_empty && old_empty) - } + if self.inner.base_text_exists != other.inner.base_text_exists { + return false; } + let left = &self.inner.base_text; + let right = &other.inner.base_text; + let (old_id, old_empty) = (left.remote_id(), left.is_empty()); + let (new_id, new_empty) = (right.remote_id(), right.is_empty()); + new_id == old_id || (new_empty && old_empty) } +} - pub fn new_secondary_text_for_stage_or_unstage( - &self, +impl BufferDiffInner { + fn stage_or_unstage_hunks( + &mut self, + unstaged_diff: &Self, stage: bool, - hunks: impl Iterator, Range)>, + hunks: &[DiffHunk], buffer: &text::BufferSnapshot, - cx: &mut App, - ) -> Option { - let secondary_diff = self.secondary_diff()?; - let head_text = self.base_text().map(|text| text.as_rope().clone()); - let index_text = secondary_diff - .base_text() - .map(|text| text.as_rope().clone()); + file_exists: bool, + ) -> (Option, Vec<(usize, PendingHunk)>) { + let head_text = self + .base_text_exists + .then(|| self.base_text.as_rope().clone()); + let index_text = unstaged_diff + .base_text_exists + .then(|| unstaged_diff.base_text.as_rope().clone()); + + // If the file doesn't exist in either HEAD or the index, then the + // entire file must be either created or deleted in the index. let (index_text, head_text) = match (index_text, head_text) { - (Some(index_text), Some(head_text)) => (index_text, head_text), - // file is deleted in both index and head - (None, None) => return None, - // file is deleted in index - (None, Some(head_text)) => { - return if stage { - Some(buffer.as_rope().clone()) - } else { - Some(head_text) - } - } - // file exists in the index, but is deleted in head - (Some(_), None) => { - return if stage { - Some(buffer.as_rope().clone()) + (Some(index_text), Some(head_text)) if file_exists || !stage => (index_text, head_text), + (_, head_text @ _) => { + if stage { + log::debug!("stage all"); + return ( + file_exists.then(|| buffer.as_rope().clone()), + vec![( + 0, + PendingHunk { + buffer_version: buffer.version().clone(), + new_status: DiffHunkSecondaryStatus::SecondaryHunkRemovalPending, + }, + )], + ); } else { - None + log::debug!("unstage all"); + return ( + head_text, + vec![( + 0, + PendingHunk { + buffer_version: buffer.version().clone(), + new_status: DiffHunkSecondaryStatus::SecondaryHunkAdditionPending, + }, + )], + ); } } }; - let mut secondary_cursor = secondary_diff.inner.hunks.cursor::(buffer); - secondary_cursor.next(buffer); + let mut unstaged_hunk_cursor = unstaged_diff.hunks.cursor::(buffer); + unstaged_hunk_cursor.next(buffer); let mut edits = Vec::new(); - let mut prev_secondary_hunk_buffer_offset = 0; - let mut prev_secondary_hunk_base_text_offset = 0; - for (buffer_range, diff_base_byte_range) in hunks { - let skipped_hunks = secondary_cursor.slice(&buffer_range.start, Bias::Left, buffer); + let mut pending_hunks = Vec::new(); + let mut prev_unstaged_hunk_buffer_offset = 0; + let mut prev_unstaged_hunk_base_text_offset = 0; + for DiffHunk { + buffer_range, + diff_base_byte_range, + secondary_status, + .. + } in hunks.iter().cloned() + { + if (stage && secondary_status == DiffHunkSecondaryStatus::None) + || (!stage && secondary_status == DiffHunkSecondaryStatus::HasSecondaryHunk) + { + continue; + } + + let skipped_hunks = unstaged_hunk_cursor.slice(&buffer_range.start, Bias::Left, buffer); if let Some(secondary_hunk) = skipped_hunks.last() { - prev_secondary_hunk_base_text_offset = secondary_hunk.diff_base_byte_range.end; - prev_secondary_hunk_buffer_offset = + prev_unstaged_hunk_base_text_offset = secondary_hunk.diff_base_byte_range.end; + prev_unstaged_hunk_buffer_offset = secondary_hunk.buffer_range.end.to_offset(buffer); } let mut buffer_offset_range = buffer_range.to_offset(buffer); - let start_overshoot = buffer_offset_range.start - prev_secondary_hunk_buffer_offset; - let mut secondary_base_text_start = - prev_secondary_hunk_base_text_offset + start_overshoot; + let start_overshoot = buffer_offset_range.start - prev_unstaged_hunk_buffer_offset; + let mut index_start = prev_unstaged_hunk_base_text_offset + start_overshoot; - while let Some(secondary_hunk) = secondary_cursor.item().filter(|item| { + while let Some(unstaged_hunk) = unstaged_hunk_cursor.item().filter(|item| { item.buffer_range .start .cmp(&buffer_range.end, buffer) .is_le() }) { - let secondary_hunk_offset_range = secondary_hunk.buffer_range.to_offset(buffer); - prev_secondary_hunk_base_text_offset = secondary_hunk.diff_base_byte_range.end; - prev_secondary_hunk_buffer_offset = secondary_hunk_offset_range.end; + let unstaged_hunk_offset_range = unstaged_hunk.buffer_range.to_offset(buffer); + prev_unstaged_hunk_base_text_offset = unstaged_hunk.diff_base_byte_range.end; + prev_unstaged_hunk_buffer_offset = unstaged_hunk_offset_range.end; - secondary_base_text_start = - secondary_base_text_start.min(secondary_hunk.diff_base_byte_range.start); + index_start = index_start.min(unstaged_hunk.diff_base_byte_range.start); buffer_offset_range.start = buffer_offset_range .start - .min(secondary_hunk_offset_range.start); + .min(unstaged_hunk_offset_range.start); - secondary_cursor.next(buffer); + unstaged_hunk_cursor.next(buffer); } let end_overshoot = buffer_offset_range .end - .saturating_sub(prev_secondary_hunk_buffer_offset); - let secondary_base_text_end = prev_secondary_hunk_base_text_offset + end_overshoot; + .saturating_sub(prev_unstaged_hunk_buffer_offset); + let index_end = prev_unstaged_hunk_base_text_offset + end_overshoot; - let secondary_base_text_range = secondary_base_text_start..secondary_base_text_end; + let index_range = index_start..index_end; buffer_offset_range.end = buffer_offset_range .end - .max(prev_secondary_hunk_buffer_offset); + .max(prev_unstaged_hunk_buffer_offset); let replacement_text = if stage { - log::debug!("staging"); + log::debug!("stage hunk {:?}", buffer_offset_range); buffer .text_for_range(buffer_offset_range) .collect::() } else { - log::debug!("unstaging"); + log::debug!("unstage hunk {:?}", buffer_offset_range); head_text .chunks_in_range(diff_base_byte_range.clone()) .collect::() }; - edits.push((secondary_base_text_range, replacement_text)); + pending_hunks.push(( + diff_base_byte_range.start, + PendingHunk { + buffer_version: buffer.version().clone(), + new_status: if stage { + DiffHunkSecondaryStatus::SecondaryHunkRemovalPending + } else { + DiffHunkSecondaryStatus::SecondaryHunkAdditionPending + }, + }, + )); + edits.push((index_range, replacement_text)); } - let buffer = cx.new(|cx| { - language::Buffer::local_normalized(index_text, text::LineEnding::default(), cx) - }); - let new_text = buffer.update(cx, |buffer, cx| { - buffer.edit(edits, None, cx); - buffer.as_rope().clone() - }); - Some(new_text) + let mut new_index_text = Rope::new(); + let mut index_cursor = index_text.cursor(0); + for (old_range, replacement_text) in edits { + new_index_text.append(index_cursor.slice(old_range.start)); + index_cursor.seek_forward(old_range.end); + new_index_text.push(&replacement_text); + } + new_index_text.append(index_cursor.suffix()); + (Some(new_index_text), pending_hunks) } -} -impl BufferDiffInner { fn hunks_intersecting_range<'a>( &'a self, range: Range, @@ -318,11 +353,14 @@ impl BufferDiffInner { ] }); - let mut secondary_cursor = secondary.as_ref().map(|diff| { - let mut cursor = diff.hunks.cursor::(buffer); + let mut secondary_cursor = None; + let mut pending_hunks = TreeMap::default(); + if let Some(secondary) = secondary.as_ref() { + let mut cursor = secondary.hunks.cursor::(buffer); cursor.next(buffer); - cursor - }); + secondary_cursor = Some(cursor); + pending_hunks = secondary.pending_hunks.clone(); + } let mut summaries = buffer.summaries_for_anchors_with_payload::(anchor_iter); iter::from_fn(move || loop { @@ -340,7 +378,19 @@ impl BufferDiffInner { } let mut secondary_status = DiffHunkSecondaryStatus::None; - if let Some(secondary_cursor) = secondary_cursor.as_mut() { + + let mut has_pending = false; + if let Some(pending_hunk) = pending_hunks.get(&start_base) { + if !buffer.has_edits_since_in_range( + &pending_hunk.buffer_version, + start_anchor..end_anchor, + ) { + has_pending = true; + secondary_status = pending_hunk.new_status; + } + } + + if let (Some(secondary_cursor), false) = (secondary_cursor.as_mut(), has_pending) { if start_anchor .cmp(&secondary_cursor.start().buffer_range.start, buffer) .is_gt() @@ -354,14 +404,15 @@ impl BufferDiffInner { secondary_range.end.row += 1; secondary_range.end.column = 0; } - if secondary_range == (start_point..end_point) { + if secondary_range.is_empty() && secondary_hunk.diff_base_byte_range.is_empty() + { + // ignore + } else if secondary_range == (start_point..end_point) { secondary_status = DiffHunkSecondaryStatus::HasSecondaryHunk; } else if secondary_range.start <= end_point { secondary_status = DiffHunkSecondaryStatus::OverlapsWithSecondaryHunk; } } - } else { - log::debug!("no secondary cursor!!"); } return Some(DiffHunk { @@ -518,6 +569,14 @@ fn compute_hunks( tree.push(hunk, &buffer); } } + } else { + tree.push( + InternalDiffHunk { + buffer_range: Anchor::MIN..Anchor::MAX, + diff_base_byte_range: 0..0, + }, + &buffer, + ); } tree @@ -631,95 +690,71 @@ impl BufferDiff { fn build( buffer: text::BufferSnapshot, - diff_base: Option>, + base_text: Option>, language: Option>, language_registry: Option>, cx: &mut App, ) -> impl Future { - let diff_base = - diff_base.map(|diff_base| (diff_base.clone(), Rope::from(diff_base.as_str()))); - let base_text_snapshot = diff_base.as_ref().map(|(_, diff_base)| { - language::Buffer::build_snapshot( - diff_base.clone(), + let base_text_pair; + let base_text_exists; + let base_text_snapshot; + if let Some(text) = &base_text { + let base_text_rope = Rope::from(text.as_str()); + base_text_pair = Some((text.clone(), base_text_rope.clone())); + let snapshot = language::Buffer::build_snapshot( + base_text_rope, language.clone(), language_registry.clone(), cx, - ) - }); - let base_text_snapshot = cx.background_spawn(OptionFuture::from(base_text_snapshot)); + ); + base_text_snapshot = cx.background_spawn(snapshot); + base_text_exists = true; + } else { + base_text_pair = None; + base_text_snapshot = Task::ready(language::Buffer::build_empty_snapshot(cx)); + base_text_exists = false; + }; let hunks = cx.background_spawn({ let buffer = buffer.clone(); - async move { compute_hunks(diff_base, buffer) } + async move { compute_hunks(base_text_pair, buffer) } }); async move { let (base_text, hunks) = futures::join!(base_text_snapshot, hunks); - BufferDiffInner { base_text, hunks } + BufferDiffInner { + base_text, + hunks, + base_text_exists, + pending_hunks: TreeMap::default(), + } } } fn build_with_base_buffer( buffer: text::BufferSnapshot, - diff_base: Option>, - diff_base_buffer: Option, + base_text: Option>, + base_text_snapshot: language::BufferSnapshot, cx: &App, ) -> impl Future { - let diff_base = diff_base.clone().zip( - diff_base_buffer - .clone() - .map(|buffer| buffer.as_rope().clone()), - ); + let base_text_exists = base_text.is_some(); + let base_text_pair = base_text.map(|text| (text, base_text_snapshot.as_rope().clone())); cx.background_spawn(async move { BufferDiffInner { - hunks: compute_hunks(diff_base, buffer), - base_text: diff_base_buffer, + base_text: base_text_snapshot, + hunks: compute_hunks(base_text_pair, buffer), + pending_hunks: TreeMap::default(), + base_text_exists, } }) } - fn build_empty(buffer: &text::BufferSnapshot) -> BufferDiffInner { + fn build_empty(buffer: &text::BufferSnapshot, cx: &mut App) -> BufferDiffInner { BufferDiffInner { + base_text: language::Buffer::build_empty_snapshot(cx), hunks: SumTree::new(buffer), - base_text: None, - } - } - - pub fn build_with_single_insertion( - insertion_present_in_secondary_diff: bool, - buffer: language::BufferSnapshot, - cx: &mut App, - ) -> BufferDiffSnapshot { - let base_text = language::Buffer::build_empty_snapshot(cx); - let hunks = SumTree::from_item( - InternalDiffHunk { - buffer_range: Anchor::MIN..Anchor::MAX, - diff_base_byte_range: 0..0, - }, - &base_text, - ); - BufferDiffSnapshot { - inner: BufferDiffInner { - hunks: hunks.clone(), - base_text: Some(base_text.clone()), - }, - secondary_diff: Some(Box::new(BufferDiffSnapshot { - inner: BufferDiffInner { - hunks: if insertion_present_in_secondary_diff { - hunks - } else { - SumTree::new(&buffer.text) - }, - base_text: Some(if insertion_present_in_secondary_diff { - base_text - } else { - buffer - }), - }, - secondary_diff: None, - is_single_insertion: true, - })), - is_single_insertion: true, + pending_hunks: TreeMap::default(), + base_text_exists: false, } } @@ -728,7 +763,38 @@ impl BufferDiff { } pub fn secondary_diff(&self) -> Option> { - Some(self.secondary_diff.as_ref()?.clone()) + self.secondary_diff.clone() + } + + pub fn stage_or_unstage_hunks( + &mut self, + stage: bool, + hunks: &[DiffHunk], + buffer: &text::BufferSnapshot, + file_exists: bool, + cx: &mut Context, + ) -> Option { + let (new_index_text, pending_hunks) = self.inner.stage_or_unstage_hunks( + &self.secondary_diff.as_ref()?.read(cx).inner, + stage, + &hunks, + buffer, + file_exists, + ); + if let Some(unstaged_diff) = &self.secondary_diff { + unstaged_diff.update(cx, |diff, _| { + for (offset, pending_hunk) in pending_hunks { + diff.inner.pending_hunks.insert(offset, pending_hunk); + } + }); + } + if let Some((first, last)) = hunks.first().zip(hunks.last()) { + let changed_range = first.buffer_range.start..last.buffer_range.end; + cx.emit(BufferDiffEvent::DiffChanged { + changed_range: Some(changed_range), + }); + } + new_index_text } pub fn range_to_hunk_range( @@ -777,7 +843,7 @@ impl BufferDiff { Self::build_with_base_buffer( buffer.clone(), base_text, - this.base_text().cloned(), + this.base_text().clone(), cx, ) })? @@ -799,22 +865,33 @@ impl BufferDiff { fn set_state( &mut self, - inner: BufferDiffInner, + new_state: BufferDiffInner, buffer: &text::BufferSnapshot, ) -> Option> { - let changed_range = match (self.inner.base_text.as_ref(), inner.base_text.as_ref()) { - (None, None) => None, - (Some(old), Some(new)) if old.remote_id() == new.remote_id() => { - inner.compare(&self.inner, buffer) - } - _ => Some(text::Anchor::MIN..text::Anchor::MAX), - }; - self.inner = inner; + let (base_text_changed, changed_range) = + match (self.inner.base_text_exists, new_state.base_text_exists) { + (false, false) => (true, None), + (true, true) + if self.inner.base_text.remote_id() == new_state.base_text.remote_id() => + { + (false, new_state.compare(&self.inner, buffer)) + } + _ => (true, Some(text::Anchor::MIN..text::Anchor::MAX)), + }; + let pending_hunks = mem::take(&mut self.inner.pending_hunks); + self.inner = new_state; + if !base_text_changed { + self.inner.pending_hunks = pending_hunks; + } changed_range } - pub fn base_text(&self) -> Option<&language::BufferSnapshot> { - self.inner.base_text.as_ref() + pub fn base_text(&self) -> &language::BufferSnapshot { + &self.inner.base_text + } + + pub fn base_text_exists(&self) -> bool { + self.inner.base_text_exists } pub fn snapshot(&self, cx: &App) -> BufferDiffSnapshot { @@ -824,7 +901,6 @@ impl BufferDiff { .secondary_diff .as_ref() .map(|diff| Box::new(diff.read(cx).snapshot(cx))), - is_single_insertion: false, } } @@ -901,15 +977,16 @@ impl BufferDiff { rx } - #[cfg(any(test, feature = "test-support"))] pub fn base_text_string(&self) -> Option { - self.inner.base_text.as_ref().map(|buffer| buffer.text()) + self.inner + .base_text_exists + .then(|| self.inner.base_text.text()) } - pub fn new(buffer: &text::BufferSnapshot) -> Self { + pub fn new(buffer: &text::BufferSnapshot, cx: &mut App) -> Self { BufferDiff { buffer_id: buffer.remote_id(), - inner: BufferDiff::build_empty(buffer), + inner: BufferDiff::build_empty(buffer, cx), secondary_diff: None, } } @@ -939,14 +1016,10 @@ impl BufferDiff { #[cfg(any(test, feature = "test-support"))] pub fn recalculate_diff_sync(&mut self, buffer: text::BufferSnapshot, cx: &mut Context) { - let base_text = self - .inner - .base_text - .as_ref() - .map(|base_text| base_text.text()); + let base_text = self.base_text_string().map(Arc::new); let snapshot = BufferDiff::build_with_base_buffer( buffer.clone(), - base_text.clone().map(Arc::new), + base_text, self.inner.base_text.clone(), cx, ); @@ -957,6 +1030,10 @@ impl BufferDiff { } impl DiffHunk { + pub fn is_created_file(&self) -> bool { + self.diff_base_byte_range == (0..0) && self.buffer_range == (Anchor::MIN..Anchor::MAX) + } + pub fn status(&self) -> DiffHunkStatus { let kind = if self.buffer_range.start == self.buffer_range.end { DiffHunkStatusKind::Deleted @@ -973,6 +1050,23 @@ impl DiffHunk { } impl DiffHunkStatus { + pub fn has_secondary_hunk(&self) -> bool { + matches!( + self.secondary, + DiffHunkSecondaryStatus::HasSecondaryHunk + | DiffHunkSecondaryStatus::SecondaryHunkAdditionPending + | DiffHunkSecondaryStatus::OverlapsWithSecondaryHunk + ) + } + + pub fn is_pending(&self) -> bool { + matches!( + self.secondary, + DiffHunkSecondaryStatus::SecondaryHunkAdditionPending + | DiffHunkSecondaryStatus::SecondaryHunkRemovalPending + ) + } + pub fn is_deleted(&self) -> bool { self.kind == DiffHunkStatusKind::Deleted } @@ -1006,7 +1100,6 @@ impl DiffHunkStatus { } } - #[cfg(any(test, feature = "test-support"))] pub fn deleted_none() -> Self { Self { kind: DiffHunkStatusKind::Deleted, @@ -1014,7 +1107,6 @@ impl DiffHunkStatus { } } - #[cfg(any(test, feature = "test-support"))] pub fn added_none() -> Self { Self { kind: DiffHunkStatusKind::Added, @@ -1022,7 +1114,6 @@ impl DiffHunkStatus { } } - #[cfg(any(test, feature = "test-support"))] pub fn modified_none() -> Self { Self { kind: DiffHunkStatusKind::Modified, @@ -1120,7 +1211,7 @@ mod tests { ], ); - diff = BufferDiff::build_empty(&buffer); + diff = cx.update(|cx| BufferDiff::build_empty(&buffer, cx)); assert_hunks( diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &buffer, None), &buffer, @@ -1435,43 +1526,55 @@ mod tests { for example in table { let (buffer_text, ranges) = marked_text_ranges(&example.buffer_marked_text, false); let buffer = Buffer::new(0, BufferId::new(1).unwrap(), buffer_text); - let uncommitted_diff = - BufferDiff::build_sync(buffer.clone(), example.head_text.clone(), cx); - let unstaged_diff = - BufferDiff::build_sync(buffer.clone(), example.index_text.clone(), cx); - let uncommitted_diff = BufferDiffSnapshot { - inner: uncommitted_diff, - secondary_diff: Some(Box::new(BufferDiffSnapshot { - inner: unstaged_diff, - is_single_insertion: false, - secondary_diff: None, - })), - is_single_insertion: false, - }; + let hunk_range = + buffer.anchor_before(ranges[0].start)..buffer.anchor_before(ranges[0].end); - let range = buffer.anchor_before(ranges[0].start)..buffer.anchor_before(ranges[0].end); - - let new_index_text = cx - .update(|cx| { - uncommitted_diff.new_secondary_text_for_stage_or_unstage( - true, - uncommitted_diff - .hunks_intersecting_range(range, &buffer) - .map(|hunk| { - (hunk.buffer_range.clone(), hunk.diff_base_byte_range.clone()) - }), - &buffer, - cx, + let unstaged = BufferDiff::build_sync(buffer.clone(), example.index_text.clone(), cx); + let uncommitted = BufferDiff::build_sync(buffer.clone(), example.head_text.clone(), cx); + + let unstaged_diff = cx.new(|cx| { + let mut diff = BufferDiff::new(&buffer, cx); + diff.set_state(unstaged, &buffer); + diff + }); + + let uncommitted_diff = cx.new(|cx| { + let mut diff = BufferDiff::new(&buffer, cx); + diff.set_state(uncommitted, &buffer); + diff.set_secondary_diff(unstaged_diff); + diff + }); + + uncommitted_diff.update(cx, |diff, cx| { + let hunks = diff + .hunks_intersecting_range(hunk_range.clone(), &buffer, &cx) + .collect::>(); + for hunk in &hunks { + assert_ne!(hunk.secondary_status, DiffHunkSecondaryStatus::None) + } + + let new_index_text = diff + .stage_or_unstage_hunks(true, &hunks, &buffer, true, cx) + .unwrap() + .to_string(); + + let hunks = diff + .hunks_intersecting_range(hunk_range.clone(), &buffer, &cx) + .collect::>(); + for hunk in &hunks { + assert_eq!( + hunk.secondary_status, + DiffHunkSecondaryStatus::SecondaryHunkRemovalPending ) - }) - .unwrap() - .to_string(); - pretty_assertions::assert_eq!( - new_index_text, - example.final_index_text, - "example: {}", - example.name - ); + } + + pretty_assertions::assert_eq!( + new_index_text, + example.final_index_text, + "example: {}", + example.name + ); + }); } } @@ -1505,7 +1608,7 @@ mod tests { let mut buffer = Buffer::new(0, BufferId::new(1).unwrap(), buffer_text_1); - let empty_diff = BufferDiff::build_empty(&buffer); + let empty_diff = cx.update(|cx| BufferDiff::build_empty(&buffer, cx)); let diff_1 = BufferDiff::build_sync(buffer.clone(), base_text.clone(), cx); let range = diff_1.compare(&empty_diff, &buffer).unwrap(); assert_eq!(range.to_point(&buffer), Point::new(0, 0)..Point::new(8, 0)); @@ -1668,7 +1771,7 @@ mod tests { index_text: &Rope, head_text: String, cx: &mut TestAppContext, - ) -> BufferDiff { + ) -> Entity { let inner = BufferDiff::build_sync(working_copy.text.clone(), head_text, cx); let secondary = BufferDiff { buffer_id: working_copy.remote_id(), @@ -1680,11 +1783,11 @@ mod tests { secondary_diff: None, }; let secondary = cx.new(|_| secondary); - BufferDiff { + cx.new(|_| BufferDiff { buffer_id: working_copy.remote_id(), inner, secondary_diff: Some(secondary), - } + }) } let operations = std::env::var("OPERATIONS") @@ -1712,7 +1815,7 @@ mod tests { }; let mut diff = uncommitted_diff(&working_copy, &index_text, head_text.clone(), cx); - let mut hunks = cx.update(|cx| { + let mut hunks = diff.update(cx, |diff, cx| { diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &working_copy, cx) .collect::>() }); @@ -1723,6 +1826,7 @@ mod tests { for _ in 0..operations { let i = rng.gen_range(0..hunks.len()); let hunk = &mut hunks[i]; + let hunk_to_change = hunk.clone(); let stage = match hunk.secondary_status { DiffHunkSecondaryStatus::HasSecondaryHunk => { hunk.secondary_status = DiffHunkSecondaryStatus::None; @@ -1735,21 +1839,13 @@ mod tests { _ => unreachable!(), }; - let snapshot = cx.update(|cx| diff.snapshot(cx)); - index_text = cx.update(|cx| { - snapshot - .new_secondary_text_for_stage_or_unstage( - stage, - [(hunk.buffer_range.clone(), hunk.diff_base_byte_range.clone())] - .into_iter(), - &working_copy, - cx, - ) + index_text = diff.update(cx, |diff, cx| { + diff.stage_or_unstage_hunks(stage, &[hunk_to_change], &working_copy, true, cx) .unwrap() }); diff = uncommitted_diff(&working_copy, &index_text, head_text.clone(), cx); - let found_hunks = cx.update(|cx| { + let found_hunks = diff.update(cx, |diff, cx| { diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &working_copy, cx) .collect::>() }); diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b2cecf87c6bd51b7d2fad09388ddda72fcbabbf3..47eb8495e9b157bf5f54a1c9c323de2a6045560f 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -52,7 +52,7 @@ pub use actions::{AcceptEditPrediction, OpenExcerpts, OpenExcerptsSplit}; use aho_corasick::AhoCorasick; use anyhow::{anyhow, Context as _, Result}; use blink_manager::BlinkManager; -use buffer_diff::{DiffHunkSecondaryStatus, DiffHunkStatus}; +use buffer_diff::DiffHunkStatus; use client::{Collaborator, ParticipantIndex}; use clock::ReplicaId; use collections::{BTreeMap, HashMap, HashSet, VecDeque}; @@ -7720,11 +7720,6 @@ impl Editor { cx: &mut Context, ) { let mut revert_changes = HashMap::default(); - let snapshot = self.buffer.read(cx).snapshot(cx); - let Some(project) = &self.project else { - return; - }; - let chunk_by = self .snapshot(window, cx) .hunks_for_ranges(ranges.into_iter()) @@ -7735,15 +7730,7 @@ impl Editor { for hunk in &hunks { self.prepare_restore_change(&mut revert_changes, hunk, cx); } - Self::do_stage_or_unstage( - project, - false, - buffer_id, - hunks.into_iter(), - &snapshot, - window, - cx, - ); + self.do_stage_or_unstage(false, buffer_id, hunks.into_iter(), window, cx); } drop(chunk_by); if !revert_changes.is_empty() { @@ -7788,7 +7775,6 @@ impl Editor { let original_text = diff .read(cx) .base_text() - .as_ref()? .as_rope() .slice(hunk.diff_base_byte_range.clone()); let buffer_snapshot = buffer.snapshot(); @@ -13524,7 +13510,7 @@ impl Editor { snapshot: &MultiBufferSnapshot, ) -> bool { let mut hunks = self.diff_hunks_in_ranges(ranges, &snapshot); - hunks.any(|hunk| hunk.secondary_status != DiffHunkSecondaryStatus::None) + hunks.any(|hunk| hunk.status().has_secondary_hunk()) } pub fn toggle_staged_selected_diff_hunks( @@ -13565,15 +13551,11 @@ impl Editor { cx: &mut Context, ) { let snapshot = self.buffer.read(cx).snapshot(cx); - let Some(project) = &self.project else { - return; - }; - let chunk_by = self .diff_hunks_in_ranges(&ranges, &snapshot) .chunk_by(|hunk| hunk.buffer_id); for (buffer_id, hunks) in &chunk_by { - Self::do_stage_or_unstage(project, stage, buffer_id, hunks, &snapshot, window, cx); + self.do_stage_or_unstage(stage, buffer_id, hunks, window, cx); } } @@ -13647,16 +13629,20 @@ impl Editor { } fn do_stage_or_unstage( - project: &Entity, + &self, stage: bool, buffer_id: BufferId, hunks: impl Iterator, - snapshot: &MultiBufferSnapshot, window: &mut Window, cx: &mut App, ) { + let Some(project) = self.project.as_ref() else { + return; + }; let Some(buffer) = project.read(cx).buffer_for_id(buffer_id, cx) else { - log::debug!("no buffer for id"); + return; + }; + let Some(diff) = self.buffer.read(cx).diff_for(buffer_id) else { return; }; let buffer_snapshot = buffer.read(cx).snapshot(); @@ -13670,37 +13656,31 @@ impl Editor { log::debug!("no git repo for buffer id"); return; }; - let Some(diff) = snapshot.diff_for_buffer_id(buffer_id) else { - log::debug!("no diff for buffer id"); - return; - }; - let new_index_text = if !stage && diff.is_single_insertion || stage && !file_exists { - log::debug!("removing from index"); - None - } else { - diff.new_secondary_text_for_stage_or_unstage( + let new_index_text = diff.update(cx, |diff, cx| { + diff.stage_or_unstage_hunks( stage, - hunks.filter_map(|hunk| { - if stage && hunk.secondary_status == DiffHunkSecondaryStatus::None { - return None; - } else if !stage - && hunk.secondary_status == DiffHunkSecondaryStatus::HasSecondaryHunk - { - return None; - } - Some((hunk.buffer_range.clone(), hunk.diff_base_byte_range.clone())) - }), + &hunks + .map(|hunk| buffer_diff::DiffHunk { + buffer_range: hunk.buffer_range, + diff_base_byte_range: hunk.diff_base_byte_range, + secondary_status: hunk.secondary_status, + row_range: 0..0, // unused + }) + .collect::>(), &buffer_snapshot, + file_exists, cx, ) - }; + }); + if file_exists { let buffer_store = project.read(cx).buffer_store().clone(); buffer_store .update(cx, |buffer_store, cx| buffer_store.save_buffer(buffer, cx)) .detach_and_log_err(cx); } + let recv = repo .read(cx) .set_index_text(&path, new_index_text.map(|rope| rope.to_string())); diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0d3583107dddd3bfc3fc50b1cd7912db7fd5038b..cda4b9ef30a4814e32380e58844723a4e9dfb8b6 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -7,7 +7,7 @@ use crate::{ }, JoinLines, }; -use buffer_diff::{BufferDiff, DiffHunkStatus, DiffHunkStatusKind}; +use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind}; use futures::StreamExt; use gpui::{ div, BackgroundExecutor, SemanticVersion, TestAppContext, UpdateGlobal, VisualTestContext, @@ -12555,7 +12555,7 @@ async fn test_addition_reverts(cx: &mut TestAppContext) { struct Row9.2; struct Row9.3; struct Row10;"#}, - vec![DiffHunkStatus::added_none(), DiffHunkStatus::added_none()], + vec![DiffHunkStatusKind::Added, DiffHunkStatusKind::Added], indoc! {r#"struct Row; struct Row1; struct Row1.1; @@ -12593,7 +12593,7 @@ async fn test_addition_reverts(cx: &mut TestAppContext) { struct Row8; struct Row9; struct Row10;"#}, - vec![DiffHunkStatus::added_none(), DiffHunkStatus::added_none()], + vec![DiffHunkStatusKind::Added, DiffHunkStatusKind::Added], indoc! {r#"struct Row; struct Row1; struct Row2; @@ -12640,11 +12640,11 @@ async fn test_addition_reverts(cx: &mut TestAppContext) { «ˇ// something on bottom» struct Row10;"#}, vec![ - DiffHunkStatus::added_none(), - DiffHunkStatus::added_none(), - DiffHunkStatus::added_none(), - DiffHunkStatus::added_none(), - DiffHunkStatus::added_none(), + DiffHunkStatusKind::Added, + DiffHunkStatusKind::Added, + DiffHunkStatusKind::Added, + DiffHunkStatusKind::Added, + DiffHunkStatusKind::Added, ], indoc! {r#"struct Row; ˇstruct Row1; @@ -12692,10 +12692,7 @@ async fn test_modification_reverts(cx: &mut TestAppContext) { struct Row99; struct Row9; struct Row10;"#}, - vec![ - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - ], + vec![DiffHunkStatusKind::Modified, DiffHunkStatusKind::Modified], indoc! {r#"struct Row; struct Row1; struct Row33; @@ -12722,10 +12719,7 @@ async fn test_modification_reverts(cx: &mut TestAppContext) { struct Row99; struct Row9; struct Row10;"#}, - vec![ - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - ], + vec![DiffHunkStatusKind::Modified, DiffHunkStatusKind::Modified], indoc! {r#"struct Row; struct Row1; struct Row33; @@ -12754,12 +12748,12 @@ async fn test_modification_reverts(cx: &mut TestAppContext) { struct Row9; struct Row1011;ˇ"#}, vec![ - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), - DiffHunkStatus::modified_none(), + DiffHunkStatusKind::Modified, + DiffHunkStatusKind::Modified, + DiffHunkStatusKind::Modified, + DiffHunkStatusKind::Modified, + DiffHunkStatusKind::Modified, + DiffHunkStatusKind::Modified, ], indoc! {r#"struct Row; ˇstruct Row1; @@ -12837,10 +12831,7 @@ struct Row10;"#}; ˇ struct Row8; struct Row10;"#}, - vec![ - DiffHunkStatus::deleted_none(), - DiffHunkStatus::deleted_none(), - ], + vec![DiffHunkStatusKind::Deleted, DiffHunkStatusKind::Deleted], indoc! {r#"struct Row; struct Row2; @@ -12863,10 +12854,7 @@ struct Row10;"#}; ˇ» struct Row8; struct Row10;"#}, - vec![ - DiffHunkStatus::deleted_none(), - DiffHunkStatus::deleted_none(), - ], + vec![DiffHunkStatusKind::Deleted, DiffHunkStatusKind::Deleted], indoc! {r#"struct Row; struct Row2; @@ -12891,10 +12879,7 @@ struct Row10;"#}; struct Row8;ˇ struct Row10;"#}, - vec![ - DiffHunkStatus::deleted_none(), - DiffHunkStatus::deleted_none(), - ], + vec![DiffHunkStatusKind::Deleted, DiffHunkStatusKind::Deleted], indoc! {r#"struct Row; struct Row1; ˇstruct Row2; @@ -12919,9 +12904,9 @@ struct Row10;"#}; struct Row8;ˇ» struct Row10;"#}, vec![ - DiffHunkStatus::deleted_none(), - DiffHunkStatus::deleted_none(), - DiffHunkStatus::deleted_none(), + DiffHunkStatusKind::Deleted, + DiffHunkStatusKind::Deleted, + DiffHunkStatusKind::Deleted, ], indoc! {r#"struct Row; struct Row1; @@ -16838,14 +16823,13 @@ pub(crate) fn init_test(cx: &mut TestAppContext, f: fn(&mut AllLanguageSettingsC #[track_caller] fn assert_hunk_revert( not_reverted_text_with_selections: &str, - expected_hunk_statuses_before: Vec, + expected_hunk_statuses_before: Vec, expected_reverted_text_with_selections: &str, base_text: &str, cx: &mut EditorLspTestContext, ) { cx.set_state(not_reverted_text_with_selections); cx.set_head_text(base_text); - cx.clear_index_text(); cx.executor().run_until_parked(); let actual_hunk_statuses_before = cx.update_editor(|editor, window, cx| { @@ -16853,7 +16837,7 @@ fn assert_hunk_revert( let reverted_hunk_statuses = snapshot .buffer_snapshot .diff_hunks_in_range(0..snapshot.buffer_snapshot.len()) - .map(|hunk| hunk.status()) + .map(|hunk| hunk.status().kind) .collect::>(); editor.git_restore(&Default::default(), window, cx); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index f8fced0d4dc4315bfdf36f621870c351ecf5fe05..ccf0205ee00cee8ecc6db22088cdc04359adc04e 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -26,7 +26,7 @@ use crate::{ FILE_HEADER_HEIGHT, GIT_BLAME_MAX_AUTHOR_CHARS_DISPLAYED, MAX_LINE_LEN, MULTI_BUFFER_EXCERPT_HEADER_HEIGHT, }; -use buffer_diff::{DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind}; +use buffer_diff::{DiffHunkStatus, DiffHunkStatusKind}; use client::ParticipantIndex; use collections::{BTreeMap, HashMap, HashSet}; use file_icons::FileIcons; @@ -4365,7 +4365,7 @@ impl EditorElement { hunk_bounds, cx.theme().colors().version_control_modified, Corners::all(px(0.)), - DiffHunkSecondaryStatus::None, + DiffHunkStatus::modified_none(), )) } DisplayDiffHunk::Unfolded { @@ -4377,19 +4377,19 @@ impl EditorElement { hunk_hitbox.bounds, cx.theme().colors().version_control_added, Corners::all(px(0.)), - status.secondary, + *status, ), DiffHunkStatusKind::Modified => ( hunk_hitbox.bounds, cx.theme().colors().version_control_modified, Corners::all(px(0.)), - status.secondary, + *status, ), DiffHunkStatusKind::Deleted if !display_row_range.is_empty() => ( hunk_hitbox.bounds, cx.theme().colors().version_control_deleted, Corners::all(px(0.)), - status.secondary, + *status, ), DiffHunkStatusKind::Deleted => ( Bounds::new( @@ -4401,19 +4401,18 @@ impl EditorElement { ), cx.theme().colors().version_control_deleted, Corners::all(1. * line_height), - status.secondary, + *status, ), }), }; - if let Some((hunk_bounds, background_color, corner_radii, secondary_status)) = + if let Some((hunk_bounds, mut background_color, corner_radii, secondary_status)) = hunk_to_paint { - let background_color = if secondary_status != DiffHunkSecondaryStatus::None { - background_color.opacity(if is_light { 0.2 } else { 0.32 }) - } else { - background_color.opacity(1.0) - }; + if secondary_status.has_secondary_hunk() { + background_color = + background_color.opacity(if is_light { 0.2 } else { 0.32 }); + } window.paint_quad(quad( hunk_bounds, corner_radii, @@ -6728,12 +6727,11 @@ impl Element for EditorElement { continue; } }; - let background_color = - if diff_status.secondary == DiffHunkSecondaryStatus::None { - background_color.opacity(staged_opacity) - } else { - background_color.opacity(unstaged_opacity) - }; + let background_color = if diff_status.has_secondary_hunk() { + background_color.opacity(unstaged_opacity) + } else { + background_color.opacity(staged_opacity) + }; highlighted_rows .entry(start_row + DisplayRow(ix as u32)) @@ -8780,65 +8778,62 @@ fn diff_hunk_controls( .rounded_b_lg() .bg(cx.theme().colors().editor_background) .gap_1() - .when(status.secondary == DiffHunkSecondaryStatus::None, |el| { - el.child( - Button::new("unstage", "Unstage") - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |window, cx| { - Tooltip::for_action_in( - "Unstage Hunk", - &::git::ToggleStaged, - &focus_handle, + .child(if status.has_secondary_hunk() { + Button::new(("stage", row as u64), "Stage") + .alpha(if status.is_pending() { 0.66 } else { 1.0 }) + .tooltip({ + let focus_handle = editor.focus_handle(cx); + move |window, cx| { + Tooltip::for_action_in( + "Stage Hunk", + &::git::ToggleStaged, + &focus_handle, + window, + cx, + ) + } + }) + .on_click({ + let editor = editor.clone(); + move |_event, window, cx| { + editor.update(cx, |editor, cx| { + editor.stage_or_unstage_diff_hunks( + true, + &[hunk_range.start..hunk_range.start], window, cx, - ) - } - }) - .on_click({ - let editor = editor.clone(); - move |_event, window, cx| { - editor.update(cx, |editor, cx| { - editor.stage_or_unstage_diff_hunks( - false, - &[hunk_range.start..hunk_range.start], - window, - cx, - ); - }); - } - }), - ) - }) - .when(status.secondary != DiffHunkSecondaryStatus::None, |el| { - el.child( - Button::new("stage", "Stage") - .tooltip({ - let focus_handle = editor.focus_handle(cx); - move |window, cx| { - Tooltip::for_action_in( - "Stage Hunk", - &::git::ToggleStaged, - &focus_handle, + ); + }); + } + }) + } else { + Button::new(("unstage", row as u64), "Unstage") + .alpha(if status.is_pending() { 0.66 } else { 1.0 }) + .tooltip({ + let focus_handle = editor.focus_handle(cx); + move |window, cx| { + Tooltip::for_action_in( + "Unstage Hunk", + &::git::ToggleStaged, + &focus_handle, + window, + cx, + ) + } + }) + .on_click({ + let editor = editor.clone(); + move |_event, window, cx| { + editor.update(cx, |editor, cx| { + editor.stage_or_unstage_diff_hunks( + false, + &[hunk_range.start..hunk_range.start], window, cx, - ) - } - }) - .on_click({ - let editor = editor.clone(); - move |_event, window, cx| { - editor.update(cx, |editor, cx| { - editor.stage_or_unstage_diff_hunks( - true, - &[hunk_range.start..hunk_range.start], - window, - cx, - ); - }); - } - }), - ) + ); + }); + } + }) }) .child( Button::new("discard", "Restore") diff --git a/crates/editor/src/proposed_changes_editor.rs b/crates/editor/src/proposed_changes_editor.rs index db25ce8d0ea2cabc10e01e6a83147a12ff7ad1bf..b113dcf9df8b26484cfcd276ac8766cf2844eb71 100644 --- a/crates/editor/src/proposed_changes_editor.rs +++ b/crates/editor/src/proposed_changes_editor.rs @@ -185,7 +185,7 @@ impl ProposedChangesEditor { } else { branch_buffer = location.buffer.update(cx, |buffer, cx| buffer.branch(cx)); new_diffs.push(cx.new(|cx| { - let mut diff = BufferDiff::new(branch_buffer.read(cx)); + let mut diff = BufferDiff::new(&branch_buffer.read(cx).snapshot(), cx); let _ = diff.set_base_text( location.buffer.clone(), branch_buffer.read(cx).text_snapshot(), diff --git a/crates/git_ui/src/project_diff.rs b/crates/git_ui/src/project_diff.rs index e4c0e25671a3a3b8519d57a0e4f2335b81ced373..f892ff0dfa00f15d10bd6f006ba42a8f0542defc 100644 --- a/crates/git_ui/src/project_diff.rs +++ b/crates/git_ui/src/project_diff.rs @@ -1,6 +1,4 @@ -use std::any::{Any, TypeId}; - -use ::git::UnstageAndNext; +use crate::git_panel::{GitPanel, GitPanelAddon, GitStatusEntry}; use anyhow::Result; use buffer_diff::{BufferDiff, DiffHunkSecondaryStatus}; use collections::HashSet; @@ -11,14 +9,17 @@ use editor::{ }; use feature_flags::FeatureFlagViewExt; use futures::StreamExt; -use git::{status::FileStatus, Commit, StageAll, StageAndNext, ToggleStaged, UnstageAll}; +use git::{ + status::FileStatus, Commit, StageAll, StageAndNext, ToggleStaged, UnstageAll, UnstageAndNext, +}; use gpui::{ actions, Action, AnyElement, AnyView, App, AppContext as _, AsyncWindowContext, Entity, EventEmitter, FocusHandle, Focusable, Render, Subscription, Task, WeakEntity, }; -use language::{Anchor, Buffer, Capability, OffsetRangeExt, Point}; +use language::{Anchor, Buffer, Capability, OffsetRangeExt}; use multi_buffer::{MultiBuffer, PathKey}; use project::{git::GitStore, Project, ProjectPath}; +use std::any::{Any, TypeId}; use theme::ActiveTheme; use ui::{prelude::*, vertical_divider, Tooltip}; use util::ResultExt as _; @@ -29,8 +30,6 @@ use workspace::{ Workspace, }; -use crate::git_panel::{GitPanel, GitPanelAddon, GitStatusEntry}; - actions!(git, [Diff]); pub struct ProjectDiff { @@ -230,14 +229,16 @@ impl ProjectDiff { let mut has_unstaged_hunks = false; for hunk in editor.diff_hunks_in_ranges(&ranges, &snapshot) { match hunk.secondary_status { - DiffHunkSecondaryStatus::HasSecondaryHunk => { + DiffHunkSecondaryStatus::HasSecondaryHunk + | DiffHunkSecondaryStatus::SecondaryHunkAdditionPending => { has_unstaged_hunks = true; } DiffHunkSecondaryStatus::OverlapsWithSecondaryHunk => { has_staged_hunks = true; has_unstaged_hunks = true; } - DiffHunkSecondaryStatus::None => { + DiffHunkSecondaryStatus::None + | DiffHunkSecondaryStatus::SecondaryHunkRemovalPending => { has_staged_hunks = true; } } @@ -378,13 +379,10 @@ impl ProjectDiff { let snapshot = buffer.read(cx).snapshot(); let diff = diff.read(cx); - let diff_hunk_ranges = if diff.base_text().is_none() { - vec![Point::zero()..snapshot.max_point()] - } else { - diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx) - .map(|diff_hunk| diff_hunk.buffer_range.to_point(&snapshot)) - .collect::>() - }; + let diff_hunk_ranges = diff + .hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx) + .map(|diff_hunk| diff_hunk.buffer_range.to_point(&snapshot)) + .collect::>(); let (was_empty, is_excerpt_newly_added) = self.multibuffer.update(cx, |multibuffer, cx| { let was_empty = multibuffer.is_empty(); @@ -971,7 +969,7 @@ mod tests { path!("/project"), json!({ ".git": {}, - "foo": "FOO\n", + "foo.txt": "FOO\n", }), ) .await; @@ -985,11 +983,15 @@ mod tests { fs.set_head_for_repo( path!("/project/.git").as_ref(), - &[("foo".into(), "foo\n".into())], + &[("foo.txt".into(), "foo\n".into())], + ); + fs.set_index_for_repo( + path!("/project/.git").as_ref(), + &[("foo.txt".into(), "foo\n".into())], ); fs.with_git_state(path!("/project/.git").as_ref(), true, |state| { state.statuses = HashMap::from_iter([( - "foo".into(), + "foo.txt".into(), TrackedStatus { index_status: StatusCode::Unmodified, worktree_status: StatusCode::Modified, @@ -1020,7 +1022,7 @@ mod tests { assert_state_with_diff(&editor, cx, &"ˇ".unindent()); - let text = String::from_utf8(fs.read_file_sync("/project/foo").unwrap()).unwrap(); + let text = String::from_utf8(fs.read_file_sync("/project/foo.txt").unwrap()).unwrap(); assert_eq!(text, "foo\n"); } diff --git a/crates/multi_buffer/src/anchor.rs b/crates/multi_buffer/src/anchor.rs index 4d77478a4e3eeb2c76098da716ea547dc04f286d..3cb8b0302edc9adef6378ad55f62294c25ed286f 100644 --- a/crates/multi_buffer/src/anchor.rs +++ b/crates/multi_buffer/src/anchor.rs @@ -73,7 +73,7 @@ impl Anchor { if let Some(base_text) = snapshot .diffs .get(&excerpt.buffer_id) - .and_then(|diff| diff.base_text()) + .map(|diff| diff.base_text()) { let self_anchor = self.diff_base_anchor.filter(|a| base_text.can_resolve(a)); let other_anchor = other.diff_base_anchor.filter(|a| base_text.can_resolve(a)); @@ -110,7 +110,7 @@ impl Anchor { if let Some(base_text) = snapshot .diffs .get(&excerpt.buffer_id) - .and_then(|diff| diff.base_text()) + .map(|diff| diff.base_text()) { if a.buffer_id == Some(base_text.remote_id()) { return a.bias_left(base_text); @@ -135,7 +135,7 @@ impl Anchor { if let Some(base_text) = snapshot .diffs .get(&excerpt.buffer_id) - .and_then(|diff| diff.base_text()) + .map(|diff| diff.base_text()) { if a.buffer_id == Some(base_text.remote_id()) { return a.bias_right(&base_text); diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index 9262b5597de3dd2a9d6fe1512d1c826108906692..21d657775f8fe8a72b33e7a06ada672ba2d7566a 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -69,7 +69,7 @@ pub struct MultiBuffer { // only used by consumers using `set_excerpts_for_buffer` buffers_by_path: BTreeMap>, diffs: HashMap, - all_diff_hunks_expanded: bool, + // all_diff_hunks_expanded: bool, subscriptions: Topic, /// If true, the multi-buffer only contains a single [`Buffer`] and a single [`Excerpt`] singleton: bool, @@ -245,14 +245,9 @@ impl DiffState { DiffState { _subscription: cx.subscribe(&diff, |this, diff, event, cx| match event { BufferDiffEvent::DiffChanged { changed_range } => { - let changed_range = if let Some(changed_range) = changed_range { - changed_range.clone() - } else if diff.read(cx).base_text().is_none() && this.all_diff_hunks_expanded { - text::Anchor::MIN..text::Anchor::MAX - } else { - return; - }; - this.buffer_diff_changed(diff, changed_range, cx) + if let Some(changed_range) = changed_range.clone() { + this.buffer_diff_changed(diff, changed_range, cx) + } } BufferDiffEvent::LanguageChanged => this.buffer_diff_language_changed(diff, cx), }), @@ -270,6 +265,7 @@ pub struct MultiBufferSnapshot { diffs: TreeMap, diff_transforms: SumTree, trailing_excerpt_update_count: usize, + all_diff_hunks_expanded: bool, non_text_state_update_count: usize, edit_count: usize, is_dirty: bool, @@ -559,7 +555,6 @@ impl MultiBuffer { }), buffers: RefCell::default(), diffs: HashMap::default(), - all_diff_hunks_expanded: false, subscriptions: Topic::default(), singleton: false, capability, @@ -581,7 +576,6 @@ impl MultiBuffer { buffers: Default::default(), buffers_by_path: Default::default(), diffs: HashMap::default(), - all_diff_hunks_expanded: false, subscriptions: Default::default(), singleton: false, capability, @@ -622,7 +616,6 @@ impl MultiBuffer { buffers: RefCell::new(buffers), buffers_by_path: Default::default(), diffs: diff_bases, - all_diff_hunks_expanded: self.all_diff_hunks_expanded, subscriptions: Default::default(), singleton: self.singleton, capability: self.capability, @@ -2231,18 +2224,7 @@ impl MultiBuffer { let buffer = buffer_state.buffer.read(cx); let diff_change_range = range.to_offset(buffer); - let mut new_diff = diff.snapshot(cx); - if new_diff.base_text().is_none() && self.all_diff_hunks_expanded { - let secondary_diff_insertion = new_diff - .secondary_diff() - .map_or(true, |secondary_diff| secondary_diff.base_text().is_none()); - new_diff = BufferDiff::build_with_single_insertion( - secondary_diff_insertion, - buffer.snapshot(), - cx, - ); - } - + let new_diff = diff.snapshot(cx); let mut snapshot = self.snapshot.borrow_mut(); let base_text_changed = snapshot .diffs @@ -2398,12 +2380,12 @@ impl MultiBuffer { } pub fn set_all_diff_hunks_expanded(&mut self, cx: &mut Context) { - self.all_diff_hunks_expanded = true; + self.snapshot.borrow_mut().all_diff_hunks_expanded = true; self.expand_or_collapse_diff_hunks(vec![Anchor::min()..Anchor::max()], true, cx); } pub fn all_diff_hunks_expanded(&self) -> bool { - self.all_diff_hunks_expanded + self.snapshot.borrow().all_diff_hunks_expanded } pub fn has_multiple_hunks(&self, cx: &App) -> bool { @@ -2459,7 +2441,7 @@ impl MultiBuffer { expand: bool, cx: &mut Context, ) { - if self.all_diff_hunks_expanded && !expand { + if self.snapshot.borrow().all_diff_hunks_expanded && !expand { return; } self.sync(cx); @@ -2964,9 +2946,10 @@ impl MultiBuffer { } // Avoid querying diff hunks if there's no possibility of hunks being expanded. + let all_diff_hunks_expanded = snapshot.all_diff_hunks_expanded; if old_expanded_hunks.is_empty() && change_kind == DiffChangeKind::BufferEdited - && !self.all_diff_hunks_expanded + && !all_diff_hunks_expanded { return false; } @@ -2976,11 +2959,7 @@ impl MultiBuffer { while let Some(excerpt) = excerpts.item() { // Recompute the expanded hunks in the portion of the excerpt that // intersects the edit. - if let Some((diff, base_text)) = snapshot - .diffs - .get(&excerpt.buffer_id) - .and_then(|diff| Some((diff, diff.base_text()?))) - { + if let Some(diff) = snapshot.diffs.get(&excerpt.buffer_id) { let buffer = &excerpt.buffer; let excerpt_start = *excerpts.start(); let excerpt_end = excerpt_start + ExcerptOffset::new(excerpt.text_summary.len); @@ -2995,17 +2974,21 @@ impl MultiBuffer { buffer.anchor_before(edit_buffer_start)..buffer.anchor_after(edit_buffer_end); for hunk in diff.hunks_intersecting_range(edit_anchor_range, buffer) { + if hunk.is_created_file() && !all_diff_hunks_expanded { + continue; + } + let hunk_buffer_range = hunk.buffer_range.to_offset(buffer); + if hunk_buffer_range.start < excerpt_buffer_start { + log::trace!("skipping hunk that starts before excerpt"); + continue; + } let hunk_info = DiffTransformHunkInfo { excerpt_id: excerpt.id, hunk_start_anchor: hunk.buffer_range.start, hunk_secondary_status: hunk.secondary_status, }; - if hunk_buffer_range.start < excerpt_buffer_start { - log::trace!("skipping hunk that starts before excerpt"); - continue; - } let hunk_excerpt_start = excerpt_start + ExcerptOffset::new( @@ -3028,21 +3011,18 @@ impl MultiBuffer { let was_previously_expanded = old_expanded_hunks.contains(&hunk_info); let should_expand_hunk = match &change_kind { DiffChangeKind::DiffUpdated { base_changed: true } => { - self.all_diff_hunks_expanded || was_previously_expanded + was_previously_expanded || all_diff_hunks_expanded } DiffChangeKind::ExpandOrCollapseHunks { expand } => { let intersects = hunk_buffer_range.is_empty() || hunk_buffer_range.end > edit_buffer_start; if *expand { - intersects - || was_previously_expanded - || self.all_diff_hunks_expanded + intersects || was_previously_expanded || all_diff_hunks_expanded } else { - !intersects - && (was_previously_expanded || self.all_diff_hunks_expanded) + !intersects && (was_previously_expanded || all_diff_hunks_expanded) } } - _ => was_previously_expanded || self.all_diff_hunks_expanded, + _ => was_previously_expanded || all_diff_hunks_expanded, }; if should_expand_hunk { @@ -3057,6 +3037,7 @@ impl MultiBuffer { && hunk_buffer_range.start >= edit_buffer_start && hunk_buffer_range.start <= excerpt_buffer_end { + let base_text = diff.base_text(); let mut text_cursor = base_text.as_rope().cursor(hunk.diff_base_byte_range.start); let mut base_text_summary = @@ -3500,11 +3481,14 @@ impl MultiBufferSnapshot { let buffer_end = buffer.anchor_after(buffer_range.end); Some( diff.hunks_intersecting_range(buffer_start..buffer_end, buffer) - .map(|hunk| { - ( + .filter_map(|hunk| { + if hunk.is_created_file() && !self.all_diff_hunks_expanded { + return None; + } + Some(( Point::new(hunk.row_range.start, 0)..Point::new(hunk.row_range.end, 0), hunk, - ) + )) }), ) }) @@ -4383,8 +4367,7 @@ impl MultiBufferSnapshot { } => { let buffer_start = base_text_byte_range.start + start_overshoot; let mut buffer_end = base_text_byte_range.start + end_overshoot; - let Some(base_text) = self.diffs.get(buffer_id).and_then(|diff| diff.base_text()) - else { + let Some(base_text) = self.diffs.get(buffer_id).map(|diff| diff.base_text()) else { panic!("{:?} is in non-existent deleted hunk", range.start) }; @@ -4432,8 +4415,7 @@ impl MultiBufferSnapshot { .. } => { let buffer_end = base_text_byte_range.start + overshoot; - let Some(base_text) = self.diffs.get(buffer_id).and_then(|diff| diff.base_text()) - else { + let Some(base_text) = self.diffs.get(buffer_id).map(|diff| diff.base_text()) else { panic!("{:?} is in non-existent deleted hunk", range.end) }; @@ -4537,7 +4519,7 @@ impl MultiBufferSnapshot { }) => { if let Some(diff_base_anchor) = &anchor.diff_base_anchor { if let Some(base_text) = - self.diffs.get(buffer_id).and_then(|diff| diff.base_text()) + self.diffs.get(buffer_id).map(|diff| diff.base_text()) { if base_text.can_resolve(&diff_base_anchor) { let base_text_offset = diff_base_anchor.to_offset(&base_text); @@ -4867,17 +4849,14 @@ impl MultiBufferSnapshot { .. }) = diff_transforms.item() { - let base_text = self - .diffs - .get(buffer_id) - .and_then(|diff| diff.base_text()) - .expect("missing diff base"); + let diff = self.diffs.get(buffer_id).expect("missing diff"); if offset_in_transform > base_text_byte_range.len() { debug_assert!(*has_trailing_newline); bias = Bias::Right; } else { diff_base_anchor = Some( - base_text.anchor_at(base_text_byte_range.start + offset_in_transform, bias), + diff.base_text() + .anchor_at(base_text_byte_range.start + offset_in_transform, bias), ); bias = Bias::Left; } @@ -6235,7 +6214,7 @@ where .. } => { let diff = self.diffs.get(&buffer_id)?; - let buffer = diff.base_text()?; + let buffer = diff.base_text(); let mut rope_cursor = buffer.as_rope().cursor(0); let buffer_start = rope_cursor.summary::(base_text_byte_range.start); let buffer_range_len = rope_cursor.summary::(base_text_byte_range.end); @@ -7282,7 +7261,7 @@ impl<'a> Iterator for MultiBufferChunks<'a> { } chunks } else { - let base_buffer = &self.diffs.get(&buffer_id)?.base_text()?; + let base_buffer = &self.diffs.get(&buffer_id)?.base_text(); base_buffer.chunks(base_text_start..base_text_end, self.language_aware) }; diff --git a/crates/multi_buffer/src/multi_buffer_tests.rs b/crates/multi_buffer/src/multi_buffer_tests.rs index 4f4b506bc297e8b99002e8386390dbd3a66e225b..e878e50dd22478d0cb1507024508af5ea7baf863 100644 --- a/crates/multi_buffer/src/multi_buffer_tests.rs +++ b/crates/multi_buffer/src/multi_buffer_tests.rs @@ -1999,8 +1999,8 @@ fn test_diff_hunks_with_multiple_excerpts(cx: &mut TestAppContext) { let id_1 = buffer_1.read_with(cx, |buffer, _| buffer.remote_id()); let id_2 = buffer_2.read_with(cx, |buffer, _| buffer.remote_id()); - let base_id_1 = diff_1.read_with(cx, |diff, _| diff.base_text().as_ref().unwrap().remote_id()); - let base_id_2 = diff_2.read_with(cx, |diff, _| diff.base_text().as_ref().unwrap().remote_id()); + let base_id_1 = diff_1.read_with(cx, |diff, _| diff.base_text().remote_id()); + let base_id_2 = diff_2.read_with(cx, |diff, _| diff.base_text().remote_id()); let buffer_lines = (0..=snapshot.max_row().0) .map(|row| { @@ -2221,8 +2221,7 @@ impl ReferenceMultibuffer { let buffer = excerpt.buffer.read(cx); let buffer_range = excerpt.range.to_offset(buffer); let diff = self.diffs.get(&buffer.remote_id()).unwrap().read(cx); - // let diff = diff.snapshot.clone(); - let base_buffer = diff.base_text().unwrap(); + let base_buffer = diff.base_text(); let mut offset = buffer_range.start; let mut hunks = diff diff --git a/crates/project/src/buffer_store.rs b/crates/project/src/buffer_store.rs index d8a71a534df25ca84b9a279218ea06a75e9f6c0a..dc12a4d84477b65799a9800cadbe49c4fe6377e9 100644 --- a/crates/project/src/buffer_store.rs +++ b/crates/project/src/buffer_store.rs @@ -816,20 +816,20 @@ impl LocalBufferStore { .any(|(work_dir, _)| file.path.starts_with(work_dir)) { let snapshot = buffer.text_snapshot(); + let has_unstaged_diff = diff_state + .unstaged_diff + .as_ref() + .is_some_and(|diff| diff.is_upgradable()); + let has_uncommitted_diff = diff_state + .uncommitted_diff + .as_ref() + .is_some_and(|set| set.is_upgradable()); diff_state_updates.push(( snapshot.clone(), file.path.clone(), - diff_state - .unstaged_diff - .as_ref() - .and_then(|set| set.upgrade()) - .is_some(), - diff_state - .uncommitted_diff - .as_ref() - .and_then(|set| set.upgrade()) - .is_some(), - )) + has_unstaged_diff.then(|| diff_state.index_text.clone()), + has_uncommitted_diff.then(|| diff_state.head_text.clone()), + )); } } @@ -845,37 +845,47 @@ impl LocalBufferStore { diff_state_updates .into_iter() .filter_map( - |(buffer_snapshot, path, needs_staged_text, needs_committed_text)| { + |(buffer_snapshot, path, current_index_text, current_head_text)| { let local_repo = snapshot.local_repo_for_path(&path)?; let relative_path = local_repo.relativize(&path).ok()?; - let staged_text = if needs_staged_text { + let index_text = if current_index_text.is_some() { local_repo.repo().load_index_text(&relative_path) } else { None }; - let committed_text = if needs_committed_text { + let head_text = if current_head_text.is_some() { local_repo.repo().load_committed_text(&relative_path) } else { None }; - let diff_bases_change = - match (needs_staged_text, needs_committed_text) { - (true, true) => Some(if staged_text == committed_text { - DiffBasesChange::SetBoth(committed_text) - } else { - DiffBasesChange::SetEach { - index: staged_text, - head: committed_text, - } - }), - (true, false) => { - Some(DiffBasesChange::SetIndex(staged_text)) - } - (false, true) => { - Some(DiffBasesChange::SetHead(committed_text)) + + // Avoid triggering a diff update if the base text has not changed. + if let Some((current_index, current_head)) = + current_index_text.as_ref().zip(current_head_text.as_ref()) + { + if current_index.as_deref() == index_text.as_ref() + && current_head.as_deref() == head_text.as_ref() + { + return None; + } + } + + let diff_bases_change = match ( + current_index_text.is_some(), + current_head_text.is_some(), + ) { + (true, true) => Some(if index_text == head_text { + DiffBasesChange::SetBoth(head_text) + } else { + DiffBasesChange::SetEach { + index: index_text, + head: head_text, } - (false, false) => None, - }; + }), + (true, false) => Some(DiffBasesChange::SetIndex(index_text)), + (false, true) => Some(DiffBasesChange::SetHead(head_text)), + (false, false) => None, + }; Some((buffer_snapshot, diff_bases_change)) }, ) @@ -1476,14 +1486,15 @@ impl BufferStore { diff_state.language = language; diff_state.language_registry = language_registry; - let diff = cx.new(|_| BufferDiff::new(&text_snapshot)); + let diff = cx.new(|cx| BufferDiff::new(&text_snapshot, cx)); match kind { DiffKind::Unstaged => diff_state.unstaged_diff = Some(diff.downgrade()), DiffKind::Uncommitted => { let unstaged_diff = if let Some(diff) = diff_state.unstaged_diff() { diff } else { - let unstaged_diff = cx.new(|_| BufferDiff::new(&text_snapshot)); + let unstaged_diff = + cx.new(|cx| BufferDiff::new(&text_snapshot, cx)); diff_state.unstaged_diff = Some(unstaged_diff.downgrade()); unstaged_diff }; @@ -2384,8 +2395,7 @@ impl BufferStore { shared.diff = Some(diff.clone()); } })?; - let staged_text = - diff.read_with(&cx, |diff, _| diff.base_text().map(|buffer| buffer.text()))?; + let staged_text = diff.read_with(&cx, |diff, _| diff.base_text_string())?; Ok(proto::OpenUnstagedDiffResponse { staged_text }) } @@ -2415,22 +2425,25 @@ impl BufferStore { diff.read_with(&cx, |diff, cx| { use proto::open_uncommitted_diff_response::Mode; - let staged_buffer = diff - .secondary_diff() - .and_then(|diff| diff.read(cx).base_text()); + let unstaged_diff = diff.secondary_diff(); + let index_snapshot = unstaged_diff.and_then(|diff| { + let diff = diff.read(cx); + diff.base_text_exists().then(|| diff.base_text()) + }); let mode; let staged_text; let committed_text; - if let Some(committed_buffer) = diff.base_text() { - committed_text = Some(committed_buffer.text()); - if let Some(staged_buffer) = staged_buffer { - if staged_buffer.remote_id() == committed_buffer.remote_id() { + if diff.base_text_exists() { + let committed_snapshot = diff.base_text(); + committed_text = Some(committed_snapshot.text()); + if let Some(index_text) = index_snapshot { + if index_text.remote_id() == committed_snapshot.remote_id() { mode = Mode::IndexMatchesHead; staged_text = None; } else { mode = Mode::IndexAndHead; - staged_text = Some(staged_buffer.text()); + staged_text = Some(index_text.text()); } } else { mode = Mode::IndexAndHead; @@ -2439,7 +2452,7 @@ impl BufferStore { } else { mode = Mode::IndexAndHead; committed_text = None; - staged_text = staged_buffer.as_ref().map(|buffer| buffer.text()); + staged_text = index_snapshot.as_ref().map(|buffer| buffer.text()); } proto::OpenUncommittedDiffResponse { diff --git a/crates/project/src/git.rs b/crates/project/src/git.rs index 33e3ff9d6e1ad729dce0dd6a04f1b2a3688666bf..8dda0f8cec2f8f3913c49d3bb41bcfe73ce5ae90 100644 --- a/crates/project/src/git.rs +++ b/crates/project/src/git.rs @@ -18,6 +18,7 @@ use language::{Buffer, LanguageRegistry}; use rpc::proto::{git_reset, ToProto}; use rpc::{proto, AnyProtoClient, TypedEnvelope}; use settings::WorktreeId; +use std::collections::VecDeque; use std::future::Future; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -25,8 +26,6 @@ use text::BufferId; use util::{maybe, ResultExt}; use worktree::{ProjectEntryId, RepositoryEntry, StatusEntry}; -type GitJob = Box Task<()>>; - pub struct GitStore { buffer_store: Entity, pub(super) project_id: Option, @@ -64,6 +63,16 @@ pub enum GitEvent { GitStateUpdated, } +struct GitJob { + job: Box Task<()>>, + key: Option, +} + +#[derive(PartialEq, Eq)] +enum GitJobKey { + WriteIndex(RepoPath), +} + impl EventEmitter for GitStore {} impl GitStore { @@ -223,9 +232,29 @@ impl GitStore { fn spawn_git_worker(cx: &mut Context<'_, GitStore>) -> mpsc::UnboundedSender { let (job_tx, mut job_rx) = mpsc::unbounded::(); + cx.spawn(|_, mut cx| async move { - while let Some(job) = job_rx.next().await { - job(&mut cx).await + let mut jobs = VecDeque::new(); + loop { + while let Ok(Some(next_job)) = job_rx.try_next() { + jobs.push_back(next_job); + } + + if let Some(job) = jobs.pop_front() { + if let Some(current_key) = &job.key { + if jobs + .iter() + .any(|other_job| other_job.key.as_ref() == Some(current_key)) + { + continue; + } + } + (job.job)(&mut cx).await; + } else if let Some(job) = job_rx.next().await { + jobs.push_back(job); + } else { + break; + } } }) .detach(); @@ -567,6 +596,15 @@ impl Repository { } fn send_job(&self, job: F) -> oneshot::Receiver + where + F: FnOnce(GitRepo) -> Fut + 'static, + Fut: Future + Send + 'static, + R: Send + 'static, + { + self.send_keyed_job(None, job) + } + + fn send_keyed_job(&self, key: Option, job: F) -> oneshot::Receiver where F: FnOnce(GitRepo) -> Fut + 'static, Fut: Future + Send + 'static, @@ -575,13 +613,16 @@ impl Repository { let (result_tx, result_rx) = futures::channel::oneshot::channel(); let git_repo = self.git_repo.clone(); self.job_sender - .unbounded_send(Box::new(|cx: &mut AsyncApp| { - let job = job(git_repo); - cx.background_spawn(async move { - let result = job.await; - result_tx.send(result).ok(); - }) - })) + .unbounded_send(GitJob { + key, + job: Box::new(|cx: &mut AsyncApp| { + let job = job(git_repo); + cx.background_spawn(async move { + let result = job.await; + result_tx.send(result).ok(); + }) + }), + }) .ok(); result_rx } @@ -1144,28 +1185,31 @@ impl Repository { content: Option, ) -> oneshot::Receiver> { let path = path.clone(); - self.send_job(|git_repo| async move { - match git_repo { - GitRepo::Local(repo) => repo.set_index_text(&path, content), - GitRepo::Remote { - project_id, - client, - worktree_id, - work_directory_id, - } => { - client - .request(proto::SetIndexText { - project_id: project_id.0, - worktree_id: worktree_id.to_proto(), - work_directory_id: work_directory_id.to_proto(), - path: path.as_ref().to_proto(), - text: content, - }) - .await?; - Ok(()) + self.send_keyed_job( + Some(GitJobKey::WriteIndex(path.clone())), + |git_repo| async move { + match git_repo { + GitRepo::Local(repo) => repo.set_index_text(&path, content), + GitRepo::Remote { + project_id, + client, + worktree_id, + work_directory_id, + } => { + client + .request(proto::SetIndexText { + project_id: project_id.0, + worktree_id: worktree_id.to_proto(), + work_directory_id: work_directory_id.to_proto(), + path: path.as_ref().to_proto(), + text: content, + }) + .await?; + Ok(()) + } } - } - }) + }, + ) } pub fn get_remotes( diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 8086ebbf98fc0e1d2e298ab2665ad890fb3a6198..e4e2335d3d4c8b1166b008750996bff3716b2c86 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -1,5 +1,5 @@ use crate::{task_inventory::TaskContexts, Event, *}; -use buffer_diff::{assert_hunks, DiffHunkSecondaryStatus, DiffHunkStatus}; +use buffer_diff::{assert_hunks, DiffHunkSecondaryStatus, DiffHunkStatus, DiffHunkStatusKind}; use fs::FakeFs; use futures::{future, StreamExt}; use gpui::{App, SemanticVersion, UpdateGlobal}; @@ -5819,7 +5819,7 @@ async fn test_unstaged_diff_for_buffer(cx: &mut gpui::TestAppContext) { assert_hunks( unstaged_diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), &snapshot, - &unstaged_diff.base_text().unwrap().text(), + &unstaged_diff.base_text().text(), &[( 2..3, "", @@ -5860,19 +5860,25 @@ async fn test_uncommitted_diff_for_buffer(cx: &mut gpui::TestAppContext) { json!({ ".git": {}, "src": { - "main.rs": file_contents, + "modification.rs": file_contents, } }), ) .await; - fs.set_index_for_repo( + fs.set_head_for_repo( Path::new("/dir/.git"), - &[("src/main.rs".into(), staged_contents)], + &[ + ("src/modification.rs".into(), committed_contents), + ("src/deletion.rs".into(), "// the-deleted-contents\n".into()), + ], ); - fs.set_head_for_repo( + fs.set_index_for_repo( Path::new("/dir/.git"), - &[("src/main.rs".into(), committed_contents)], + &[ + ("src/modification.rs".into(), staged_contents), + ("src/deletion.rs".into(), "// the-deleted-contents\n".into()), + ], ); let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await; @@ -5880,33 +5886,28 @@ async fn test_uncommitted_diff_for_buffer(cx: &mut gpui::TestAppContext) { let language = rust_lang(); language_registry.add(language.clone()); - let buffer = project + let buffer_1 = project .update(cx, |project, cx| { - project.open_local_buffer("/dir/src/main.rs", cx) + project.open_local_buffer("/dir/src/modification.rs", cx) }) .await .unwrap(); - let uncommitted_diff = project + let diff_1 = project .update(cx, |project, cx| { - project.open_uncommitted_diff(buffer.clone(), cx) + project.open_uncommitted_diff(buffer_1.clone(), cx) }) .await .unwrap(); - - uncommitted_diff.read_with(cx, |diff, _| { - assert_eq!( - diff.base_text().and_then(|base| base.language().cloned()), - Some(language) - ) + diff_1.read_with(cx, |diff, _| { + assert_eq!(diff.base_text().language().cloned(), Some(language)) }); - cx.run_until_parked(); - uncommitted_diff.update(cx, |uncommitted_diff, cx| { - let snapshot = buffer.read(cx).snapshot(); + diff_1.update(cx, |diff, cx| { + let snapshot = buffer_1.read(cx).snapshot(); assert_hunks( - uncommitted_diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), + diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), &snapshot, - &uncommitted_diff.base_text_string().unwrap(), + &diff.base_text_string().unwrap(), &[ ( 0..1, @@ -5924,25 +5925,29 @@ async fn test_uncommitted_diff_for_buffer(cx: &mut gpui::TestAppContext) { ); }); + // Reset HEAD to a version that differs from both the buffer and the index. let committed_contents = r#" // print goodbye fn main() { } "# .unindent(); - fs.set_head_for_repo( Path::new("/dir/.git"), - &[("src/main.rs".into(), committed_contents)], + &[ + ("src/modification.rs".into(), committed_contents.clone()), + ("src/deletion.rs".into(), "// the-deleted-contents\n".into()), + ], ); + // Buffer now has an unstaged hunk. cx.run_until_parked(); - uncommitted_diff.update(cx, |uncommitted_diff, cx| { - let snapshot = buffer.read(cx).snapshot(); + diff_1.update(cx, |diff, cx| { + let snapshot = buffer_1.read(cx).snapshot(); assert_hunks( - uncommitted_diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), + diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), &snapshot, - &uncommitted_diff.base_text().unwrap().text(), + &diff.base_text().text(), &[( 2..3, "", @@ -5951,6 +5956,56 @@ async fn test_uncommitted_diff_for_buffer(cx: &mut gpui::TestAppContext) { )], ); }); + + // Open a buffer for a file that's been deleted. + let buffer_2 = project + .update(cx, |project, cx| { + project.open_local_buffer("/dir/src/deletion.rs", cx) + }) + .await + .unwrap(); + let diff_2 = project + .update(cx, |project, cx| { + project.open_uncommitted_diff(buffer_2.clone(), cx) + }) + .await + .unwrap(); + cx.run_until_parked(); + diff_2.update(cx, |diff, cx| { + let snapshot = buffer_2.read(cx).snapshot(); + assert_hunks( + diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), + &snapshot, + &diff.base_text_string().unwrap(), + &[( + 0..0, + "// the-deleted-contents\n", + "", + DiffHunkStatus::deleted(DiffHunkSecondaryStatus::HasSecondaryHunk), + )], + ); + }); + + // Stage the deletion of this file + fs.set_index_for_repo( + Path::new("/dir/.git"), + &[("src/modification.rs".into(), committed_contents.clone())], + ); + cx.run_until_parked(); + diff_2.update(cx, |diff, cx| { + let snapshot = buffer_2.read(cx).snapshot(); + assert_hunks( + diff.hunks_intersecting_range(Anchor::MIN..Anchor::MAX, &snapshot, cx), + &snapshot, + &diff.base_text_string().unwrap(), + &[( + 0..0, + "// the-deleted-contents\n", + "", + DiffHunkStatus::deleted(DiffHunkSecondaryStatus::None), + )], + ); + }); } #[gpui::test] @@ -5958,16 +6013,16 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) { init_test(cx); let committed_contents = r#" - fn main() { - println!("hello from HEAD"); - } - "# + fn main() { + println!("hello from HEAD"); + } + "# .unindent(); let file_contents = r#" - fn main() { - println!("hello from the working copy"); - } - "# + fn main() { + println!("hello from the working copy"); + } + "# .unindent(); let fs = FakeFs::new(cx.background_executor.clone()); @@ -5984,7 +6039,11 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) { fs.set_head_for_repo( Path::new("/dir/.git"), - &[("src/main.rs".into(), committed_contents)], + &[("src/main.rs".into(), committed_contents.clone())], + ); + fs.set_index_for_repo( + Path::new("/dir/.git"), + &[("src/main.rs".into(), committed_contents.clone())], ); let project = Project::test(fs.clone(), ["/dir/src/main.rs".as_ref()], cx).await; @@ -6013,7 +6072,10 @@ async fn test_single_file_diffs(cx: &mut gpui::TestAppContext) { 1..2, " println!(\"hello from HEAD\");\n", " println!(\"hello from the working copy\");\n", - DiffHunkStatus::modified_none(), + DiffHunkStatus { + kind: DiffHunkStatusKind::Modified, + secondary: DiffHunkSecondaryStatus::HasSecondaryHunk, + }, )], ); });