feat: follow forward working-checkout changes in diffs

This commit is contained in:
Valdemar Lorenzen
2026-10-13 13:38:46 +02:00
parent b16232aa1d
commit bfdb7c544f
12 changed files with 1118 additions and 62 deletions
+209 -3
View File
@@ -122,6 +122,10 @@ pub enum Operation {
mode: ComparisonMode,
buffers: Box<[(PathBuf, String)]>,
},
RefreshWorking {
comparison_id: u64,
buffers: Box<[(PathBuf, String)]>,
},
LoadFile {
comparison_id: u64,
file_index: usize,
@@ -131,6 +135,7 @@ pub enum Operation {
pub enum Outcome {
Catalog(Catalog),
Comparison(Comparison),
WorkingRefresh(Option<Comparison>),
File {
comparison_id: u64,
file_index: usize,
@@ -190,6 +195,9 @@ pub enum DisplayRow {
enum RequestKind {
Catalog,
Comparison,
WorkingRefresh {
comparison_id: u64,
},
File {
comparison_id: u64,
file_index: usize,
@@ -297,6 +305,9 @@ impl DiffState {
}
pub(crate) fn take_request(&mut self) -> Option<Operation> {
if !self.request_ready() {
return None;
}
self.pending.take()
}
@@ -304,7 +315,49 @@ impl DiffState {
self.pending.is_some()
}
pub(crate) fn request_ready(&self) -> bool {
self.pending.as_ref().is_some_and(|operation| {
!self.refresh_pending() || matches!(operation, Operation::RefreshWorking { .. })
})
}
pub(crate) fn refresh_pending(&self) -> bool {
matches!(self.request_kind, Some(RequestKind::WorkingRefresh { .. }))
}
pub(crate) fn queue_working_refresh(&mut self) {
if self.phase != Phase::View
|| self.loading
|| self.request_kind.is_some()
|| self.pending.is_some()
|| self.comparison.is_none()
|| !self
.after
.as_ref()
.is_some_and(|after| after.target == RevisionTarget::Working)
{
return;
}
if let Some(comparison_id) = self.comparison_id {
self.request_kind = Some(RequestKind::WorkingRefresh { comparison_id });
self.pending = Some(Operation::RefreshWorking {
comparison_id,
buffers: Box::new([]),
});
}
}
fn queue(&mut self, operation: Operation) {
if self.refresh_pending()
&& !matches!(self.pending, Some(Operation::RefreshWorking { .. }))
&& matches!(operation, Operation::LoadFile { .. })
{
// Finish the refresh before issuing a load against its frozen comparison ID.
self.pending = Some(operation);
self.loading = true;
self.error = None;
return;
}
self.request_kind = Some(match operation {
Operation::Compare { .. } => RequestKind::Comparison,
Operation::LoadFile {
@@ -321,9 +374,66 @@ impl DiffState {
self.error = None;
}
pub(crate) fn apply_result(&mut self, id: u64, result: Result<Outcome, String>) {
pub(crate) fn apply_result(&mut self, id: u64, result: Result<Outcome, String>) -> bool {
if self.request_kind.is_none() {
return;
return false;
}
if let Some(RequestKind::WorkingRefresh { comparison_id }) = self.request_kind {
if self.comparison_id != Some(comparison_id) {
return false;
}
self.request_kind = None;
let deferred = self.pending.take();
match result {
Ok(Outcome::WorkingRefresh(None)) => {
if let Some(operation) = deferred {
self.queue(operation);
}
return false;
}
Ok(Outcome::WorkingRefresh(Some(comparison))) => {
let selected = self
.comparison
.as_ref()
.and_then(|comparison| comparison.files.get(self.file))
.map(|file| &file.path);
self.file = selected
.and_then(|path| {
comparison.files.iter().position(|file| &file.path == path)
})
.unwrap_or(0);
if let Some(files) = &mut self.files {
files.replace_paths(
comparison
.files
.iter()
.enumerate()
.map(|(index, file)| (index, file.path.clone())),
);
}
self.comparison = Some(comparison);
self.comparison_id = Some(id);
self.snapshot_roots = [None, None];
self.reviewed_files.clear();
self.live_files.clear();
self.live_buffer = None;
self.live_revision = None;
self.follow_caret = false;
self.prepare_selected_file();
self.normalize_focus();
}
Err(error) => {
if let Some(operation) = deferred {
self.queue(operation);
}
self.error = Some(error);
}
Ok(_) => self.error = Some("Unexpected repository response".into()),
}
return true;
}
if matches!(result, Ok(Outcome::WorkingRefresh(_))) {
return false;
}
if let Ok(Outcome::File {
comparison_id,
@@ -337,7 +447,7 @@ impl DiffState {
file_index: *file_index,
}))
{
return;
return false;
}
let expected = self.request_kind.take();
self.loading = false;
@@ -436,6 +546,7 @@ impl DiffState {
Ok(_) => self.error = Some("Unexpected repository response".into()),
Err(error) => self.error = Some(error),
}
true
}
fn refilter(&mut self) {
@@ -817,6 +928,15 @@ impl DiffState {
self.focus = match (self.focus, self.pane_layout()) {
(Focus::Files, _) if self.files_visible => Focus::Files,
(Focus::Before | Focus::After, PaneLayout::Split) => self.focus,
(Focus::Before | Focus::After, PaneLayout::Empty)
if self
.comparison
.as_ref()
.is_some_and(|comparison| !comparison.files.is_empty())
&& !self.selected_file_loaded() =>
{
self.focus
}
_ => self.first_text_focus(),
};
if let Some(files) = &mut self.files {
@@ -956,6 +1076,16 @@ impl DiffState {
}
fn prepare_selected_file(&mut self) {
if self.refresh_pending() {
self.align_file();
if !self.selected_file_loaded() {
self.load_file(self.file);
} else if matches!(self.pending, Some(Operation::LoadFile { .. })) {
self.pending = None;
self.loading = false;
}
return;
}
self.pending = None;
self.request_kind = None;
self.loading = false;
@@ -3148,6 +3278,82 @@ mod tests {
state
}
#[test]
fn working_refresh_preserves_noops_errors_and_view_state_but_invalidates_old_identity() {
let mut state = live_state(vec!["old".into()], vec!["new".into()], false);
state.loading = false;
state.pending = None;
state.request_kind = None;
state.comparison_id = Some(7);
state.word_wrap = true;
state.view_mode = ViewMode::Unified;
state.comparison_mode = ComparisonMode::Snapshots;
state.split_axis = crate::panes::SplitAxis::Horizontal;
state.files_visible = false;
state.reviewed_files.insert(0);
state.error = Some("previous notice".into());
let files = state.files.as_mut().unwrap();
files.set_view(FileView::Tree);
files.start_search();
files.query = "file".into();
files.refilter();
let original = format!("{state:?}");
state.queue_working_refresh();
assert!(matches!(
state.take_request(),
Some(Operation::RefreshWorking {
comparison_id: 7,
..
})
));
assert!(!state.apply_result(8, Ok(Outcome::WorkingRefresh(None))));
assert_eq!(format!("{state:?}"), original);
state.queue_working_refresh();
state.take_request();
assert!(state.apply_result(9, Err("refresh failed".into())));
assert_eq!(state.comparison_id, Some(7));
assert_eq!(state.comparison.as_ref().unwrap().files[0].after, ["new"]);
assert_eq!(state.reviewed_files, BTreeSet::from([0]));
assert_eq!(state.error.as_deref(), Some("refresh failed"));
state.queue_working_refresh();
state.take_request();
let mut replacement = live_state(vec!["old".into()], vec!["ahead".into()], false)
.comparison
.take()
.unwrap();
replacement.files.insert(0, comparison().files.remove(0));
assert!(state.apply_result(10, Ok(Outcome::WorkingRefresh(Some(replacement)))));
assert_eq!(state.comparison_id, Some(10));
assert_eq!(state.file, 1);
assert_eq!(state.focus, Focus::After);
assert!(state.word_wrap && !state.files_visible);
assert_eq!(state.split_axis, crate::panes::SplitAxis::Horizontal);
assert_eq!(state.view_mode, ViewMode::Unified);
assert_eq!(state.comparison_mode, ComparisonMode::Snapshots);
assert!(state.reviewed_files.is_empty());
let files = state.files.as_ref().unwrap();
assert_eq!(files.query, "file");
assert_eq!(files.view, FileView::Tree);
assert!(files.searching);
assert_eq!(
files.entries().nth(files.selected).unwrap().path,
Path::new("file")
);
let refreshed = format!("{state:?}");
assert!(!state.apply_result(7, Ok(Outcome::WorkingRefresh(None))));
assert_eq!(format!("{state:?}"), refreshed);
state.after.as_mut().unwrap().target = RevisionTarget::Commit("historical".into());
state.queue_working_refresh();
assert!(!state.request_queued());
state.after.as_mut().unwrap().target = RevisionTarget::Working;
state.load_file(0);
let user_request = format!("{state:?}");
state.queue_working_refresh();
assert_eq!(format!("{state:?}"), user_request);
}
#[test]
fn reviewed_files_use_highlighted_catalog_indices_not_sorted_or_filtered_rows() {
let mut state = DiffState::new();
+262 -12
View File
@@ -2542,8 +2542,13 @@ impl Editor {
self.diff.as_ref().or(self.parked_diff.as_ref())
}
pub(crate) fn diff_root(&mut self) -> PathBuf {
let owner = self.diff_owner.unwrap_or(self.pane_id);
pub(crate) fn diff_root(&mut self, tab: usize) -> PathBuf {
let owner = if tab == self.tab_id {
self.diff_owner.unwrap_or(self.pane_id)
} else {
let tab = &self.tabs[&tab];
tab.diff_owner.unwrap_or(tab.pane_id)
};
self.with_pane(owner, |editor| {
if let Some(root) = &editor.active.snapshot_root {
return root.clone();
@@ -2575,15 +2580,60 @@ impl Editor {
self.diff_display_options = (view_mode, split_axis);
}
pub(crate) fn queue_working_refreshes(&mut self) {
for index in 0..self.tab_order.len() {
let tab = self.tab_order[index];
let owner = if tab == self.tab_id {
self.diff_owner
} else {
self.tabs.get(&tab).and_then(|tab| tab.diff_owner)
};
if let Some(owner) = owner {
self.with_pane(owner, |editor| {
if let Some(diff) = &mut editor.diff {
diff.queue_working_refresh();
}
});
}
}
}
pub(crate) fn take_next_diff_request(&mut self) -> Option<(usize, u64, Operation)> {
let tab = std::iter::once(self.tab_id)
.chain(
self.tab_order
.iter()
.copied()
.filter(|tab| *tab != self.tab_id),
)
.find(|tab| self.tab_diff(*tab).is_some_and(DiffState::request_ready))?;
let owner = if tab == self.tab_id {
self.diff_owner
} else {
self.tabs.get(&tab)?.diff_owner
}?;
self.with_pane(owner, |editor| {
let (id, operation) = editor.take_diff_request()?;
Some((tab, id, operation))
})
}
pub(crate) fn take_diff_request(&mut self) -> Option<(u64, Operation)> {
let mut operation = self
.diff
.as_mut()
.or(self.parked_diff.as_mut())?
.take_request()?;
if let Operation::Compare { after, buffers, .. } = &mut operation
&& after.target == RevisionTarget::Working
{
let buffers = match &mut operation {
Operation::Compare { after, buffers, .. }
if after.target == RevisionTarget::Working =>
{
Some(buffers)
}
Operation::RefreshWorking { buffers, .. } => Some(buffers),
_ => None,
};
if let Some(buffers) = buffers {
*buffers = std::iter::once(&self.active)
.chain(self.parked.values())
.filter(|buffer| buffer.dirty())
@@ -2639,15 +2689,32 @@ impl Editor {
let Some(diff) = &mut self.diff else {
return false;
};
if diff.request_queued() {
if diff.request_queued() && !diff.refresh_pending() {
return false;
}
diff.apply_result(id, result);
if let Some(files) = &mut diff.files {
let refresh = diff.refresh_pending();
let previous = diff.comparison_id;
if !diff.apply_result(id, result) {
return false;
}
let replaced = previous != diff.comparison_id;
if refresh {
if !replaced {
return true;
}
self.pending_lsp_navigation = None;
self.diff_bound = None;
} else if let Some(files) = &mut diff.files {
files.set_view(self.file_explorer_view);
}
let obsolete_snapshot = self.active.diff_snapshot.is_some_and(|role| {
self.diff.as_ref().and_then(|diff| diff.comparison_id) != Some(role.0)
});
self.bind_diff_buffer();
self.refresh_live_diff();
if refresh || obsolete_snapshot {
self.prune_diff_snapshots();
}
let completed_navigation = self
.pending_lsp_navigation
.as_ref()
@@ -2792,6 +2859,17 @@ impl Editor {
.is_some_and(|bound| bound.3 != self.active.id)
{
if let Some(role) = self.active.diff_snapshot {
if self
.diff
.as_ref()
.is_none_or(|diff| diff.comparison_id != Some(role.0))
{
self.diff_bound = None;
self.diff = None;
self.diff_owner = None;
self.outstanding_diff_request = None;
return;
}
self.diff_bound = Some((role.0, role.1, role.2, self.active.id));
if let Some(diff) = &mut self.diff {
if diff.file != role.1
@@ -2821,9 +2899,18 @@ impl Editor {
{
self.activate(buffer);
}
self.prune_diff_snapshots();
self.diff_bound = None;
self.diff_return_buffer = None;
}
fn prune_diff_snapshots(&mut self) {
self.parked.retain(|id, buffer| {
buffer.diff_snapshot.is_none()
|| self.panes.values().any(|pane| pane.buffer_id == *id)
|| self
.panes
.iter()
.any(|(pane_id, pane)| *pane_id != self.pane_id && pane.buffer_id == *id)
|| buffer.diff_snapshot.is_some_and(|role| {
self.tabs.values().any(|tab| {
tab.diff()
@@ -2835,8 +2922,6 @@ impl Editor {
pane.views
.retain(|id, _| *id == self.active.id || self.parked.contains_key(id));
}
self.diff_bound = None;
self.diff_return_buffer = None;
}
fn bind_diff_buffer(&mut self) {
@@ -12638,6 +12723,171 @@ mod tests {
assert!(editor.diff_interacting());
}
#[test]
fn working_refresh_routes_hidden_owners_and_keeps_obsolete_shared_snapshots_readonly() {
let directory = Directory::new();
fs::write(directory.path("file.txt"), "working").unwrap();
let mut editor = editor();
working_diff(&mut editor, &directory.0, &["file.txt"]);
editor.diff.as_mut().unwrap().focus = Focus::Before;
editor.bind_diff_buffer();
let old_snapshot = editor.buffer_id();
let old_comparison = editor.diff().unwrap().comparison_id.unwrap();
let owner = editor.pane_id();
let hidden_tab = editor.tab_id();
editor.execute("tab.new");
let visible_tab = editor.tab_id();
let visible_pane = editor.pane_id();
let cursor = editor.cursor();
editor.queue_working_refreshes();
let (tab, request, operation) = editor.take_next_diff_request().unwrap();
assert_eq!(tab, hidden_tab);
assert!(
matches!(operation, Operation::RefreshWorking { comparison_id, .. }
if comparison_id == old_comparison)
);
assert!(!editor.apply_diff_result(request, Ok(Outcome::WorkingRefresh(None))));
assert_eq!(
editor.tab_diff(hidden_tab).unwrap().comparison_id,
Some(old_comparison)
);
assert_eq!(editor.tab_id(), visible_tab);
assert_eq!(editor.pane_id(), visible_pane);
assert_eq!(editor.buffer_id(), old_snapshot);
assert_eq!(editor.cursor(), cursor);
editor.queue_working_refreshes();
let (_, request, _) = editor.take_next_diff_request().unwrap();
let replacement = editor.with_pane(owner, |editor| {
editor.diff.as_mut().unwrap().comparison.take().unwrap()
});
assert!(editor.apply_diff_result(request, Ok(Outcome::WorkingRefresh(Some(replacement)))));
assert_eq!(
editor.tab_diff(hidden_tab).unwrap().comparison_id,
Some(request)
);
assert_eq!(editor.tab_id(), visible_tab);
assert_eq!(editor.buffer_id(), old_snapshot);
assert_eq!(editor.cursor(), cursor);
assert!(editor.active.diff_snapshot.is_some());
assert!(
editor
.apply(vec![Effect::Insert("rejected".into())])
.is_err()
);
assert!(!editor.apply_diff_result(request, Ok(Outcome::WorkingRefresh(None))));
editor.with_pane(owner, |editor| {
assert_ne!(editor.buffer_id(), old_snapshot);
editor.switch_buffer(old_snapshot);
editor.leave_diff_on_navigation();
assert!(editor.diff().is_none());
assert!(editor.active.diff_snapshot.is_some());
assert!(editor.mutation_blocked());
});
}
#[test]
fn completed_working_refresh_serializes_lazy_selection_and_invalidates_pending_navigation() {
for request_kind in [
"selection",
"after_selection",
"navigation",
"noop_navigation",
] {
let directory = Directory::new();
fs::write(directory.path("file.txt"), "working").unwrap();
let mut editor = editor();
working_diff(&mut editor, &directory.0, &["file.txt"]);
editor.diff.as_mut().unwrap().focus = Focus::Before;
editor.bind_diff_buffer();
let old_snapshot = editor.buffer_id();
let mut target = lazy_lsp_target(&mut editor);
let old_comparison = editor.diff().unwrap().comparison_id.unwrap();
let virtual_root = editor.diff().unwrap().snapshot_roots[0].clone().unwrap();
editor.queue_working_refreshes();
let (_, refresh, _) = editor.take_next_diff_request().unwrap();
let loaded_before = target.before.clone();
let loaded_after = target.after.clone();
target.loaded = false;
let replacement = crate::diff::Comparison {
root: directory.0.clone(),
before: "base".into(),
after: "ahead".into(),
editable_paths: Default::default(),
files: vec![target],
};
let selection = request_kind.ends_with("selection");
let focus = if request_kind == "after_selection" {
Focus::After
} else {
Focus::Before
};
if selection {
assert!(editor.diff.as_mut().unwrap().select_file_index(1));
editor.diff.as_mut().unwrap().focus = focus;
} else {
editor
.navigate_lsp_to(virtual_root.join("nested/old.txt"), lsp_point(1, 4), None)
.unwrap();
assert!(editor.pending_lsp_navigation.is_some());
assert_eq!(editor.diff().unwrap().file, 0);
}
assert!(editor.take_next_diff_request().is_none());
let changed = request_kind != "noop_navigation";
let outcome = if changed { Some(replacement) } else { None };
assert_eq!(
editor.apply_diff_result(refresh, Ok(Outcome::WorkingRefresh(outcome))),
changed
);
if changed {
assert!(editor.pending_lsp_navigation.is_none());
assert_eq!(editor.diff().unwrap().comparison_id, Some(refresh));
assert_eq!(editor.diff().unwrap().file, 0);
assert_eq!(editor.diff().unwrap().focus, focus);
} else {
assert!(editor.pending_lsp_navigation.is_some());
assert_eq!(editor.diff().unwrap().comparison_id, Some(old_comparison));
}
let (_, load, operation) = editor.take_next_diff_request().unwrap();
assert!(
matches!(operation, Operation::LoadFile { comparison_id, file_index }
if comparison_id == if changed { refresh } else { old_comparison }
&& file_index == if changed { 0 } else { 1 })
);
if !changed {
assert_eq!(
editor.pending_lsp_navigation.as_ref().unwrap().request,
Some(load)
);
}
if changed {
let file = crate::diff::ChangedFile {
path: "nested/new.txt".into(),
old_path: Some("nested/old.txt".into()),
status: crate::diff::FileStatus::Renamed,
loaded: true,
hunks: crate::diff::line_hunks(&loaded_before, false, &loaded_after, false),
before: loaded_before,
after: loaded_after,
before_eof: false,
after_eof: false,
notice: None,
};
assert!(editor.apply_diff_result(
load,
Ok(Outcome::File {
comparison_id: refresh,
file_index: 0,
file,
})
));
assert_eq!(editor.diff().unwrap().focus, focus);
assert!(!editor.parked.contains_key(&old_snapshot));
}
}
}
fn lsp_point(row: usize, column: usize) -> Utf16Range {
Utf16Range {
start: (row, column),
@@ -13010,7 +13260,7 @@ mod tests {
editor.interaction_document_root(editor.buffer_id()),
directory.0.as_path()
);
assert_eq!(editor.diff_root(), directory.0);
assert_eq!(editor.diff_root(editor.tab_id()), directory.0);
editor.handle(Input::Char('\t'));
assert_eq!(editor.buffer_id(), live_id);
assert!(editor.diff_editing());
+1
View File
@@ -73,6 +73,7 @@ Diff: :diff (Normal); choose Before then After; Working checkout After edits liv
Normal [/] explicitly expands context; Insert brackets remain literal text
Normal Space v cycles split/unified; Space s flips left/right vs above/below split
Defaults: config.diff_view_mode = \"split\"; config.diff_split_orientation = \"vertical\"
Working After follows descendant checkouts; config.diff_follow_working_copy = false disables it
Esc closes Before/After selectors; :diff.close exits an opened comparison explicitly
Remote refs/history load into temporary comparison repos without changing active refs/checkout
Changed files: normal picker/explorer bindings, Ctrl+w close; A/M/D/R status and rename paths
+136 -18
View File
@@ -307,6 +307,10 @@ struct PreparedComparison {
left: Snapshot,
right: Snapshot,
files: Vec<(PathBuf, Option<PathBuf>, FileStatus)>,
before: String,
after: String,
working_after: bool,
working_anchor: Option<String>,
}
pub struct Repository {
@@ -1331,6 +1335,53 @@ impl Repository {
}
Ok(base.into())
}
fn working_anchor(&self) -> Result<Option<String>, String> {
if self.kind == RepoKind::Jujutsu {
return Ok(Some(
text(self.jj(
&self.root,
&["log", "--no-graph", "-r", "@", "-T", "change_id"],
)?)?
.trim()
.into(),
));
}
let mut command = git_command(&self.store);
command.args(["rev-parse", "--verify", "--quiet", "HEAD^{commit}"]);
let output = run(command, vec![], &self.cancelled)?;
if output.code == Some(1) {
return Ok(None); // An unborn checkout has no commit anchor yet.
}
Ok(Some(text(checked(output)?)?.trim().into()))
}
fn working_advanced(&self, before: Option<&str>, after: Option<&str>) -> Result<bool, String> {
if before == after || after.is_none() {
return Ok(false);
}
let Some(before) = before else {
return Ok(true);
};
if self.kind == RepoKind::Jujutsu {
let revset = format!(
"ancestors(@) & change_id({})",
serde_json::to_string(before).unwrap()
);
return Ok(!self
.jj(
&self.root,
&["log", "--no-graph", "-r", &revset, "-T", "change_id"],
)?
.is_empty());
}
let mut command = git_command(&self.store);
command.args(["merge-base", "--is-ancestor", before, after.unwrap()]);
let output = run(command, vec![], &self.cancelled)?;
if output.code == Some(1) {
Ok(false)
} else {
checked(output).map(|_| true)
}
}
/// Freeze comparison sources and return path/status metadata only.
/// A new comparison replaces sources retained for the previous request ID.
pub fn compare(
@@ -1341,7 +1392,7 @@ impl Repository {
mode: ComparisonMode,
buffers: &[(PathBuf, String)],
) -> Result<Comparison, String> {
self.comparison = None;
self.check_cancelled()?;
let resolved = if mode == ComparisonMode::BranchChanges
&& before.target != RevisionTarget::Working
&& after.target != RevisionTarget::Working
@@ -1365,26 +1416,98 @@ impl Repository {
(&before.target, &after.target, before.label.as_str()),
|(before, after, label)| (before, after, label.as_str()),
);
let mut left = self.snapshot(before_target)?;
let working_after = *after_target == RevisionTarget::Working;
let working_anchor = if working_after {
self.working_anchor()?
} else {
None
};
let left = self.snapshot(before_target)?;
let same = before_target == after_target && (!working_after || buffers.is_empty());
let mut right = if same {
let right = if same {
BTreeMap::new()
} else if working_after {
self.working_snapshot(buffers)?
} else {
self.snapshot(after_target)?
};
let mut prepared = PreparedComparison {
id: comparison_id,
left,
right,
files: vec![],
before: before_label.into(),
after: after.label.clone(),
working_after,
working_anchor,
};
let comparison = self.build_comparison(&mut prepared, same)?;
if working_after && self.working_anchor()? != prepared.working_anchor {
return Err("Working checkout moved during comparison".into());
}
self.comparison = Some(prepared);
Ok(comparison)
}
/// Advance only a selected Working After, keeping its original Before frozen.
pub fn refresh_working_after(
&mut self,
comparison_id: u64,
new_id: u64,
buffers: &[(PathBuf, String)],
) -> Result<Option<Comparison>, String> {
let Some(prepared) = self
.comparison
.as_ref()
.filter(|prepared| prepared.id == comparison_id && prepared.working_after)
else {
return Ok(None);
};
let anchor = self.working_anchor()?;
if !self.working_advanced(prepared.working_anchor.as_deref(), anchor.as_deref())? {
return Ok(None);
}
let right = self.working_snapshot(buffers)?;
let mut prepared = self.comparison.take().unwrap();
let previous_right = std::mem::replace(&mut prepared.right, right);
let previous_files = std::mem::take(&mut prepared.files);
let result = self
.build_comparison(&mut prepared, false)
.and_then(|comparison| {
self.check_cancelled()?;
if self.working_anchor()? != anchor {
return Err("Working checkout moved during comparison".into());
}
Ok(comparison)
});
if result.is_ok() {
prepared.id = new_id;
prepared.working_anchor = anchor;
} else {
prepared.right = previous_right;
prepared.files = previous_files;
}
self.comparison = Some(prepared);
result.map(Some)
}
fn build_comparison(
&self,
prepared: &mut PreparedComparison,
same: bool,
) -> Result<Comparison, String> {
let left = &mut prepared.left;
let right = &mut prepared.right;
// Frozen checkout/conflict bytes have no object identity. Compare them in a
// single batch, while immutable tree/tree comparisons need only object IDs.
let frozen = |file: &FileData| {
file.object.is_none() && matches!(file.mode.as_str(), "100644" | "100755" | "120000")
};
if !same && (left.values().any(frozen) || right.values().any(frozen)) {
self.materialize(&mut left)?;
self.materialize(&mut right)?;
self.materialize(left)?;
self.materialize(right)?;
}
let right_view = if same { &left } else { &right };
let right_view = if same { &*left } else { &*right };
let mut files = vec![];
let paths: BTreeSet<_> = left.keys().chain(right_view.keys()).collect();
for path in paths {
@@ -1442,7 +1565,7 @@ impl Repository {
.enumerate()
.filter_map(|(i, f)| (!removed.contains(&i)).then_some(f))
.collect();
let editable_paths = if working_after {
let editable_paths = if prepared.working_after {
files
.iter()
.filter(|file| {
@@ -1455,18 +1578,13 @@ impl Repository {
} else {
BTreeSet::new()
};
self.comparison = Some(PreparedComparison {
id: comparison_id,
left,
right,
files: files
.iter()
.map(|file| (file.path.clone(), file.old_path.clone(), file.status))
.collect(),
});
prepared.files = files
.iter()
.map(|file| (file.path.clone(), file.old_path.clone(), file.status))
.collect();
Ok(Comparison {
before: before_label.into(),
after: after.label.clone(),
before: prepared.before.clone(),
after: prepared.after.clone(),
files,
root: self.root.clone(),
editable_paths,
+30 -6
View File
@@ -47,6 +47,7 @@ pub struct Config {
pub diff_expand_lines: usize,
pub diff_view_mode: crate::diff::ViewMode,
pub diff_split_orientation: crate::panes::SplitAxis,
pub diff_follow_working_copy: bool,
}
#[derive(Debug, Deserialize)]
@@ -90,6 +91,7 @@ impl Default for Config {
diff_expand_lines: 15,
diff_view_mode: crate::diff::ViewMode::Split,
diff_split_orientation: crate::panes::SplitAxis::Vertical,
diff_follow_working_copy: true,
}
}
}
@@ -120,6 +122,7 @@ pub fn load_config(path: &Path) -> Result<Config, String> {
diff_expand_lines,
diff_view_mode,
diff_split_orientation,
diff_follow_working_copy,
..
} = *response;
let mut config = Config::default();
@@ -151,6 +154,7 @@ pub fn load_config(path: &Path) -> Result<Config, String> {
config.diff_expand_lines = diff_expand_lines;
config.diff_view_mode = diff_view_mode;
config.diff_split_orientation = diff_split_orientation;
config.diff_follow_working_copy = diff_follow_working_copy;
Ok(config)
}
Response::Error {
@@ -471,6 +475,7 @@ struct ConfigFrame {
diff_expand_lines: usize,
diff_view_mode: crate::diff::ViewMode,
diff_split_orientation: crate::panes::SplitAxis,
diff_follow_working_copy: bool,
}
#[derive(Deserialize)]
@@ -1211,9 +1216,9 @@ end
assert!(load_config(&path).unwrap_err().contains(error), "{value}");
}
for frame in [
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical"}"#,
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":null,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical"}"#,
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":7,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical"}"#,
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#,
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":null,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#,
r#"{"type":"config","version":1,"plugins":[],"lsp":{},"bindings":[],"leader":7,"file_explorer_side":"right","file_picker_view":"list","file_explorer_view":"tree","clipboard_mode":"system","git_blame":true,"top_bar":{},"bottom_bar":{},"popup":{"command":{},"search":{}},"labels":{},"diff_context_lines":10,"diff_expand_lines":15,"diff_view_mode":"split","diff_split_orientation":"vertical","diff_follow_working_copy":true}"#,
] {
assert!(serde_json::from_str::<Response>(frame).is_err(), "{frame}");
}
@@ -1263,7 +1268,8 @@ end
"file_explorer_view": "tree", "clipboard_mode": "system", "git_blame": true,
"top_bar": {}, "bottom_bar": {}, "popup": {"command": {}, "search": {}},
"labels": {}, "diff_context_lines": 10, "diff_expand_lines": 15,
"diff_view_mode": "split", "diff_split_orientation": "vertical"
"diff_view_mode": "split", "diff_split_orientation": "vertical",
"diff_follow_working_copy": true
});
assert!(serde_json::from_value::<Response>(frame.clone()).is_ok());
for value in [
@@ -1278,6 +1284,22 @@ end
assert!(serde_json::from_value::<Response>(frame).is_err());
}
#[test]
fn working_diff_tracking_rejects_non_boolean_configuration() {
let fixture = Fixture::new();
for value in ["nil", "0", r#""false""#, "[]", "{}"] {
let path = fixture.file(
"config.rb",
&format!("Vamp.configure {{ |config| config.diff_follow_working_copy = {value} }}"),
);
assert!(
load_config(&path)
.unwrap_err()
.contains("diff_follow_working_copy must be true or false")
);
}
}
#[test]
fn chrome_configuration_normalizes_ruby_values_and_retains_unspecified_defaults() {
let fixture = Fixture::new();
@@ -1496,7 +1518,8 @@ end
"top_bar": {}, "bottom_bar": {}, "labels": {},
"popup": {"command": {}, "search": {}},
"clipboard_mode": "system", "diff_context_lines": 0, "diff_expand_lines": 7,
"diff_view_mode": "split", "diff_split_orientation": "vertical"
"diff_view_mode": "split", "diff_split_orientation": "vertical",
"diff_follow_working_copy": true
});
for setting in ["diff_context_lines", "diff_expand_lines"] {
for value in [
@@ -1912,7 +1935,8 @@ end"#,
"top_bar": {}, "bottom_bar": {}, "labels": {},
"popup": {"command": {}, "search": {}},
"clipboard_mode": "system", "diff_context_lines": 10, "diff_expand_lines": 15,
"diff_view_mode": "split", "diff_split_orientation": "vertical"
"diff_view_mode": "split", "diff_split_orientation": "vertical",
"diff_follow_working_copy": true
});
let Response::Config(config) = serde_json::from_value::<Response>(frame.clone()).unwrap()
else {
+11 -1
View File
@@ -144,6 +144,7 @@ module Vamp
@diff_expand_lines = 15
@diff_view_mode = "split"
@diff_split_orientation = "vertical"
@diff_follow_working_copy = true
end
# @param path [String] relative to the configuration file
@@ -234,6 +235,14 @@ module Vamp
@diff_split_orientation = orientation
end
# @param enabled [Boolean]
# @return [Boolean]
def diff_follow_working_copy=(enabled)
raise TypeError, "diff_follow_working_copy must be true or false" unless [true, false].include?(enabled)
@diff_follow_working_copy = enabled
end
# @return [Module<Vamp::Core>] frozen namespace with catalog-generated Action methods
def core
Core
@@ -269,7 +278,8 @@ module Vamp
"top_bar" => @bar.top, "bottom_bar" => @bar.bottom, "labels" => @labels,
"popup" => { "command" => @popup.command, "search" => @popup.search },
"diff_context_lines" => @diff_context_lines, "diff_expand_lines" => @diff_expand_lines,
"diff_view_mode" => @diff_view_mode, "diff_split_orientation" => @diff_split_orientation }
"diff_view_mode" => @diff_view_mode, "diff_split_orientation" => @diff_split_orientation,
"diff_follow_working_copy" => @diff_follow_working_copy }
end
private
+113 -21
View File
@@ -84,6 +84,12 @@ impl DiffWorker {
} => repository
.compare(id, &before, &after, mode, &buffers)
.map(DiffOutcome::Comparison),
DiffOperation::RefreshWorking {
comparison_id,
buffers,
} => repository
.refresh_working_after(comparison_id, id, &buffers)
.map(DiffOutcome::WorkingRefresh),
DiffOperation::LoadFile {
comparison_id,
file_index,
@@ -181,6 +187,8 @@ pub struct EditorSession {
battery: Option<crate::battery::Battery>,
battery_enabled: bool,
last_file_refresh: Instant,
last_diff_refresh: Instant,
pub(crate) diff_follow_working_copy: bool,
}
impl EditorSession {
@@ -287,6 +295,8 @@ impl EditorSession {
battery: None,
battery_enabled: false,
last_file_refresh: Instant::now(),
last_diff_refresh: Instant::now(),
diff_follow_working_copy: config.diff_follow_working_copy,
};
session.synchronize_documents();
session.poll_battery();
@@ -352,6 +362,7 @@ impl EditorSession {
return false;
};
if !diff.loading
&& !diff.refresh_pending()
&& let Some(id) = worker.pending.take()
{
worker.cancelled_request.store(id, Ordering::Release);
@@ -359,29 +370,28 @@ impl EditorSession {
}
true
});
let Some((id, operation)) = self.editor.take_diff_request() else {
return;
};
let tab = self.editor.tab_id();
if let std::collections::btree_map::Entry::Vacant(entry) = self.diff_workers.entry(tab) {
match DiffWorker::start(self.editor.diff_root()) {
Ok(worker) => {
entry.insert(worker);
}
Err(error) => {
self.editor.apply_diff_result(id, Err(error));
return;
while let Some((tab, id, operation)) = self.editor.take_next_diff_request() {
if let std::collections::btree_map::Entry::Vacant(entry) = self.diff_workers.entry(tab)
{
match DiffWorker::start(self.editor.diff_root(tab)) {
Ok(worker) => {
entry.insert(worker);
}
Err(error) => {
self.editor.apply_diff_result(id, Err(error));
continue;
}
}
}
}
if let Err(error) = self
.diff_workers
.get_mut(&tab)
.expect("diff worker started")
.request(id, operation)
{
self.editor.apply_diff_result(id, Err(error));
self.diff_workers.remove(&tab);
if let Err(error) = self
.diff_workers
.get_mut(&tab)
.expect("diff worker started")
.request(id, operation)
{
self.editor.apply_diff_result(id, Err(error));
self.diff_workers.remove(&tab);
}
}
}
@@ -1141,6 +1151,16 @@ impl EditorSession {
self.editor.refresh_files()
}
fn refresh_working_diffs_at(&mut self, now: Instant) {
if !self.diff_follow_working_copy
|| now.saturating_duration_since(self.last_diff_refresh) < Duration::from_secs(1)
{
return;
}
self.last_diff_refresh = now;
self.editor.queue_working_refreshes();
}
/// Drains ready plugin work without waiting for command or server responses.
pub fn poll(&mut self) -> bool {
self.poll_at(Instant::now())
@@ -1150,6 +1170,7 @@ impl EditorSession {
let mut changed = self.refresh_files_at(now);
changed |= self.synchronize_documents();
changed |= self.poll_battery();
self.refresh_working_diffs_at(now);
changed |= self.poll_diff();
for update in self.lsp.poll() {
match self.apply_update("core", update) {
@@ -1212,6 +1233,77 @@ mod tests {
sync::atomic::{AtomicU64, Ordering},
};
#[test]
fn working_diff_polling_uses_injected_interval_and_never_queues_historical_or_disabled_views() {
use crate::diff::{Catalog, Category, Comparison, RepoKind, Revision, RevisionTarget};
let fixture = Fixture::new();
fixture.file("buffer.txt", "base");
for working in [true, false] {
let mut session = fixture.open("");
session.editor.execute("diff");
let (id, _) = session.editor.take_diff_request().unwrap();
session.editor.apply_diff_result(
id,
Ok(DiffOutcome::Catalog(Catalog {
kind: RepoKind::Git,
notice: None,
entries: [
RevisionTarget::Working,
RevisionTarget::Commit("base".into()),
]
.into_iter()
.map(|target| Revision {
target,
label: "revision".into(),
description: String::new(),
category: Category::Commits,
})
.collect(),
})),
);
session.editor.handle(Input::Down);
session.editor.handle(Input::Enter);
if !working {
session.editor.handle(Input::Down);
}
session.editor.handle(Input::Enter);
let (id, _) = session.editor.take_diff_request().unwrap();
session.editor.apply_diff_result(
id,
Ok(DiffOutcome::Comparison(Comparison {
root: fixture.0.clone(),
before: "base".into(),
after: "after".into(),
files: Vec::new(),
editable_paths: Default::default(),
})),
);
let now = session.last_diff_refresh;
session.diff_follow_working_copy = false;
session.refresh_working_diffs_at(now + Duration::from_secs(1));
assert!(session.editor.take_next_diff_request().is_none());
session.diff_follow_working_copy = true;
session.refresh_working_diffs_at(now + Duration::from_millis(999));
assert!(session.editor.take_next_diff_request().is_none());
session.refresh_working_diffs_at(now + Duration::from_secs(1));
let request = session.editor.take_next_diff_request();
assert_eq!(request.is_some(), working);
if let Some((_, refresh, operation)) = request {
assert!(
matches!(operation, DiffOperation::RefreshWorking { comparison_id, .. }
if comparison_id == id)
);
session.refresh_working_diffs_at(now + Duration::from_secs(2));
assert!(session.editor.take_next_diff_request().is_none());
assert!(
!session
.editor
.apply_diff_result(refresh, Ok(DiffOutcome::WorkingRefresh(None)))
);
}
}
}
#[test]
fn blame_sources_are_deduplicated_and_stale_results_cannot_reappear() {
let fixture = Fixture::new();