Skip to content

Commit

Permalink
cli: op diff: be more strict about multiple diff sources/destinations
Browse files Browse the repository at this point in the history
As the doc comment states, this function only supports change of "a single
added and removed commit", "only a single added", or "single removed commit."
It doesn't make sense to show diffs from the parent commit ignoring multiple
removed commits for example.
  • Loading branch information
yuja committed Aug 7, 2024
1 parent b42e949 commit 92ba6d3
Showing 1 changed file with 23 additions and 22 deletions.
45 changes: 23 additions & 22 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -552,30 +552,31 @@ fn show_change_diff(
formatter: &mut dyn Formatter,
repo: &dyn Repo,
diff_renderer: &DiffRenderer,
modified_change: &ModifiedChange,
change: &ModifiedChange,
width: usize,
) -> Result<(), CommandError> {
if modified_change.added_commits.len() == 1 && modified_change.removed_commits.len() == 1 {
let commit = &modified_change.added_commits[0];
let predecessor = &modified_change.removed_commits[0];
let predecessor_tree = rebase_to_dest_parent(repo, predecessor, commit)?;
let tree = commit.tree()?;
diff_renderer.show_diff(
ui,
formatter,
&predecessor_tree,
&tree,
&EverythingMatcher,
width,
)?;
} else if modified_change.added_commits.len() == 1 {
let commit = &modified_change.added_commits[0];
diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
} else if modified_change.removed_commits.len() == 1 {
// TODO: Should we show a reverse diff?
let commit = &modified_change.removed_commits[0];
diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
match (&*change.removed_commits, &*change.added_commits) {
([predecessor], [commit]) => {
let predecessor_tree = rebase_to_dest_parent(repo, predecessor, commit)?;
let tree = commit.tree()?;
diff_renderer.show_diff(
ui,
formatter,
&predecessor_tree,
&tree,
&EverythingMatcher,
width,
)?;
}
([], [commit]) => {
diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
}
([commit], []) => {
// TODO: Should we show a reverse diff?
diff_renderer.show_patch(ui, formatter, commit, &EverythingMatcher, width)?;
}
([_, _, ..], _) | (_, [_, _, ..]) => {}
([], []) => panic!("ModifiedChange should have at least one entry"),
}

Ok(())
}

0 comments on commit 92ba6d3

Please sign in to comment.