Cole Miller created
Imitating the approach of #41829. Prevents e.g. reverting a hunk and
having that excerpt yanked out from under the cursor.
Release Notes:
- git: Improved stability of excerpts when editing in the project diff.crates/acp_thread/src/diff.rs | 2
crates/agent_ui/src/agent_diff.rs | 7 +
crates/editor/src/editor.rs | 40 ----------
crates/editor/src/items.rs | 1
crates/git_ui/src/project_diff.rs | 114 +++++++++++++++++++++++-------
crates/multi_buffer/src/path_key.rs | 15 ++-
6 files changed, 104 insertions(+), 75 deletions(-)
@@ -166,7 +166,7 @@ impl Diff {
}
pub fn has_revealed_range(&self, cx: &App) -> bool {
- self.multibuffer().read(cx).excerpt_paths().next().is_some()
+ self.multibuffer().read(cx).paths().next().is_some()
}
pub fn needs_update(&self, old_text: &str, new_text: &str, cx: &App) -> bool {
@@ -130,7 +130,12 @@ impl AgentDiffPane {
.action_log()
.read(cx)
.changed_buffers(cx);
- let mut paths_to_delete = self.multibuffer.read(cx).paths().collect::<HashSet<_>>();
+ let mut paths_to_delete = self
+ .multibuffer
+ .read(cx)
+ .paths()
+ .cloned()
+ .collect::<HashSet<_>>();
for (buffer, diff_handle) in changed_buffers {
if buffer.read(cx).file().is_none() {
@@ -22956,10 +22956,7 @@ impl Editor {
window: &mut Window,
cx: &mut Context<Self>,
) {
- let workspace = self.workspace();
- let project = self.project();
- let save_tasks = self.buffer().update(cx, |multi_buffer, cx| {
- let mut tasks = Vec::new();
+ self.buffer().update(cx, |multi_buffer, cx| {
for (buffer_id, changes) in revert_changes {
if let Some(buffer) = multi_buffer.buffer(buffer_id) {
buffer.update(cx, |buffer, cx| {
@@ -22971,44 +22968,9 @@ impl Editor {
cx,
);
});
-
- if let Some(project) =
- project.filter(|_| multi_buffer.all_diff_hunks_expanded())
- {
- project.update(cx, |project, cx| {
- tasks.push((buffer.clone(), project.save_buffer(buffer, cx)));
- })
- }
}
}
- tasks
});
- cx.spawn_in(window, async move |_, cx| {
- for (buffer, task) in save_tasks {
- let result = task.await;
- if result.is_err() {
- let Some(path) = buffer
- .read_with(cx, |buffer, cx| buffer.project_path(cx))
- .ok()
- else {
- continue;
- };
- if let Some((workspace, path)) = workspace.as_ref().zip(path) {
- let Some(task) = cx
- .update_window_entity(workspace, |workspace, window, cx| {
- workspace
- .open_path_preview(path, None, false, false, false, window, cx)
- })
- .ok()
- else {
- continue;
- };
- task.await.log_err();
- }
- }
- }
- })
- .detach();
self.change_selections(SelectionEffects::no_scroll(), window, cx, |selections| {
selections.refresh()
});
@@ -842,7 +842,6 @@ impl Item for Editor {
.map(|handle| handle.read(cx).base_buffer().unwrap_or(handle.clone()))
.collect::<HashSet<_>>();
- // let mut buffers_to_save =
let buffers_to_save = if self.buffer.read(cx).is_singleton() && !options.autosave {
buffers
} else {
@@ -74,6 +74,13 @@ pub struct ProjectDiff {
_subscription: Subscription,
}
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub enum RefreshReason {
+ DiffChanged,
+ StatusesChanged,
+ EditorSaved,
+}
+
const CONFLICT_SORT_PREFIX: u64 = 1;
const TRACKED_SORT_PREFIX: u64 = 2;
const NEW_SORT_PREFIX: u64 = 3;
@@ -278,7 +285,7 @@ impl ProjectDiff {
BranchDiffEvent::FileListChanged => {
this._task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
})
}
},
@@ -297,7 +304,7 @@ impl ProjectDiff {
this._task = {
window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
})
}
}
@@ -308,7 +315,7 @@ impl ProjectDiff {
let task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::StatusesChanged, cx).await
});
Self {
@@ -448,19 +455,27 @@ impl ProjectDiff {
window: &mut Window,
cx: &mut Context<Self>,
) {
- if let EditorEvent::SelectionsChanged { local: true } = event {
- let Some(project_path) = self.active_path(cx) else {
- return;
- };
- self.workspace
- .update(cx, |workspace, cx| {
- if let Some(git_panel) = workspace.panel::<GitPanel>(cx) {
- git_panel.update(cx, |git_panel, cx| {
- git_panel.select_entry_by_path(project_path, window, cx)
- })
- }
- })
- .ok();
+ match event {
+ EditorEvent::SelectionsChanged { local: true } => {
+ let Some(project_path) = self.active_path(cx) else {
+ return;
+ };
+ self.workspace
+ .update(cx, |workspace, cx| {
+ if let Some(git_panel) = workspace.panel::<GitPanel>(cx) {
+ git_panel.update(cx, |git_panel, cx| {
+ git_panel.select_entry_by_path(project_path, window, cx)
+ })
+ }
+ })
+ .ok();
+ }
+ EditorEvent::Saved => {
+ self._task = cx.spawn_in(window, async move |this, cx| {
+ Self::refresh(this, RefreshReason::EditorSaved, cx).await
+ });
+ }
+ _ => {}
}
if editor.focus_handle(cx).contains_focused(window, cx)
&& self.multibuffer.read(cx).is_empty()
@@ -482,7 +497,7 @@ impl ProjectDiff {
let subscription = cx.subscribe_in(&diff, window, move |this, _, _, window, cx| {
this._task = window.spawn(cx, {
let this = cx.weak_entity();
- async |cx| Self::refresh(this, cx).await
+ async |cx| Self::refresh(this, RefreshReason::DiffChanged, cx).await
})
});
self.buffer_diff_subscriptions
@@ -581,14 +596,23 @@ impl ProjectDiff {
}
}
- pub async fn refresh(this: WeakEntity<Self>, cx: &mut AsyncWindowContext) -> Result<()> {
+ pub async fn refresh(
+ this: WeakEntity<Self>,
+ reason: RefreshReason,
+ cx: &mut AsyncWindowContext,
+ ) -> Result<()> {
let mut path_keys = Vec::new();
let buffers_to_load = this.update(cx, |this, cx| {
let (repo, buffers_to_load) = this.branch_diff.update(cx, |branch_diff, cx| {
let load_buffers = branch_diff.load_buffers(cx);
(branch_diff.repo().cloned(), load_buffers)
});
- let mut previous_paths = this.multibuffer.read(cx).paths().collect::<HashSet<_>>();
+ let mut previous_paths = this
+ .multibuffer
+ .read(cx)
+ .paths()
+ .cloned()
+ .collect::<HashSet<_>>();
if let Some(repo) = repo {
let repo = repo.read(cx);
@@ -605,8 +629,20 @@ impl ProjectDiff {
this.multibuffer.update(cx, |multibuffer, cx| {
for path in previous_paths {
+ if let Some(buffer) = multibuffer.buffer_for_path(&path, cx) {
+ let skip = match reason {
+ RefreshReason::DiffChanged | RefreshReason::EditorSaved => {
+ buffer.read(cx).is_dirty()
+ }
+ RefreshReason::StatusesChanged => false,
+ };
+ if skip {
+ continue;
+ }
+ }
+
this.buffer_diff_subscriptions.remove(&path.path);
- multibuffer.remove_excerpts_for_path(path, cx);
+ multibuffer.remove_excerpts_for_path(path.clone(), cx);
}
});
buffers_to_load
@@ -619,7 +655,27 @@ impl ProjectDiff {
yield_now().await;
cx.update(|window, cx| {
this.update(cx, |this, cx| {
- this.register_buffer(path_key, entry.file_status, buffer, diff, window, cx)
+ let multibuffer = this.multibuffer.read(cx);
+ let skip = multibuffer.buffer(buffer.read(cx).remote_id()).is_some()
+ && multibuffer
+ .diff_for(buffer.read(cx).remote_id())
+ .is_some_and(|prev_diff| prev_diff.entity_id() == diff.entity_id())
+ && match reason {
+ RefreshReason::DiffChanged | RefreshReason::EditorSaved => {
+ buffer.read(cx).is_dirty()
+ }
+ RefreshReason::StatusesChanged => false,
+ };
+ if !skip {
+ this.register_buffer(
+ path_key,
+ entry.file_status,
+ buffer,
+ diff,
+ window,
+ cx,
+ )
+ }
})
.ok();
})?;
@@ -637,7 +693,7 @@ impl ProjectDiff {
pub fn excerpt_paths(&self, cx: &App) -> Vec<std::sync::Arc<util::rel_path::RelPath>> {
self.multibuffer
.read(cx)
- .excerpt_paths()
+ .paths()
.map(|key| key.path.clone())
.collect()
}
@@ -1650,9 +1706,13 @@ mod tests {
.unindent(),
);
- editor.update_in(cx, |editor, window, cx| {
- editor.git_restore(&Default::default(), window, cx);
- });
+ editor
+ .update_in(cx, |editor, window, cx| {
+ editor.git_restore(&Default::default(), window, cx);
+ editor.save(SaveOptions::default(), project.clone(), window, cx)
+ })
+ .await
+ .unwrap();
cx.run_until_parked();
assert_state_with_diff(&editor, cx, &"Λ".unindent());
@@ -1841,8 +1901,8 @@ mod tests {
cx,
&"
- original
- + Λdifferent
- "
+ + different
+ Λ"
.unindent(),
);
}
@@ -43,8 +43,8 @@ impl PathKey {
}
impl MultiBuffer {
- pub fn paths(&self) -> impl Iterator<Item = PathKey> + '_ {
- self.excerpts_by_path.keys().cloned()
+ pub fn paths(&self) -> impl Iterator<Item = &PathKey> + '_ {
+ self.excerpts_by_path.keys()
}
pub fn remove_excerpts_for_path(&mut self, path: PathKey, cx: &mut Context<Self>) {
@@ -58,15 +58,18 @@ impl MultiBuffer {
}
}
- pub fn location_for_path(&self, path: &PathKey, cx: &App) -> Option<Anchor> {
+ pub fn buffer_for_path(&self, path: &PathKey, cx: &App) -> Option<Entity<Buffer>> {
let excerpt_id = self.excerpts_by_path.get(path)?.first()?;
let snapshot = self.read(cx);
let excerpt = snapshot.excerpt(*excerpt_id)?;
- Some(Anchor::in_buffer(excerpt.id, excerpt.range.context.start))
+ self.buffer(excerpt.buffer_id)
}
- pub fn excerpt_paths(&self) -> impl Iterator<Item = &PathKey> {
- self.excerpts_by_path.keys()
+ pub fn location_for_path(&self, path: &PathKey, cx: &App) -> Option<Anchor> {
+ let excerpt_id = self.excerpts_by_path.get(path)?.first()?;
+ let snapshot = self.read(cx);
+ let excerpt = snapshot.excerpt(*excerpt_id)?;
+ Some(Anchor::in_buffer(excerpt.id, excerpt.range.context.start))
}
/// Sets excerpts, returns `true` if at least one new excerpt was added.