Make jj next work when the working copy is a merge commit

CHANGELOG.md

@@ -30,7 +30,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.
 
 ### Fixed bugs
 

cli/src/commands/next.rs

@@ -115,24 +115,18 @@ 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)
+            .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())
@@ -148,7 +142,7 @@ pub(crate) fn cmd_next(
         }
         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 {

cli/tests/test_next_prev_commands.rs

@@ -131,21 +131,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
     "###);
 }