Skip to content

Commit

Permalink
Make jj next work when the working copy is a merge commit
Browse files Browse the repository at this point in the history
  • Loading branch information
@emesterhazy
emesterhazy committed Apr 8, 2024
1 parent 3787bd3 commit 457185c
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 31 deletions.
2 changes: 1 addition & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* File path arguments now support [file pattern
syntax](docs/filesets.md#file-patterns).

* `jj prev` now works when the working copy revision is a merge.
* `jj prev` and `jj next` now work when the working copy revision is a merge.

* Operation objects in templates now have a `snapshot() -> Boolean` method that
evaluates to true if the operation was a snapshot created by a non-mutating
Expand Down
36 changes: 16 additions & 20 deletions cli/src/commands/next.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ pub(crate) fn cmd_next(
args: &NextArgs,
) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let offset = args.offset;
let current_wc_id = workspace_command
.get_wc_commit_id()
.ok_or_else(|| user_error("This command requires a working copy"))?;
Expand All @@ -115,24 +114,20 @@ pub(crate) fn cmd_next(
.view()
.heads()
.contains(current_wc_id);
let current_wc = workspace_command.repo().store().get_commit(current_wc_id)?;
// If we're editing, start at the working-copy commit.
// Otherwise start from our direct parent.
let start_id = if edit {
current_wc_id
let wc_revset = RevsetExpression::commit(current_wc_id.clone());
// If we're editing, start at the working-copy commit. Otherwise, start from
// its direct parent(s).
let target_revset = if edit {
wc_revset.descendants_at(args.offset)
} else {
match current_wc.parent_ids() {
[parent_id] => parent_id,
_ => return Err(user_error("Cannot run `jj next` on a merge commit")),
}
};
let descendant_expression = RevsetExpression::commit(start_id.clone()).descendants_at(offset);
let target_expression = if edit {
descendant_expression
} else {
descendant_expression.minus(&RevsetExpression::commit(current_wc_id.clone()).descendants())
wc_revset
.parents()
.descendants_at(args.offset)
// In previous versions we subtracted `wc_revset.descendants()`. That's
// unnecessary now that --edit is implied if `@` has descendants.
.minus(&wc_revset)
};
let targets: Vec<Commit> = target_expression
let targets: Vec<Commit> = target_revset
.evaluate_programmatic(workspace_command.repo().as_ref())?
.iter()
.commits(workspace_command.repo().store())
Expand All @@ -142,13 +137,14 @@ pub(crate) fn cmd_next(
[] => {
// We found no descendant.
return Err(user_error(format!(
"No descendant found {offset} commit{} forward",
if offset > 1 { "s" } else { "" }
"No descendant found {} commit{} forward",
args.offset,
if args.offset > 1 { "s" } else { "" }
)));
}
commits => choose_commit(ui, &workspace_command, "next", commits)?,
};
let current_short = short_commit_hash(current_wc.id());
let current_short = short_commit_hash(current_wc_id);
let target_short = short_commit_hash(target.id());
// We're editing, just move to the target commit.
if edit {
Expand Down
126 changes: 116 additions & 10 deletions cli/tests/test_next_prev_commands.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,127 @@ fn test_next_exceeding_history() {
"###);
}

// The working copy commit is a child of a "fork" with two children.
#[test]
fn test_next_fails_on_merge_commit() {
fn test_next_parent_has_multiple_children() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
test_env.jj_cmd_ok(&repo_path, &["branch", "c", "left"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "first"]);
test_env.jj_cmd_ok(&repo_path, &["new", "@--"]);
test_env.jj_cmd_ok(&repo_path, &["branch", "c", "right"]);
test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
test_env.jj_cmd_ok(&repo_path, &["new", "left", "right"]);
// Try to advance the working copy commit.
let stderr = test_env.jj_cmd_failure(&repo_path, &["next"]);
// Setup.
test_env.jj_cmd_ok(&repo_path, &["desc", "-m", "1"]);
test_env.jj_cmd_ok(&repo_path, &["new", "-m", "2"]);
test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "3"]);
test_env.jj_cmd_ok(&repo_path, &["new", "-m", "4"]);
test_env.jj_cmd_ok(&repo_path, &["edit", "description(3)"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ mzvwutvlkqwt 4
@ zsuskulnrvyr 3
│ ◉ kkmpptxzrspx 2
│ ◉ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);

// --edit is implied since the working copy isn't a leaf commit.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout,@r###""###);
insta::assert_snapshot!(stderr,@r###"
Error: Cannot run `jj next` on a merge commit
Working copy now at: mzvwutvl 1b8531ce (empty) 4
Parent commit : zsuskuln b1394455 (empty) 3
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ mzvwutvlkqwt 4
◉ zsuskulnrvyr 3
│ ◉ kkmpptxzrspx 2
│ ◉ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);
}

#[test]
fn test_next_with_merge_commit_parent() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
// Setup.
test_env.jj_cmd_ok(&repo_path, &["desc", "-m", "1"]);
test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "2"]);
test_env.jj_cmd_ok(
&repo_path,
&["new", "description(1)", "description(2)", "-m", "3"],
);
test_env.jj_cmd_ok(&repo_path, &["new", "-m", "4"]);
test_env.jj_cmd_ok(&repo_path, &["prev", "0"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ royxmykxtrkr
│ ◉ mzvwutvlkqwt 4
├─╯
◉ zsuskulnrvyr 3
├─╮
│ ◉ kkmpptxzrspx 2
◉ │ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);

let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout,@r###""###);
insta::assert_snapshot!(stderr,@r###"
Working copy now at: vruxwmqv 718bbcd9 (empty) (no description set)
Parent commit : mzvwutvl cb5881ec (empty) 4
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ vruxwmqvtpmx
◉ mzvwutvlkqwt 4
◉ zsuskulnrvyr 3
├─╮
│ ◉ kkmpptxzrspx 2
◉ │ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);
}

#[test]
fn test_next_on_merge_commit() {
let test_env = TestEnvironment::default();
test_env.jj_cmd_ok(test_env.env_root(), &["init", "repo", "--git"]);
let repo_path = test_env.env_root().join("repo");
// Setup.
test_env.jj_cmd_ok(&repo_path, &["desc", "-m", "1"]);
test_env.jj_cmd_ok(&repo_path, &["new", "root()", "-m", "2"]);
test_env.jj_cmd_ok(
&repo_path,
&["new", "description(1)", "description(2)", "-m", "3"],
);
test_env.jj_cmd_ok(&repo_path, &["new", "-m", "4"]);
test_env.jj_cmd_ok(&repo_path, &["edit", "description(3)"]);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ mzvwutvlkqwt 4
@ zsuskulnrvyr 3
├─╮
│ ◉ kkmpptxzrspx 2
◉ │ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);

// --edit is implied since the working copy is not a leaf commit.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["next"]);
insta::assert_snapshot!(stdout,@r###""###);
insta::assert_snapshot!(stderr,@r###"
Working copy now at: mzvwutvl cb5881ec (empty) 4
Parent commit : zsuskuln 038acb86 (empty) 3
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ mzvwutvlkqwt 4
◉ zsuskulnrvyr 3
├─╮
│ ◉ kkmpptxzrspx 2
◉ │ qpvuntsmwlqt 1
├─╯
◉ zzzzzzzzzzzz
"###);
}

Expand Down

0 comments on commit 457185c

Please sign in to comment.