Restructure git diff state management to allow viewing buffers with different diff bases (#21258)
This is a pure refactor of our Git diff state management. Buffers are no longer are associated with one single diff (the unstaged changes). Instead, there is an explicit project API for retrieving a buffer's unstaged changes, and the `Editor` view layer is responsible for choosing what diff to associate with a buffer. The reason for this change is that we'll soon want to add multiple "git diff views" to Zed, one of which will show the *uncommitted* changes for a buffer. But that view will need to co-exist with other views of the same buffer, which may want to show the unstaged changes. ### Todo * [x] Get git gutter and git hunks working with new structure * [x] Update editor tests to use new APIs * [x] Update buffer tests * [x] Restructure remoting/collab protocol * [x] Update assertions about staged text in `random_project_collaboration_tests` * [x] Move buffer tests for git diff management to a new spot, using the new APIs Release Notes: - N/A --------- Co-authored-by: Richard <richard@zed.dev> Co-authored-by: Cole <cole@zed.dev> Co-authored-by: Conrad <conrad@zed.dev>
This commit is contained in:
parent
31796171de
commit
a2115e7242
29 changed files with 1832 additions and 1651 deletions
|
@ -90,22 +90,11 @@ pub enum Capability {
|
|||
|
||||
pub type BufferRow = u32;
|
||||
|
||||
#[derive(Clone)]
|
||||
enum BufferDiffBase {
|
||||
Git(Rope),
|
||||
PastBufferVersion {
|
||||
buffer: Model<Buffer>,
|
||||
rope: Rope,
|
||||
merged_operations: Vec<Lamport>,
|
||||
},
|
||||
}
|
||||
|
||||
/// An in-memory representation of a source code file, including its text,
|
||||
/// syntax trees, git status, and diagnostics.
|
||||
pub struct Buffer {
|
||||
text: TextBuffer,
|
||||
diff_base: Option<BufferDiffBase>,
|
||||
git_diff: git::diff::BufferDiff,
|
||||
branch_state: Option<BufferBranchState>,
|
||||
/// Filesystem state, `None` when there is no path.
|
||||
file: Option<Arc<dyn File>>,
|
||||
/// The mtime of the file when this buffer was last loaded from
|
||||
|
@ -135,7 +124,6 @@ pub struct Buffer {
|
|||
deferred_ops: OperationQueue<Operation>,
|
||||
capability: Capability,
|
||||
has_conflict: bool,
|
||||
diff_base_version: usize,
|
||||
/// Memoize calls to has_changes_since(saved_version).
|
||||
/// The contents of a cell are (self.version, has_changes) at the time of a last call.
|
||||
has_unsaved_edits: Cell<(clock::Global, bool)>,
|
||||
|
@ -148,11 +136,15 @@ pub enum ParseStatus {
|
|||
Parsing,
|
||||
}
|
||||
|
||||
struct BufferBranchState {
|
||||
base_buffer: Model<Buffer>,
|
||||
merged_operations: Vec<Lamport>,
|
||||
}
|
||||
|
||||
/// An immutable, cheaply cloneable representation of a fixed
|
||||
/// state of a buffer.
|
||||
pub struct BufferSnapshot {
|
||||
text: text::BufferSnapshot,
|
||||
git_diff: git::diff::BufferDiff,
|
||||
pub(crate) syntax: SyntaxSnapshot,
|
||||
file: Option<Arc<dyn File>>,
|
||||
diagnostics: SmallVec<[(LanguageServerId, DiagnosticSet); 2]>,
|
||||
|
@ -345,10 +337,6 @@ pub enum BufferEvent {
|
|||
Reloaded,
|
||||
/// The buffer is in need of a reload
|
||||
ReloadNeeded,
|
||||
/// The buffer's diff_base changed.
|
||||
DiffBaseChanged,
|
||||
/// Buffer's excerpts for a certain diff base were recalculated.
|
||||
DiffUpdated,
|
||||
/// The buffer's language was changed.
|
||||
LanguageChanged,
|
||||
/// The buffer's syntax trees were updated.
|
||||
|
@ -626,7 +614,6 @@ impl Buffer {
|
|||
Self::build(
|
||||
TextBuffer::new(0, cx.entity_id().as_non_zero_u64().into(), base_text.into()),
|
||||
None,
|
||||
None,
|
||||
Capability::ReadWrite,
|
||||
)
|
||||
}
|
||||
|
@ -645,7 +632,6 @@ impl Buffer {
|
|||
base_text_normalized,
|
||||
),
|
||||
None,
|
||||
None,
|
||||
Capability::ReadWrite,
|
||||
)
|
||||
}
|
||||
|
@ -660,7 +646,6 @@ impl Buffer {
|
|||
Self::build(
|
||||
TextBuffer::new(replica_id, remote_id, base_text.into()),
|
||||
None,
|
||||
None,
|
||||
capability,
|
||||
)
|
||||
}
|
||||
|
@ -676,7 +661,7 @@ impl Buffer {
|
|||
let buffer_id = BufferId::new(message.id)
|
||||
.with_context(|| anyhow!("Could not deserialize buffer_id"))?;
|
||||
let buffer = TextBuffer::new(replica_id, buffer_id, message.base_text);
|
||||
let mut this = Self::build(buffer, message.diff_base, file, capability);
|
||||
let mut this = Self::build(buffer, file, capability);
|
||||
this.text.set_line_ending(proto::deserialize_line_ending(
|
||||
rpc::proto::LineEnding::from_i32(message.line_ending)
|
||||
.ok_or_else(|| anyhow!("missing line_ending"))?,
|
||||
|
@ -692,7 +677,6 @@ impl Buffer {
|
|||
id: self.remote_id().into(),
|
||||
file: self.file.as_ref().map(|f| f.to_proto(cx)),
|
||||
base_text: self.base_text().to_string(),
|
||||
diff_base: self.diff_base().as_ref().map(|h| h.to_string()),
|
||||
line_ending: proto::serialize_line_ending(self.line_ending()) as i32,
|
||||
saved_version: proto::serialize_version(&self.saved_version),
|
||||
saved_mtime: self.saved_mtime.map(|time| time.into()),
|
||||
|
@ -766,15 +750,9 @@ impl Buffer {
|
|||
}
|
||||
|
||||
/// Builds a [`Buffer`] with the given underlying [`TextBuffer`], diff base, [`File`] and [`Capability`].
|
||||
pub fn build(
|
||||
buffer: TextBuffer,
|
||||
diff_base: Option<String>,
|
||||
file: Option<Arc<dyn File>>,
|
||||
capability: Capability,
|
||||
) -> Self {
|
||||
pub fn build(buffer: TextBuffer, file: Option<Arc<dyn File>>, capability: Capability) -> Self {
|
||||
let saved_mtime = file.as_ref().and_then(|file| file.disk_state().mtime());
|
||||
let snapshot = buffer.snapshot();
|
||||
let git_diff = git::diff::BufferDiff::new(&snapshot);
|
||||
let syntax_map = Mutex::new(SyntaxMap::new(&snapshot));
|
||||
Self {
|
||||
saved_mtime,
|
||||
|
@ -785,12 +763,7 @@ impl Buffer {
|
|||
was_dirty_before_starting_transaction: None,
|
||||
has_unsaved_edits: Cell::new((buffer.version(), false)),
|
||||
text: buffer,
|
||||
diff_base: diff_base.map(|mut raw_diff_base| {
|
||||
LineEnding::normalize(&mut raw_diff_base);
|
||||
BufferDiffBase::Git(Rope::from(raw_diff_base))
|
||||
}),
|
||||
diff_base_version: 0,
|
||||
git_diff,
|
||||
branch_state: None,
|
||||
file,
|
||||
capability,
|
||||
syntax_map,
|
||||
|
@ -824,7 +797,6 @@ impl Buffer {
|
|||
BufferSnapshot {
|
||||
text,
|
||||
syntax,
|
||||
git_diff: self.git_diff.clone(),
|
||||
file: self.file.clone(),
|
||||
remote_selections: self.remote_selections.clone(),
|
||||
diagnostics: self.diagnostics.clone(),
|
||||
|
@ -837,21 +809,15 @@ impl Buffer {
|
|||
let this = cx.handle();
|
||||
cx.new_model(|cx| {
|
||||
let mut branch = Self {
|
||||
diff_base: Some(BufferDiffBase::PastBufferVersion {
|
||||
buffer: this.clone(),
|
||||
rope: self.as_rope().clone(),
|
||||
branch_state: Some(BufferBranchState {
|
||||
base_buffer: this.clone(),
|
||||
merged_operations: Default::default(),
|
||||
}),
|
||||
language: self.language.clone(),
|
||||
has_conflict: self.has_conflict,
|
||||
has_unsaved_edits: Cell::new(self.has_unsaved_edits.get_mut().clone()),
|
||||
_subscriptions: vec![cx.subscribe(&this, Self::on_base_buffer_event)],
|
||||
..Self::build(
|
||||
self.text.branch(),
|
||||
None,
|
||||
self.file.clone(),
|
||||
self.capability(),
|
||||
)
|
||||
..Self::build(self.text.branch(), self.file.clone(), self.capability())
|
||||
};
|
||||
if let Some(language_registry) = self.language_registry() {
|
||||
branch.set_language_registry(language_registry);
|
||||
|
@ -870,7 +836,7 @@ impl Buffer {
|
|||
/// If `ranges` is empty, then all changes will be applied. This buffer must
|
||||
/// be a branch buffer to call this method.
|
||||
pub fn merge_into_base(&mut self, ranges: Vec<Range<usize>>, cx: &mut ModelContext<Self>) {
|
||||
let Some(base_buffer) = self.diff_base_buffer() else {
|
||||
let Some(base_buffer) = self.base_buffer() else {
|
||||
debug_panic!("not a branch buffer");
|
||||
return;
|
||||
};
|
||||
|
@ -906,14 +872,14 @@ impl Buffer {
|
|||
}
|
||||
|
||||
let operation = base_buffer.update(cx, |base_buffer, cx| {
|
||||
cx.emit(BufferEvent::DiffBaseChanged);
|
||||
// cx.emit(BufferEvent::DiffBaseChanged);
|
||||
base_buffer.edit(edits, None, cx)
|
||||
});
|
||||
|
||||
if let Some(operation) = operation {
|
||||
if let Some(BufferDiffBase::PastBufferVersion {
|
||||
if let Some(BufferBranchState {
|
||||
merged_operations, ..
|
||||
}) = &mut self.diff_base
|
||||
}) = &mut self.branch_state
|
||||
{
|
||||
merged_operations.push(operation);
|
||||
}
|
||||
|
@ -929,9 +895,9 @@ impl Buffer {
|
|||
let BufferEvent::Operation { operation, .. } = event else {
|
||||
return;
|
||||
};
|
||||
let Some(BufferDiffBase::PastBufferVersion {
|
||||
let Some(BufferBranchState {
|
||||
merged_operations, ..
|
||||
}) = &mut self.diff_base
|
||||
}) = &mut self.branch_state
|
||||
else {
|
||||
return;
|
||||
};
|
||||
|
@ -950,8 +916,6 @@ impl Buffer {
|
|||
let counts = [(timestamp, u32::MAX)].into_iter().collect();
|
||||
self.undo_operations(counts, cx);
|
||||
}
|
||||
|
||||
self.diff_base_version += 1;
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
@ -1123,74 +1087,8 @@ impl Buffer {
|
|||
}
|
||||
}
|
||||
|
||||
/// Returns the current diff base, see [`Buffer::set_diff_base`].
|
||||
pub fn diff_base(&self) -> Option<&Rope> {
|
||||
match self.diff_base.as_ref()? {
|
||||
BufferDiffBase::Git(rope) | BufferDiffBase::PastBufferVersion { rope, .. } => {
|
||||
Some(rope)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Sets the text that will be used to compute a Git diff
|
||||
/// against the buffer text.
|
||||
pub fn set_diff_base(&mut self, diff_base: Option<String>, cx: &ModelContext<Self>) {
|
||||
self.diff_base = diff_base.map(|mut raw_diff_base| {
|
||||
LineEnding::normalize(&mut raw_diff_base);
|
||||
BufferDiffBase::Git(Rope::from(raw_diff_base))
|
||||
});
|
||||
self.diff_base_version += 1;
|
||||
if let Some(recalc_task) = self.recalculate_diff(cx) {
|
||||
cx.spawn(|buffer, mut cx| async move {
|
||||
recalc_task.await;
|
||||
buffer
|
||||
.update(&mut cx, |_, cx| {
|
||||
cx.emit(BufferEvent::DiffBaseChanged);
|
||||
})
|
||||
.ok();
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
}
|
||||
|
||||
/// Returns a number, unique per diff base set to the buffer.
|
||||
pub fn diff_base_version(&self) -> usize {
|
||||
self.diff_base_version
|
||||
}
|
||||
|
||||
pub fn diff_base_buffer(&self) -> Option<Model<Self>> {
|
||||
match self.diff_base.as_ref()? {
|
||||
BufferDiffBase::Git(_) => None,
|
||||
BufferDiffBase::PastBufferVersion { buffer, .. } => Some(buffer.clone()),
|
||||
}
|
||||
}
|
||||
|
||||
/// Recomputes the diff.
|
||||
pub fn recalculate_diff(&self, cx: &ModelContext<Self>) -> Option<Task<()>> {
|
||||
let diff_base_rope = match self.diff_base.as_ref()? {
|
||||
BufferDiffBase::Git(rope) => rope.clone(),
|
||||
BufferDiffBase::PastBufferVersion { buffer, .. } => buffer.read(cx).as_rope().clone(),
|
||||
};
|
||||
|
||||
let snapshot = self.snapshot();
|
||||
let mut diff = self.git_diff.clone();
|
||||
let diff = cx.background_executor().spawn(async move {
|
||||
diff.update(&diff_base_rope, &snapshot).await;
|
||||
(diff, diff_base_rope)
|
||||
});
|
||||
|
||||
Some(cx.spawn(|this, mut cx| async move {
|
||||
let (buffer_diff, diff_base_rope) = diff.await;
|
||||
this.update(&mut cx, |this, cx| {
|
||||
this.git_diff = buffer_diff;
|
||||
this.non_text_state_update_count += 1;
|
||||
if let Some(BufferDiffBase::PastBufferVersion { rope, .. }) = &mut this.diff_base {
|
||||
*rope = diff_base_rope;
|
||||
}
|
||||
cx.emit(BufferEvent::DiffUpdated);
|
||||
})
|
||||
.ok();
|
||||
}))
|
||||
pub fn base_buffer(&self) -> Option<Model<Self>> {
|
||||
Some(self.branch_state.as_ref()?.base_buffer.clone())
|
||||
}
|
||||
|
||||
/// Returns the primary [`Language`] assigned to this [`Buffer`].
|
||||
|
@ -3992,37 +3890,6 @@ impl BufferSnapshot {
|
|||
})
|
||||
}
|
||||
|
||||
/// Whether the buffer contains any Git changes.
|
||||
pub fn has_git_diff(&self) -> bool {
|
||||
!self.git_diff.is_empty()
|
||||
}
|
||||
|
||||
/// Returns all the Git diff hunks intersecting the given row range.
|
||||
pub fn git_diff_hunks_in_row_range(
|
||||
&self,
|
||||
range: Range<BufferRow>,
|
||||
) -> impl '_ + Iterator<Item = git::diff::DiffHunk> {
|
||||
self.git_diff.hunks_in_row_range(range, self)
|
||||
}
|
||||
|
||||
/// Returns all the Git diff hunks intersecting the given
|
||||
/// range.
|
||||
pub fn git_diff_hunks_intersecting_range(
|
||||
&self,
|
||||
range: Range<Anchor>,
|
||||
) -> impl '_ + Iterator<Item = git::diff::DiffHunk> {
|
||||
self.git_diff.hunks_intersecting_range(range, self)
|
||||
}
|
||||
|
||||
/// Returns all the Git diff hunks intersecting the given
|
||||
/// range, in reverse order.
|
||||
pub fn git_diff_hunks_intersecting_range_rev(
|
||||
&self,
|
||||
range: Range<Anchor>,
|
||||
) -> impl '_ + Iterator<Item = git::diff::DiffHunk> {
|
||||
self.git_diff.hunks_intersecting_range_rev(range, self)
|
||||
}
|
||||
|
||||
/// Returns if the buffer contains any diagnostics.
|
||||
pub fn has_diagnostics(&self) -> bool {
|
||||
!self.diagnostics.is_empty()
|
||||
|
@ -4167,7 +4034,6 @@ impl Clone for BufferSnapshot {
|
|||
fn clone(&self) -> Self {
|
||||
Self {
|
||||
text: self.text.clone(),
|
||||
git_diff: self.git_diff.clone(),
|
||||
syntax: self.syntax.clone(),
|
||||
file: self.file.clone(),
|
||||
remote_selections: self.remote_selections.clone(),
|
||||
|
|
|
@ -6,7 +6,6 @@ use crate::Buffer;
|
|||
use clock::ReplicaId;
|
||||
use collections::BTreeMap;
|
||||
use futures::FutureExt as _;
|
||||
use git::diff::assert_hunks;
|
||||
use gpui::{AppContext, BorrowAppContext, Model};
|
||||
use gpui::{Context, TestAppContext};
|
||||
use indoc::indoc;
|
||||
|
@ -2608,15 +2607,6 @@ fn test_branch_and_merge(cx: &mut TestAppContext) {
|
|||
);
|
||||
});
|
||||
|
||||
// The branch buffer maintains a diff with respect to its base buffer.
|
||||
start_recalculating_diff(&branch, cx);
|
||||
cx.run_until_parked();
|
||||
assert_diff_hunks(
|
||||
&branch,
|
||||
cx,
|
||||
&[(1..2, "", "1.5\n"), (3..4, "three\n", "THREE\n")],
|
||||
);
|
||||
|
||||
// Edits to the base are applied to the branch.
|
||||
base.update(cx, |buffer, cx| {
|
||||
buffer.edit([(Point::new(0, 0)..Point::new(0, 0), "ZERO\n")], None, cx)
|
||||
|
@ -2626,21 +2616,6 @@ fn test_branch_and_merge(cx: &mut TestAppContext) {
|
|||
assert_eq!(buffer.text(), "ZERO\none\n1.5\ntwo\nTHREE\n");
|
||||
});
|
||||
|
||||
// Until the git diff recalculation is complete, the git diff references
|
||||
// the previous content of the base buffer, so that it stays in sync.
|
||||
start_recalculating_diff(&branch, cx);
|
||||
assert_diff_hunks(
|
||||
&branch,
|
||||
cx,
|
||||
&[(2..3, "", "1.5\n"), (4..5, "three\n", "THREE\n")],
|
||||
);
|
||||
cx.run_until_parked();
|
||||
assert_diff_hunks(
|
||||
&branch,
|
||||
cx,
|
||||
&[(2..3, "", "1.5\n"), (4..5, "three\n", "THREE\n")],
|
||||
);
|
||||
|
||||
// Edits to any replica of the base are applied to the branch.
|
||||
base_replica.update(cx, |buffer, cx| {
|
||||
buffer.edit([(Point::new(2, 0)..Point::new(2, 0), "2.5\n")], None, cx)
|
||||
|
@ -2731,29 +2706,6 @@ fn test_undo_after_merge_into_base(cx: &mut TestAppContext) {
|
|||
branch.read_with(cx, |branch, _| assert_eq!(branch.text(), "ABCdefgHIjk"));
|
||||
}
|
||||
|
||||
fn start_recalculating_diff(buffer: &Model<Buffer>, cx: &mut TestAppContext) {
|
||||
buffer
|
||||
.update(cx, |buffer, cx| buffer.recalculate_diff(cx).unwrap())
|
||||
.detach();
|
||||
}
|
||||
|
||||
#[track_caller]
|
||||
fn assert_diff_hunks(
|
||||
buffer: &Model<Buffer>,
|
||||
cx: &mut TestAppContext,
|
||||
expected_hunks: &[(Range<u32>, &str, &str)],
|
||||
) {
|
||||
let (snapshot, diff_base) = buffer.read_with(cx, |buffer, _| {
|
||||
(buffer.snapshot(), buffer.diff_base().unwrap().to_string())
|
||||
});
|
||||
assert_hunks(
|
||||
snapshot.git_diff_hunks_intersecting_range(Anchor::MIN..Anchor::MAX),
|
||||
&snapshot,
|
||||
&diff_base,
|
||||
expected_hunks,
|
||||
);
|
||||
}
|
||||
|
||||
#[gpui::test(iterations = 100)]
|
||||
fn test_random_collaboration(cx: &mut AppContext, mut rng: StdRng) {
|
||||
let min_peers = env::var("MIN_PEERS")
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue