feat: default diffs to branch changes

This commit is contained in:
Valdemar Lorenzen
2026-10-13 10:41:37 +02:00
parent fafec2a6b0
commit 6c254f9b0d
10 changed files with 562 additions and 33 deletions
+3
View File
@@ -18,6 +18,9 @@
- Space v cycles split/unified; configurable startup default, dual line numbers, native side focus/editing, and shared highlighting/wrap/excerpts. - Space v cycles split/unified; configurable startup default, dual line numbers, native side focus/editing, and shared highlighting/wrap/excerpts.
- [x] diff view: for split view, change split orientation (left/right vs above/below) - left/right by default. - [x] diff view: for split view, change split orientation (left/right vs above/below) - left/right by default.
- Space s flips orientation; configurable startup default and shared pane geometry/directional focus. - Space s flips orientation; configurable startup default and shared pane geometry/directional focus.
- [ ] add `C` and `D` for deleting line from current cursor position (and entering insert mode: `C`); add `Y` for copying line from current cursor position; add `r` for replacing character at cursor position; add `s` for deleting character at current cursor position and entering insert mode; add `d<count>w` for deleting whole words; add `d<count>b` for deleting backwards (same as `d<count>w` but backwards).
- [ ] diff view: make foreground of etched areas (where no lines exist) the same dimmed color as the normal gutter text
- [ ] copying is pretty slow; optimize performance to reduce latency.
- [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc. - [ ] diff view: github integration (code review) - add comments on a pr, add review comments at specific lines of files, request changes, approve pr, etc.
- [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding. - [ ] diff view: local review comments (same features as for github reviews) that allows review of local changes to deliver specific reviews to ai agents used for coding.
- design question: would this require a server that the agent can interact with, or would local review files be fine? don't want to use server client stuff, if it would be easier and better to just use local files. - design question: would this require a server that the agent can interact with, or would local review files be fine? don't want to use server client stuff, if it would be easier and better to just use local files.
+3 -2
View File
@@ -49,6 +49,7 @@ Plugin paths resolve relative to the config file, not the shell's working direct
`config.diff_view_mode` selects the initial view for each new comparison: `"split"` (default) or `"unified"`. `config.diff_split_orientation` selects `"vertical"` (left/right, default) or `"horizontal"` (above/below) for split views. Invalid values reject the configuration. Normal-mode **Space v** cycles split/unified; **Space s** flips split orientation. These controls affect the current comparison, not startup defaults, and retain their choices across file switches and tabs. Orientation changes apply only in split view. `config.diff_view_mode` selects the initial view for each new comparison: `"split"` (default) or `"unified"`. `config.diff_split_orientation` selects `"vertical"` (left/right, default) or `"horizontal"` (above/below) for split views. Invalid values reject the configuration. Normal-mode **Space v** cycles split/unified; **Space s** flips split orientation. These controls affect the current comparison, not startup defaults, and retain their choices across file switches and tabs. Orientation changes apply only in split view.
```ruby ```ruby
config.bind "ctrl+b", config.core.diff.mode.toggle
config.bind "leader v", config.core.diff.view.toggle config.bind "leader v", config.core.diff.view.toggle
config.bind "leader s", config.core.diff.orientation.toggle config.bind "leader s", config.core.diff.orientation.toggle
``` ```
@@ -160,7 +161,7 @@ Normal `dd` and `cc` delete/change complete rows, with leading counts. Bind `con
| Normal | `save`, `force_save`, `quit`, `force_quit`, `restart`, `force_restart`, `save_and_quit`, `force_save_and_quit`, `plugins` | | Normal | `save`, `force_save`, `quit`, `force_quit`, `restart`, `force_restart`, `save_and_quit`, `force_save_and_quit`, `plugins` |
| Normal | `edit`, `buffers`, `buffer`, `buffer_next`, `buffer_previous`, `buffer_delete`, `buffer_force_delete` | | Normal | `edit`, `buffers`, `buffer`, `buffer_next`, `buffer_previous`, `buffer_delete`, `buffer_force_delete` |
| Normal | `follow_link` | | Normal | `follow_link` |
| Normal | `diff`, `diff.view.toggle`, `diff.orientation.toggle` | | Normal | `diff`, `diff.mode.toggle`, `diff.view.toggle`, `diff.orientation.toggle` |
| Normal, Insert, Visual, Command, Search | `diff.close` | | Normal, Insert, Visual, Command, Search | `diff.close` |
| Normal, Insert, Visual, Command, Search | `word_wrap.toggle` | | Normal, Insert, Visual, Command, Search | `word_wrap.toggle` |
| Normal | `pane.split_horizontal`, `pane.split_vertical`, `pane.focus_left`, `pane.focus_down`, `pane.focus_up`, `pane.focus_right`, `pane.close`, `pane.force_close` | | Normal | `pane.split_horizontal`, `pane.split_vertical`, `pane.focus_left`, `pane.focus_down`, `pane.focus_up`, `pane.focus_right`, `pane.close`, `pane.force_close` |
@@ -237,7 +238,7 @@ config.bind "ctrl+h", nil, mode: config.modes.tree
The diff's changed-file explorer and floating picker use these same tree and selection contexts, including remaps and disabled keys. Enter opens the selected file's comparison; Working checkout After reuses its editable buffer. The catalog contains only changed paths, with A/M/D/R status markers, rather than a disk scan. The diff's changed-file explorer and floating picker use these same tree and selection contexts, including remaps and disabled keys. Enter opens the selected file's comparison; Working checkout After reuses its editable buffer. The catalog contains only changed paths, with A/M/D/R status markers, rather than a disk scan.
Ordinary `config.core.file_picker` and `config.core.file_explorer` bindings work inside comparisons. Ctrl+w closes a focused picker/explorer, retaining its query, selection, and expansion state; the panes use the reclaimed width. Ctrl+e has no reserved diff behavior. Escape closes the revision selector in either stage, but an opened comparison still closes explicitly through `:diff.close`, its palette action, or the generated action value below. `config.core.diff` remains the opening action and exposes immutable `close`, `view.toggle`, and `orientation.toggle` children. Ordinary `config.core.file_picker` and `config.core.file_explorer` bindings work inside comparisons. Ctrl+w closes a focused picker/explorer, retaining its query, selection, and expansion state; the panes use the reclaimed width. Ctrl+e has no reserved diff behavior. Escape closes the revision selector in either stage, but an opened comparison still closes explicitly through `:diff.close`, its palette action, or the generated action value below. `config.core.diff` remains the opening action and exposes immutable `close`, `mode.toggle`, `view.toggle`, and `orientation.toggle` children. New comparisons default to Branch changes (merge base → selected After); Normal-mode Ctrl+b, `:diff.mode.toggle`, or the palette toggles to direct Snapshots and back, preserving the selected endpoints. Selecting Working checkout on either side forces Snapshots.
`config.core.pane.close` and `config.core.pane.force_close` close a focused **docked** file list instead of its editor pane, without discarding buffers or exiting the comparison. A floating picker does not intercept these actions: normal close still protects underlying dirty buffers, while force-close retains its existing discard semantics. For example, bind `config.bind "ctrl+x", config.core.pane.close`; focused tree/selection overrides and printable filter input retain priority. Palette-originated close actions retain the originating dock target through force confirmation. `config.core.pane.close` and `config.core.pane.force_close` close a focused **docked** file list instead of its editor pane, without discarding buffers or exiting the comparison. A floating picker does not intercept these actions: normal close still protects underlying dirty buffers, while force-close retains its existing discard semantics. For example, bind `config.bind "ctrl+x", config.core.pane.close`; focused tree/selection overrides and printable filter input retain priority. Palette-originated close actions retain the originating dock target through force confirmation.
+3
View File
@@ -313,11 +313,14 @@ Run `:diff` in Normal mode, choose **Before**, then **After**, and press Enter t
The floating selector has **Revisions / Bookmarks** tabs for jj, or **Commits / Branches / Tags** for Git. Type to filter by revision/change ID, commit ID, reference name, or description. **Working checkout** stays first in every category, even when it does not match the query. As Before it is a disk snapshot, including eligible untracked files. As After it includes unsaved regular-text buffers and enables live editing; opening a comparison never saves them. The floating selector has **Revisions / Bookmarks** tabs for jj, or **Commits / Branches / Tags** for Git. Type to filter by revision/change ID, commit ID, reference name, or description. **Working checkout** stays first in every category, even when it does not match the query. As Before it is a disk snapshot, including eligible untracked files. As After it includes unsaved regular-text buffers and enables live editing; opening a comparison never saves them.
New comparisons default to **Branch changes**: Before becomes the unique merge base of the selected Before and After revisions, and After stays the selected tip. Changes exclusive to the Before branch are excluded. The Before border identifies both selected names and the immutable merge-base commit ID. **Snapshots** compares the two selected snapshots directly. **Ctrl+b** toggles the mode, also available as **`:diff.mode.toggle`**, the palette action, and remappable **`config.core.diff.mode.toggle`**. Toggling an opened comparison recomputes using the original selected endpoints. Selecting **Working checkout** on either side immediately forces **Snapshots**; Branch changes is unavailable while it is selected. Missing or multiple merge bases produce an explicit, recoverable selector error rather than silently falling back; choose Snapshots to compare such histories. jj's synthetic root is the empty ancestor of every jj history.
| Selector input | Effect | | Selector input | Effect |
| --- | --- | | --- | --- |
| Tab / Shift+Tab | Next / previous category | | Tab / Shift+Tab | Next / previous category |
| Up / Down, Ctrl+k / Ctrl+j | Previous / next entry | | Up / Down, Ctrl+k / Ctrl+j | Previous / next entry |
| Backspace / Ctrl+u | Remove a query grapheme / clear query | | Backspace / Ctrl+u | Remove a query grapheme / clear query |
| Ctrl+b | Toggle Branch changes / Snapshots (Working checkout requires Snapshots) |
| Enter | Choose the entry | | Enter | Choose the entry |
| Esc | Close either selector stage, cancel outstanding work, and restore the source buffer without discarding edits | | Esc | Close either selector stage, cancel outstanding work, and restore the source buffer without discarding edits |
+99 -5
View File
@@ -39,6 +39,20 @@ impl RepoKind {
} }
} }
} }
#[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
pub enum ComparisonMode {
#[default]
BranchChanges,
Snapshots,
}
impl ComparisonMode {
pub fn label(self) -> &'static str {
match self {
Self::BranchChanges => "Branch changes",
Self::Snapshots => "Snapshots",
}
}
}
#[derive(Clone, Debug, Eq, PartialEq)] #[derive(Clone, Debug, Eq, PartialEq)]
pub enum RevisionTarget { pub enum RevisionTarget {
Working, Working,
@@ -105,6 +119,7 @@ pub enum Operation {
Compare { Compare {
before: Revision, before: Revision,
after: Revision, after: Revision,
mode: ComparisonMode,
buffers: Box<[(PathBuf, String)]>, buffers: Box<[(PathBuf, String)]>,
}, },
LoadFile { LoadFile {
@@ -187,6 +202,7 @@ pub struct DiffState {
pub phase: Phase, pub phase: Phase,
pub word_wrap: bool, pub word_wrap: bool,
pub view_mode: ViewMode, pub view_mode: ViewMode,
pub comparison_mode: ComparisonMode,
pub split_axis: crate::panes::SplitAxis, pub split_axis: crate::panes::SplitAxis,
pub before: Option<Revision>, pub before: Option<Revision>,
pub after: Option<Revision>, pub after: Option<Revision>,
@@ -231,6 +247,7 @@ impl DiffState {
phase: Phase::Before, phase: Phase::Before,
word_wrap: false, word_wrap: false,
view_mode: ViewMode::Split, view_mode: ViewMode::Split,
comparison_mode: ComparisonMode::default(),
split_axis: crate::panes::SplitAxis::Vertical, split_axis: crate::panes::SplitAxis::Vertical,
before: None, before: None,
after: None, after: None,
@@ -548,6 +565,9 @@ impl DiffState {
} }
return; return;
}; };
if revision.target == RevisionTarget::Working {
self.comparison_mode = ComparisonMode::Snapshots;
}
if let RevisionTarget::FetchRemote(remote) = &revision.target { if let RevisionTarget::FetchRemote(remote) = &revision.target {
self.queue(Operation::FetchRemote(remote.clone())); self.queue(Operation::FetchRemote(remote.clone()));
} else if let RevisionTarget::ListRemote(remote) = &revision.target { } else if let RevisionTarget::ListRemote(remote) = &revision.target {
@@ -559,13 +579,43 @@ impl DiffState {
self.query.clear(); self.query.clear();
self.refilter(); self.refilter();
self.error = None; self.error = None;
} else if let Some(before) = &self.before { } else {
self.after = Some(revision.clone()); self.after = Some(revision);
self.queue(Operation::Compare { self.queue_comparison();
}
}
pub(crate) fn working_selected(&self) -> bool {
[&self.before, &self.after]
.into_iter()
.flatten()
.any(|revision| revision.target == RevisionTarget::Working)
}
pub(crate) fn toggle_comparison_mode(&mut self) -> Result<(), &'static str> {
if self.working_selected() {
return Err("Working checkout requires Snapshots");
}
self.comparison_mode = match self.comparison_mode {
ComparisonMode::BranchChanges => ComparisonMode::Snapshots,
ComparisonMode::Snapshots => ComparisonMode::BranchChanges,
};
self.queue_comparison();
Ok(())
}
fn queue_comparison(&mut self) {
if let (Some(before), Some(after)) = (&self.before, &self.after) {
let operation = Operation::Compare {
before: before.clone(), before: before.clone(),
after: revision, after: after.clone(),
mode: self.comparison_mode,
buffers: Box::new([]), buffers: Box::new([]),
}); };
self.phase = Phase::After;
self.comparison = None;
self.comparison_id = None;
self.queue(operation);
} }
} }
@@ -3299,4 +3349,48 @@ mod tests {
}) })
)); ));
} }
#[test]
fn mode_action_keeps_filter_input_and_rejects_superseded_comparisons() {
let mut editor = editor();
open(&mut editor);
editor
.bind("normal", "m", Some("core.diff.mode.toggle".into()))
.unwrap();
text(&mut editor, "m");
assert_eq!(editor.diff().unwrap().query, "m");
assert_eq!(
editor.diff().unwrap().comparison_mode,
ComparisonMode::BranchChanges
);
editor.handle(Input::Ctrl('u'));
editor.handle(Input::Down);
editor.handle(Input::Enter);
editor.handle(Input::Down);
editor.handle(Input::Enter);
let (old, _) = editor.take_diff_request().unwrap();
editor.execute("diff.mode.toggle");
assert!(!editor.apply_diff_result(old, Err("superseded".into())));
let (current, operation) = editor.take_diff_request().unwrap();
assert!(matches!(
operation,
Operation::Compare {
mode: ComparisonMode::Snapshots,
..
}
));
assert!(editor.apply_diff_result(current, Err("retryable".into())));
assert_eq!(editor.diff().unwrap().phase, Phase::After);
assert_eq!(editor.diff().unwrap().error.as_deref(), Some("retryable"));
editor.execute("diff.mode.toggle");
let (_, operation) = editor.take_diff_request().unwrap();
assert!(matches!(
operation,
Operation::Compare {
mode: ComparisonMode::BranchChanges,
..
}
));
assert!(editor.diff().unwrap().error.is_none());
}
} }
+23 -1
View File
@@ -703,6 +703,12 @@ core_actions![
true true
), ),
("core.diff", Action::Diff, &[Mode::Normal], true), ("core.diff", Action::Diff, &[Mode::Normal], true),
(
"core.diff.mode.toggle",
Action::DiffModeToggle,
&[Mode::Normal],
true,
),
( (
"core.diff.view.toggle", "core.diff.view.toggle",
Action::DiffViewToggle, Action::DiffViewToggle,
@@ -1070,6 +1076,7 @@ fn default_bindings() -> HashMap<Mode, HashMap<Input, Binding>> {
} }
for (key, action) in [ for (key, action) in [
(Input::Ctrl('o'), JumpBack), (Input::Ctrl('o'), JumpBack),
(Input::Ctrl('b'), DiffModeToggle),
(Input::Char('\t'), JumpForward), (Input::Char('\t'), JumpForward),
(Input::Alt('h'), PaneFocusLeft), (Input::Alt('h'), PaneFocusLeft),
(Input::Alt('j'), PaneFocusDown), (Input::Alt('j'), PaneFocusDown),
@@ -3144,6 +3151,7 @@ impl Editor {
| Action::FilePicker | Action::FilePicker
| Action::FileExplorer | Action::FileExplorer
| Action::DiffClose | Action::DiffClose
| Action::DiffModeToggle
| Action::DiffViewToggle | Action::DiffViewToggle
| Action::DiffOrientationToggle | Action::DiffOrientationToggle
| Action::WordWrapToggle | Action::WordWrapToggle
@@ -5905,6 +5913,7 @@ impl Editor {
| Action::BufferForceDelete | Action::BufferForceDelete
| Action::Diff | Action::Diff
| Action::DiffClose | Action::DiffClose
| Action::DiffModeToggle
| Action::DiffViewToggle | Action::DiffViewToggle
| Action::DiffOrientationToggle | Action::DiffOrientationToggle
| Action::WordWrapToggle | Action::WordWrapToggle
@@ -6713,7 +6722,7 @@ impl Editor {
} }
if matches!( if matches!(
entry.action, entry.action,
Action::DiffViewToggle | Action::DiffOrientationToggle Action::DiffModeToggle | Action::DiffViewToggle | Action::DiffOrientationToggle
) { ) {
let name = entry.ex.expect("diff display action has an Ex command"); let name = entry.ex.expect("diff display action has an Ex command");
if !args.is_empty() { if !args.is_empty() {
@@ -6730,6 +6739,19 @@ impl Editor {
self.active.view.command_line.clear(); self.active.view.command_line.clear();
self.active.view.mode = Mode::Normal; self.active.view.mode = Mode::Normal;
} }
if entry.action == Action::DiffModeToggle {
let Some(diff) = self.diff.as_mut() else {
self.set_message("No comparison is open");
return None;
};
if let Err(error) = diff.toggle_comparison_mode() {
self.set_message(error);
return None;
}
self.release_diff_buffers();
self.diff_return_buffer = Some(self.active.id);
return None;
}
let Some(diff) = self.diff.as_mut().filter(|diff| diff.phase == Phase::View) else { let Some(diff) = self.diff.as_mut().filter(|diff| diff.phase == Phase::View) else {
self.set_message("No comparison is open"); self.set_message("No comparison is open");
return None; return None;
+88 -12
View File
@@ -1,7 +1,7 @@
//! Read-only active-repository discovery and isolated revision comparison. //! Read-only active-repository discovery and isolated revision comparison.
use crate::diff::{ use crate::diff::{
Catalog, Category, ChangedFile, Comparison, FileStatus, RepoKind, Revision, RevisionTarget, Catalog, Category, ChangedFile, Comparison, ComparisonMode, FileStatus, RepoKind, Revision,
line_hunks, RevisionTarget, line_hunks,
}; };
use std::{ use std::{
collections::{BTreeMap, BTreeSet}, collections::{BTreeMap, BTreeSet},
@@ -664,6 +664,27 @@ impl Repository {
checked(run(cmd, vec![], &self.cancelled)?)?; checked(run(cmd, vec![], &self.cancelled)?)?;
self.private = Some((root.clone(), root)); self.private = Some((root.clone(), root));
} }
// Only the private store gains alternates; active refs and objects stay untouched.
let objects = self.git(
&self.store,
&[
"rev-parse",
"--path-format=absolute",
"--git-path",
"objects",
],
)?;
let mut alternate = vec![b'"'];
for &byte in objects.strip_suffix(b"\n").unwrap_or(&objects) {
if (b' '..=b'~').contains(&byte) && !matches!(byte, b'"' | b'\\') {
alternate.push(byte);
} else {
write!(&mut alternate, "\\{byte:03o}").unwrap();
}
}
alternate.extend_from_slice(b"\"\n");
let store = &self.private.as_ref().unwrap().1;
fs::write(store.join("objects/info/alternates"), alternate).map_err(|e| e.to_string())?;
Ok(()) Ok(())
} }
fn load_remote(&mut self, name: &str, reference: Option<&str>) -> Result<(), String> { fn load_remote(&mut self, name: &str, reference: Option<&str>) -> Result<(), String> {
@@ -681,10 +702,7 @@ impl Repository {
let (root, store) = self.private.as_ref().unwrap(); let (root, store) = self.private.as_ref().unwrap();
if self.kind == RepoKind::Jujutsu { if self.kind == RepoKind::Jujutsu {
if !self.registered.contains(name) { if !self.registered.contains(name) {
self.jj( self.jj(root, &["git", "remote", "add", "--", name, &url])?;
root,
&["git", "remote", "add", "--fetch-tags=all", "--", name, &url],
)?;
self.registered.insert(name.into()); self.registered.insert(name.into());
} }
self.jj( self.jj(
@@ -1279,6 +1297,40 @@ impl Repository {
self.commit_snapshot(&store, &id) self.commit_snapshot(&store, &id)
} }
} }
fn merge_base(&mut self, before: &str, after: &str) -> Result<String, String> {
const JJ_ROOT: &str = "0000000000000000000000000000000000000000";
if self.kind == RepoKind::Jujutsu && (before == JJ_ROOT || after == JJ_ROOT) {
return Ok(JJ_ROOT.into());
}
let store = self
.private
.as_ref()
.map_or(&self.store, |(_, store)| store);
let mut command = git_command(store);
command.args(["merge-base", "--all", before, after]);
let output = run(command, vec![], &self.cancelled)?;
if output.code == Some(1) && output.stdout.is_empty() {
// Every jj history has the synthetic root, absent from Git's graph.
return if self.kind == RepoKind::Jujutsu {
Ok(JJ_ROOT.into())
} else {
Err(
"Branch changes: no merge base; use Snapshots to compare unrelated histories"
.into(),
)
};
}
let bases = text(checked(output).map_err(|e| format!("Cannot resolve merge base: {e}"))?)?;
let mut bases = bases.lines();
let base = bases.next().ok_or("Branch changes: no merge base")?;
if bases.next().is_some() {
return Err(
"Branch changes: multiple merge bases; select Snapshots or different endpoints"
.into(),
);
}
Ok(base.into())
}
/// Freeze comparison sources and return path/status metadata only. /// Freeze comparison sources and return path/status metadata only.
/// A new comparison replaces sources retained for the previous request ID. /// A new comparison replaces sources retained for the previous request ID.
pub fn compare( pub fn compare(
@@ -1286,19 +1338,43 @@ impl Repository {
comparison_id: u64, comparison_id: u64,
before: &Revision, before: &Revision,
after: &Revision, after: &Revision,
mode: ComparisonMode,
buffers: &[(PathBuf, String)], buffers: &[(PathBuf, String)],
) -> Result<Comparison, String> { ) -> Result<Comparison, String> {
self.comparison = None; self.comparison = None;
let mut left = self.snapshot(&before.target)?; let resolved = if mode == ComparisonMode::BranchChanges
let working_after = after.target == RevisionTarget::Working; && before.target != RevisionTarget::Working
let mut right = if before.target == after.target && (!working_after || buffers.is_empty()) { && after.target != RevisionTarget::Working
{
let (_, before_id) = self.resolve(&before.target)?;
let (_, after_id) = self.resolve(&after.target)?;
let base = self.merge_base(&before_id, &after_id)?;
let label = format!(
"merge base of {} and {} · {base}",
before.label, after.label
);
Some((
RevisionTarget::Commit(base),
RevisionTarget::Commit(after_id),
label,
))
} else {
None
};
let (before_target, after_target, before_label) = resolved.as_ref().map_or(
(&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 same = before_target == after_target && (!working_after || buffers.is_empty());
let mut right = if same {
BTreeMap::new() BTreeMap::new()
} else if working_after { } else if working_after {
self.working_snapshot(buffers)? self.working_snapshot(buffers)?
} else { } else {
self.snapshot(&after.target)? self.snapshot(after_target)?
}; };
let same = before.target == after.target && (!working_after || buffers.is_empty());
// Frozen checkout/conflict bytes have no object identity. Compare them in a // Frozen checkout/conflict bytes have no object identity. Compare them in a
// single batch, while immutable tree/tree comparisons need only object IDs. // single batch, while immutable tree/tree comparisons need only object IDs.
let frozen = |file: &FileData| { let frozen = |file: &FileData| {
@@ -1389,7 +1465,7 @@ impl Repository {
.collect(), .collect(),
}); });
Ok(Comparison { Ok(Comparison {
before: before.label.clone(), before: before_label.into(),
after: after.label.clone(), after: after.label.clone(),
files, files,
root: self.root.clone(), root: self.root.clone(),
+2 -1
View File
@@ -78,9 +78,10 @@ impl DiffWorker {
DiffOperation::Compare { DiffOperation::Compare {
before, before,
after, after,
mode,
buffers, buffers,
} => repository } => repository
.compare(id, &before, &after, &buffers) .compare(id, &before, &after, mode, &buffers)
.map(DiffOutcome::Comparison), .map(DiffOutcome::Comparison),
DiffOperation::LoadFile { DiffOperation::LoadFile {
comparison_id, comparison_id,
+13 -6
View File
@@ -2343,7 +2343,7 @@ fn draw_diff(
} else { } else {
"After" "After"
}; };
let title = format!(" Diff · Choose {stage} "); let title = format!(" Diff · Choose {stage} · {} ", diff.comparison_mode.label());
let content = diff_panel(frame, panel, &title, "", "", true); let content = diff_panel(frame, panel, &title, "", "", true);
let notice = diff.error.as_deref().or_else(|| { let notice = diff.error.as_deref().or_else(|| {
if diff.loading { if diff.loading {
@@ -2381,7 +2381,11 @@ fn draw_diff(
.as_ref() .as_ref()
.map_or("not selected", |revision| revision.label.as_str()); .map_or("not selected", |revision| revision.label.as_str());
frame.render_widget( frame.render_widget(
Paragraph::new(format!("Choose {stage} · Before: {}", display_text(before))), Paragraph::new(format!(
"{} · Ctrl+b toggle · Before: {}",
diff.comparison_mode.label(),
display_text(before)
)),
stage_area, stage_area,
); );
if let Some(catalog) = &diff.catalog { if let Some(catalog) = &diff.catalog {
@@ -2869,7 +2873,7 @@ fn diff_pane_panel(
return area; return area;
} }
let title = file.map_or_else( let title = file.map_or_else(
|| " Diff ".into(), || format!(" Diff · {} ", diff.comparison_mode.label()),
|file| { |file| {
let label = if diff.view_mode == ViewMode::Unified { let label = if diff.view_mode == ViewMode::Unified {
if before { if before {
@@ -2888,9 +2892,10 @@ fn diff_pane_panel(
&file.path &file.path
}; };
format!( format!(
" {label} · {} · {} ", " {label} · {} · {} · {} ",
diff_status(file.status), diff_status(file.status),
display_text(&path.to_string_lossy()) display_text(&path.to_string_lossy()),
diff.comparison_mode.label()
) )
}, },
); );
@@ -5179,7 +5184,9 @@ mod tests {
assert!(text.contains("> commit-19")); assert!(text.contains("> commit-19"));
diff.handle(Input::Enter); diff.handle(Input::Enter);
render_diff(&mut terminal, &diff, &mut panels); render_diff(&mut terminal, &diff, &mut panels);
assert!(screen_text(&terminal).contains("Choose After · Before: commit-19")); let text = screen_text(&terminal);
assert!(text.contains("Choose After"));
assert!(text.contains("Branch changes · Ctrl+b toggle · Before: commit-19"));
assert!(screen_text(&terminal).contains("Tab categories")); assert!(screen_text(&terminal).contains("Tab categories"));
assert!(screen_text(&terminal).contains("Enter select")); assert!(screen_text(&terminal).contains("Enter select"));
diff.handle(Input::Ctrl('u')); diff.handle(Input::Ctrl('u'));
+126 -1
View File
@@ -7,7 +7,11 @@ use std::{
time::{Duration, Instant}, time::{Duration, Instant},
}; };
use vamp::{diff::Phase, editor::Input, session::EditorSession}; use vamp::{
diff::{ComparisonMode, Phase},
editor::Input,
session::EditorSession,
};
static NEXT: AtomicU64 = AtomicU64::new(0); static NEXT: AtomicU64 = AtomicU64::new(0);
@@ -111,6 +115,10 @@ fn typed_diff_edits_unsaved_working_after_and_preserves_undo_on_explicit_close()
session.handle(Input::Enter); session.handle(Input::Enter);
assert_eq!(session.editor().diff().unwrap().phase, Phase::After); assert_eq!(session.editor().diff().unwrap().phase, Phase::After);
session.handle(Input::Enter); session.handle(Input::Enter);
assert_eq!(
session.editor().diff().unwrap().comparison_mode,
ComparisonMode::Snapshots
);
wait_until(&mut session, |session| { wait_until(&mut session, |session| {
session session
.editor() .editor()
@@ -485,3 +493,120 @@ fn readonly_jump_history_restores_the_comparison_file_and_source_cursor() {
fs::read_to_string(root.join("first.rs")).unwrap() fs::read_to_string(root.join("first.rs")).unwrap()
); );
} }
#[test]
fn branch_changes_default_and_mode_switches_compare_the_selected_divergent_tips() {
let fixture = Fixture::new();
let root = fixture.0.join("repo");
fixture.git(&["init", "-q"]);
fs::write(root.join("common"), "common\n").unwrap();
fixture.git(&["add", "."]);
fixture.git(&["commit", "-qm", "common"]);
let base = fixture.git(&["rev-parse", "HEAD"]);
fs::write(root.join("base-only"), "base branch\n").unwrap();
fixture.git(&["add", "."]);
fixture.git(&["commit", "-qm", "base only"]);
let before = fixture.git(&["rev-parse", "HEAD"]);
fixture.git(&["checkout", "-qb", "feature", &base]);
fs::write(root.join("feature-only"), "feature branch\n").unwrap();
fixture.git(&["add", "."]);
fixture.git(&["commit", "-qm", "feature only"]);
let after = fixture.git(&["rev-parse", "HEAD"]);
let config = fixture.0.join("config.rb");
fs::write(&config, "Vamp.configure do |config|\n config.clipboard_mode = 'separate'\n config.bind 'ctrl+b', config.core.diff.mode.toggle\nend\n").unwrap();
let mut session =
EditorSession::open(Some(root.join("common")), vec![], Some(&config)).unwrap();
session.execute("diff");
wait_until(&mut session, |session| {
!session.editor().diff().unwrap().loading
});
assert_eq!(
session.editor().diff().unwrap().comparison_mode,
ComparisonMode::BranchChanges
);
session.handle(Input::Ctrl('b'));
assert_eq!(
session.editor().diff().unwrap().comparison_mode,
ComparisonMode::Snapshots
);
session.handle(Input::Ctrl('p'));
type_query(&mut session, "diff.mode.toggle");
session.handle(Input::Enter);
assert_eq!(
session.editor().diff().unwrap().comparison_mode,
ComparisonMode::BranchChanges
);
for id in [&before, &after] {
type_query(&mut session, id);
session.handle(Input::Down);
session.handle(Input::Enter);
}
wait_until(&mut session, |session| {
session.editor().diff().unwrap().selected_file_loaded()
});
let diff = session.editor().diff().unwrap();
let comparison = diff.comparison.as_ref().unwrap();
assert_eq!(comparison.files.len(), 1);
assert_eq!(comparison.files[0].path, Path::new("feature-only"));
assert_eq!(comparison.files[0].after, ["feature branch"]);
assert_eq!(
comparison.before,
format!("merge base of {before} and {after} · {base}")
);
assert_eq!(diff.before.as_ref().unwrap().label, before);
session.execute("diff.mode.toggle");
wait_until(&mut session, |session| {
session.editor().diff().unwrap().selected_file_loaded()
});
let comparison = session
.editor()
.diff()
.unwrap()
.comparison
.as_ref()
.unwrap();
assert_eq!(comparison.before, before);
assert_eq!(comparison.after, after);
assert_eq!(
comparison
.files
.iter()
.map(|file| file.path.as_path())
.collect::<Vec<_>>(),
[Path::new("base-only"), Path::new("feature-only")]
);
// Superseding an in-flight recomparison must not restore the old mode's result.
session.execute("diff.mode.toggle");
session.execute("diff.mode.toggle");
session.execute("diff.mode.toggle");
wait_until(&mut session, |session| {
session.editor().diff().unwrap().selected_file_loaded()
});
let diff = session.editor().diff().unwrap();
assert_eq!(diff.comparison_mode, ComparisonMode::BranchChanges);
assert_eq!(diff.comparison.as_ref().unwrap().files.len(), 1);
assert_eq!(diff.before.as_ref().unwrap().label, before);
assert_eq!(diff.after.as_ref().unwrap().label, after);
session.execute("diff.close");
assert_eq!(session.editor().path(), Some(root.join("common").as_path()));
session.execute("diff");
wait_until(&mut session, |session| {
!session.editor().diff().unwrap().loading
});
session.handle(Input::Enter);
let diff = session.editor().diff().unwrap();
assert_eq!(diff.phase, Phase::After);
assert_eq!(diff.comparison_mode, ComparisonMode::Snapshots);
session.execute("diff.mode.toggle");
assert_eq!(
session.editor().diff().unwrap().comparison_mode,
ComparisonMode::Snapshots
);
assert_eq!(
session.editor().message(),
"Working checkout requires Snapshots"
);
}
+202 -5
View File
@@ -16,7 +16,7 @@ use std::{
}, },
}; };
use vamp::{ use vamp::{
diff::{Category, Comparison, FileStatus, RepoKind, Revision, RevisionTarget}, diff::{Category, Comparison, ComparisonMode, FileStatus, RepoKind, Revision, RevisionTarget},
repository::Repository, repository::Repository,
}; };
@@ -134,7 +134,8 @@ fn compare_loaded(
after: &Revision, after: &Revision,
buffers: &[(PathBuf, String)], buffers: &[(PathBuf, String)],
) -> Result<Comparison, String> { ) -> Result<Comparison, String> {
let mut comparison = repository.compare(1, before, after, buffers)?; let mut comparison =
repository.compare(1, before, after, ComparisonMode::Snapshots, buffers)?;
for (index, file) in comparison.files.iter_mut().enumerate() { for (index, file) in comparison.files.iter_mut().enumerate() {
*file = repository.load_file(1, index)?; *file = repository.load_file(1, index)?;
if file if file
@@ -381,7 +382,13 @@ fn git_working_reads_disk_and_handles_ranges_ignored_files_binary_links_and_odd_
); );
assert!( assert!(
repository repository
.compare(1, &working(RepoKind::Git), &working(RepoKind::Git), &[]) .compare(
1,
&working(RepoKind::Git),
&working(RepoKind::Git),
ComparisonMode::Snapshots,
&[]
)
.unwrap() .unwrap()
.files .files
.is_empty() .is_empty()
@@ -574,6 +581,40 @@ fn jj_catalog_and_working_comparisons_prefer_jj_and_never_snapshot_or_merge_oper
.iter() .iter()
.any(|f| f.path == Path::new("ignored-untracked")) .any(|f| f.path == Path::new("ignored-untracked"))
); );
let synthetic_root = revision(&"0".repeat(40), RepoKind::Jujutsu);
let branch = repository
.compare(
20,
&synthetic_root,
baseline,
ComparisonMode::BranchChanges,
&[],
)
.unwrap();
assert!(branch.before.ends_with(&"0".repeat(40)));
assert!(
branch
.files
.iter()
.any(|file| file.path == Path::new("file"))
);
let reversed = repository
.compare(
21,
baseline,
&synthetic_root,
ComparisonMode::BranchChanges,
&[],
)
.unwrap();
assert!(reversed.files.is_empty());
assert!(
repository
.compare(22, baseline, baseline, ComparisonMode::BranchChanges, &[])
.unwrap()
.files
.is_empty()
);
assert_eq!(disk_state(&active), state); assert_eq!(disk_state(&active), state);
} }
} }
@@ -657,6 +698,15 @@ fn jj_private_fetch_discovers_never_fetched_bookmark_and_searchable_remote_chang
.iter() .iter()
.any(|r| r.target == RevisionTarget::Commit(ancestor.clone())) .any(|r| r.target == RevisionTarget::Commit(ancestor.clone()))
); );
let local = catalog.entries.iter().find(|revision| {
matches!(&revision.target, RevisionTarget::Commit(id) if id.bytes().any(|byte| byte != b'0'))
}).unwrap();
let branch = repository
.compare(60, local, &remote, ComparisonMode::BranchChanges, &[])
.unwrap();
assert!(branch.before.ends_with(&"0".repeat(40)));
assert_eq!(branch.files.len(), 1);
assert_eq!(repository.load_file(60, 0).unwrap().after, ["remote tip"]);
let comparison = let comparison =
compare_loaded(&mut repository, &remote, &working(RepoKind::Jujutsu), &[]).unwrap(); compare_loaded(&mut repository, &remote, &working(RepoKind::Jujutsu), &[]).unwrap();
assert_eq!( assert_eq!(
@@ -682,6 +732,14 @@ fn jj_private_fetch_discovers_never_fetched_bookmark_and_searchable_remote_chang
changed(&comparison, Path::new("file")).after, changed(&comparison, Path::new("file")).after,
["remote tip"] ["remote tip"]
); );
let branch = repository
.compare(61, historic, &remote, ComparisonMode::BranchChanges, &[])
.unwrap();
assert!(branch.before.ends_with(&ancestor));
assert_eq!(
repository.load_file(61, 0).unwrap().before,
["remote ancestor"]
);
let store = active.join(".jj/repo/store"); let store = active.join(".jj/repo/store");
let active_store = let active_store =
fs::canonicalize(store.join(fs::read_to_string(store.join("git_target")).unwrap().trim())) fs::canonicalize(store.join(fs::read_to_string(store.join("git_target")).unwrap().trim()))
@@ -747,6 +805,7 @@ fn errors_are_explicit_and_do_not_modify_active_repository() {
1, 1,
&revision("--bad", RepoKind::Git), &revision("--bad", RepoKind::Git),
&working(RepoKind::Git), &working(RepoKind::Git),
ComparisonMode::Snapshots,
&[] &[]
) )
.unwrap_err() .unwrap_err()
@@ -758,6 +817,7 @@ fn errors_are_explicit_and_do_not_modify_active_repository() {
1, 1,
&revision(&"f".repeat(40), RepoKind::Git), &revision(&"f".repeat(40), RepoKind::Git),
&working(RepoKind::Git), &working(RepoKind::Git),
ComparisonMode::Snapshots,
&[], &[],
) )
.unwrap_err() .unwrap_err()
@@ -769,7 +829,13 @@ fn errors_are_explicit_and_do_not_modify_active_repository() {
}; };
assert!( assert!(
repository repository
.compare(1, &loader, &revision(&id, RepoKind::Git), &[]) .compare(
1,
&loader,
&revision(&id, RepoKind::Git),
ComparisonMode::Snapshots,
&[]
)
.unwrap_err() .unwrap_err()
.contains("history loader") .contains("history loader")
); );
@@ -813,6 +879,7 @@ fn unchanged_submodules_are_not_false_changes_and_changed_pointers_are_explicit(
1, 1,
&revision(&baseline, RepoKind::Git), &revision(&baseline, RepoKind::Git),
&working(RepoKind::Git), &working(RepoKind::Git),
ComparisonMode::Snapshots,
&[] &[]
) )
.unwrap() .unwrap()
@@ -1033,6 +1100,29 @@ fn jj_conflicted_revisions_show_cli_projection_and_keep_ordinary_files_navigable
assert_eq!(ordinary.before, ["old"]); assert_eq!(ordinary.before, ["old"]);
assert_eq!(ordinary.after, ["left ordinary"]); assert_eq!(ordinary.after, ["left ordinary"]);
assert!(ordinary.notice.is_none()); assert!(ordinary.notice.is_none());
let branch = repository
.compare(
30,
&revision(&base, RepoKind::Jujutsu),
&revision(&merged, RepoKind::Jujutsu),
ComparisonMode::BranchChanges,
&[],
)
.unwrap();
let index = branch
.files
.iter()
.position(|file| file.path == Path::new("conflict"))
.unwrap();
let conflict = repository.load_file(30, index).unwrap();
assert_eq!(conflict.after, expected);
assert!(
conflict
.notice
.as_deref()
.unwrap()
.contains("conflict projection")
);
let working_comparison = compare_loaded( let working_comparison = compare_loaded(
&mut repository, &mut repository,
&revision(&merged, RepoKind::Jujutsu), &revision(&merged, RepoKind::Jujutsu),
@@ -1112,7 +1202,7 @@ fn unsaved_overlays_are_working_after_only_and_cannot_follow_nonregular_paths()
); );
assert!( assert!(
repository repository
.compare(1, &base, &base, &buffers) .compare(1, &base, &base, ComparisonMode::Snapshots, &buffers)
.unwrap() .unwrap()
.files .files
.is_empty() .is_empty()
@@ -1152,6 +1242,7 @@ fn immutable_catalog_skips_unchanged_blobs_and_prepares_only_requested_details()
100, 100,
&revision(&before, RepoKind::Git), &revision(&before, RepoKind::Git),
&revision(&after, RepoKind::Git), &revision(&after, RepoKind::Git),
ComparisonMode::Snapshots,
&[], &[],
) )
.unwrap(); .unwrap();
@@ -1176,6 +1267,7 @@ fn immutable_catalog_skips_unchanged_blobs_and_prepares_only_requested_details()
101, 101,
&revision(&after, RepoKind::Git), &revision(&after, RepoKind::Git),
&revision(&after, RepoKind::Git), &revision(&after, RepoKind::Git),
ComparisonMode::Snapshots,
&[], &[],
) )
.unwrap(); .unwrap();
@@ -1198,6 +1290,7 @@ fn deferred_file_details_keep_frozen_checkout_and_unsaved_after() {
50, 50,
&working(RepoKind::Git), &working(RepoKind::Git),
&working(RepoKind::Git), &working(RepoKind::Git),
ComparisonMode::Snapshots,
&[(root.join("file"), "unsaved after\n".into())], &[(root.join("file"), "unsaved after\n".into())],
) )
.unwrap(); .unwrap();
@@ -1211,3 +1304,107 @@ fn deferred_file_details_keep_frozen_checkout_and_unsaved_after() {
assert_eq!(again.before, details.before); assert_eq!(again.before, details.before);
assert_eq!(again.after, details.after); assert_eq!(again.after, details.after);
} }
#[test]
fn branch_changes_resolves_local_and_private_remote_ancestry_without_active_mutation() {
let fixture = Fixture::new();
let active = fixture.dir("active\nquoted\"\\");
init_git(&active);
fs::write(active.join("common"), "base\n").unwrap();
git(&active, &["add", "."]);
git(&active, &["commit", "-m", "base"]);
let base = git(&active, &["rev-parse", "HEAD"]);
let source = fixture.0.join("source");
git(
&fixture.0,
&["clone", active.to_str().unwrap(), source.to_str().unwrap()],
);
git(&source, &["config", "user.name", "Diff Test"]);
git(&source, &["config", "user.email", "diff@example.invalid"]);
fs::write(source.join("remote-only"), "remote\n").unwrap();
git(&source, &["add", "."]);
git(&source, &["commit", "-m", "remote"]);
fs::write(active.join("local-only"), "local\n").unwrap();
git(&active, &["add", "."]);
git(&active, &["commit", "-m", "local"]);
let local = revision(&git(&active, &["rev-parse", "HEAD"]), RepoKind::Git);
git(
&active,
&["remote", "add", "origin", source.to_str().unwrap()],
);
let state = disk_state(&active);
let mut repository = open(&active);
let catalog = repository.list_remote("origin").unwrap();
let remote = catalog
.entries
.iter()
.find(|r| matches!(r.target, RevisionTarget::Remote { .. }))
.unwrap();
let branch = repository
.compare(40, &local, remote, ComparisonMode::BranchChanges, &[])
.unwrap();
assert_eq!(branch.files.len(), 1);
assert_eq!(branch.files[0].path, Path::new("remote-only"));
assert!(branch.before.ends_with(&base));
assert_eq!(repository.load_file(40, 0).unwrap().after, ["remote"]);
let reverse = repository
.compare(41, remote, &local, ComparisonMode::BranchChanges, &[])
.unwrap();
assert_eq!(reverse.files.len(), 1);
assert_eq!(reverse.files[0].path, Path::new("local-only"));
assert_eq!(repository.load_file(41, 0).unwrap().after, ["local"]);
assert!(
repository
.compare(42, &local, &local, ComparisonMode::BranchChanges, &[])
.unwrap()
.files
.is_empty()
);
assert_eq!(disk_state(&active), state);
}
#[test]
fn branch_changes_rejects_missing_and_multiple_merge_bases_without_snapshot_fallback() {
let fixture = Fixture::new();
let root = fixture.dir("graphs");
init_git(&root);
fs::write(root.join("file"), "base\n").unwrap();
git(&root, &["add", "."]);
git(&root, &["commit", "-m", "base"]);
let base = git(&root, &["rev-parse", "HEAD"]);
let tree = git(&root, &["rev-parse", "HEAD^{tree}"]);
let left = git(&root, &["commit-tree", &tree, "-p", &base, "-m", "left"]);
let right = git(&root, &["commit-tree", &tree, "-p", &base, "-m", "right"]);
let one = git(
&root,
&["commit-tree", &tree, "-p", &left, "-p", &right, "-m", "one"],
);
let two = git(
&root,
&["commit-tree", &tree, "-p", &right, "-p", &left, "-m", "two"],
);
let unrelated = git(&root, &["commit-tree", &tree, "-m", "unrelated"]);
let state = disk_state(&root);
let mut repository = open(&root);
for (before, after, message) in [
(&base, &unrelated, "no merge base"),
(&one, &two, "multiple merge bases"),
] {
let before = revision(before, RepoKind::Git);
let after = revision(after, RepoKind::Git);
assert!(
repository
.compare(50, &before, &after, ComparisonMode::BranchChanges, &[])
.unwrap_err()
.contains(message)
);
assert!(
repository
.compare(51, &before, &after, ComparisonMode::Snapshots, &[])
.unwrap()
.files
.is_empty()
);
}
assert_eq!(disk_state(&root), state);
}