Skip to content

Commit

Permalink
cli: obslog: show diffs from all predecessors, not first predecessor
Browse files Browse the repository at this point in the history
Suppose a squash node in obslog is analogous to a merge in revisions log, it
makes sense to show diffs from auto-merge (or auto-squash) parents. This
basically means a non-partial squash node no longer shows diffs.

This also fixes missing diffs at the root predecessors if there were.
  • Loading branch information
yuja committed Aug 7, 2024
1 parent 83ee648 commit f7836aa
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 30 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

* `jj --at-op=@` no longer merges concurrent operations if explicitly specified.

* `jj obslog -p` no longer shows diffs at non-partial squash operations.
Previously, it showed the same diffs as the second predecessor.

### Deprecations

* The original configuration syntax for `jj fix` is now deprecated in favor of
Expand Down
8 changes: 7 additions & 1 deletion cli/src/commands/interdiff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.

use std::slice;

use clap::ArgGroup;
use jj_lib::rewrite::rebase_to_dest_parent;
use tracing::instrument;
Expand Down Expand Up @@ -53,7 +55,11 @@ pub(crate) fn cmd_interdiff(
workspace_command.resolve_single_rev(args.from.as_ref().unwrap_or(&RevisionArg::AT))?;
let to = workspace_command.resolve_single_rev(args.to.as_ref().unwrap_or(&RevisionArg::AT))?;

let from_tree = rebase_to_dest_parent(workspace_command.repo().as_ref(), &from, &to)?;
let from_tree = rebase_to_dest_parent(
workspace_command.repo().as_ref(),
slice::from_ref(&from),
&to,
)?;
let to_tree = to.tree()?;
let matcher = workspace_command
.parse_file_patterns(&args.paths)?
Expand Down
8 changes: 2 additions & 6 deletions cli/src/commands/obslog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,8 @@ fn show_predecessor_patch(
commit: &Commit,
width: usize,
) -> Result<(), CommandError> {
let mut predecessors = commit.predecessors();
let predecessor = match predecessors.next() {
Some(predecessor) => predecessor?,
None => return Ok(()),
};
let predecessor_tree = rebase_to_dest_parent(repo, &predecessor, commit)?;
let predecessors: Vec<_> = commit.predecessors().try_collect()?;
let predecessor_tree = rebase_to_dest_parent(repo, &predecessors, commit)?;
let tree = commit.tree()?;
renderer.show_diff(
ui,
Expand Down
4 changes: 2 additions & 2 deletions cli/src/commands/operation/diff.rs
Original file line number Diff line number Diff line change
Expand Up @@ -556,8 +556,8 @@ fn show_change_diff(
width: usize,
) -> Result<(), CommandError> {
match (&*change.removed_commits, &*change.added_commits) {
([predecessor], [commit]) => {
let predecessor_tree = rebase_to_dest_parent(repo, predecessor, commit)?;
(predecessors @ [_], [commit]) => {
let predecessor_tree = rebase_to_dest_parent(repo, predecessors, commit)?;
let tree = commit.tree()?;
diff_renderer.show_diff(
ui,
Expand Down
17 changes: 5 additions & 12 deletions cli/tests/test_obslog_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ fn test_obslog_squash() {
test_env.jj_cmd_ok(&repo_path, &["new", "-m", "second"]);
std::fs::write(repo_path.join("file1"), "foo\nbar\n").unwrap();

// full
// not partial
test_env.jj_cmd_ok(&repo_path, &["squash", "-m", "squashed 1"]);

test_env.jj_cmd_ok(&repo_path, &["describe", "-m", "third"]);
Expand Down Expand Up @@ -267,10 +267,6 @@ fn test_obslog_squash() {
insta::assert_snapshot!(stdout, @r###"
○ qpvuntsm [email protected] 2001-02-03 08:05:15 d49749bf
├─┬─╮ squashed 3
│ │ │ Added regular file file4:
│ │ │ 1: foo4
│ │ │ Added regular file file5:
│ │ │ 1: foo5
│ │ ○ vruxwmqv hidden [email protected] 2001-02-03 08:05:15 8f2ae2b5
│ │ │ fifth
│ │ │ Added regular file file5:
Expand All @@ -285,10 +281,10 @@ fn test_obslog_squash() {
│ (empty) fourth
○ qpvuntsm hidden [email protected] 2001-02-03 08:05:12 1408a0a7
├─╮ squashed 2
│ │ Modified regular file file1:
│ │ 1 1: foo
│ │ 2 2: bar
│ │ 3: baz
│ │ Removed regular file file2:
│ │ 1 : foo2
│ │ Removed regular file file3:
│ │ 1 : foo3
│ ○ zsuskuln hidden [email protected] 2001-02-03 08:05:12 c9460789
│ │ third
│ │ Modified regular file file1:
Expand All @@ -305,9 +301,6 @@ fn test_obslog_squash() {
│ (empty) (no description set)
○ qpvuntsm hidden [email protected] 2001-02-03 08:05:10 e3c2a446
├─╮ squashed 1
│ │ Modified regular file file1:
│ │ 1 1: foo
│ │ 2: bar
│ ○ kkmpptxz hidden [email protected] 2001-02-03 08:05:10 46acd22a
│ │ second
│ │ Modified regular file file1:
Expand Down
23 changes: 14 additions & 9 deletions lib/src/rewrite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,20 +303,25 @@ pub fn rebase_commit_with_options(
}
}

/// Moves changes from `sources` to the `destination` parent, returns new tree.
pub fn rebase_to_dest_parent(
repo: &dyn Repo,
source: &Commit,
sources: &[Commit],
destination: &Commit,
) -> BackendResult<MergedTree> {
if source.parent_ids() == destination.parent_ids() {
Ok(source.tree()?)
} else {
let destination_parent_tree = destination.parent_tree(repo)?;
let source_parent_tree = source.parent_tree(repo)?;
let source_tree = source.tree()?;
let rebased_tree = destination_parent_tree.merge(&source_parent_tree, &source_tree)?;
Ok(rebased_tree)
if let [source] = sources {
if source.parent_ids() == destination.parent_ids() {
return source.tree();
}
}
sources.iter().try_fold(
destination.parent_tree(repo)?,
|destination_tree, source| {
let source_parent_tree = source.parent_tree(repo)?;
let source_tree = source.tree()?;
destination_tree.merge(&source_parent_tree, &source_tree)
},
)
}

#[derive(Clone, Copy, Default, PartialEq, Eq, Debug)]
Expand Down

0 comments on commit f7836aa

Please sign in to comment.