next/prev: fix a few bugs in --conflict

jj-vcs · Jun 21, 2024 · a185d49 · a185d49
1 parent d938104
commit a185d49
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 12 deletions.
diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs
@@ -128,12 +128,14 @@ pub(crate) fn cmd_next(
 
     let target_revset = if args.conflict {
         start_revset
+            .children()
             .descendants()
             .filtered(RevsetFilterPredicate::HasConflict)
             .roots()
     } else {
-        start_revset.descendants_at(args.offset).minus(&wc_revset)
-    };
+        start_revset.descendants_at(args.offset)
+    }
+    .minus(&wc_revset);
 
     let targets: Vec<Commit> = target_revset
         .evaluate_programmatic(workspace_command.repo().as_ref())?

diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs
@@ -90,10 +90,9 @@ pub(crate) fn cmd_prev(
         // If people desire to move to the root conflict, replace the `heads()` below
         // with `roots(). But let's wait for feedback.
         target_revset
+            .parents()
             .ancestors()
             .filtered(RevsetFilterPredicate::HasConflict)
-            // We need to filter out empty commits to not land on empty working-copies lying around.
-            .minus(&RevsetExpression::is_empty())
             .heads()
     } else {
         target_revset.ancestors_at(args.offset)

diff --git a/cli/tests/test_next_prev_commands.rs b/cli/tests/test_next_prev_commands.rs
@@ -594,12 +594,11 @@ fn test_prev_conflict() {
     ◉  zzzzzzzzzzzz
     "###);
     test_env.jj_cmd_ok(&repo_path, &["prev", "--conflict"]);
-    // TODO: We now should be a child of `third`.
     insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
     @  yostqsxwqrlt conflict
     │ ◉  royxmykxtrkr conflict fourth
-    │ ◉  kkmpptxzrspx conflict third
     ├─╯
+    ◉  kkmpptxzrspx conflict third
     ◉  rlvkpnrzqnoo conflict second
     ◉  qpvuntsmwlqt first
     ◉  zzzzzzzzzzzz
@@ -723,12 +722,14 @@ fn test_next_conflict_head() {
     @  rlvkpnrzqnoo conflict
     ◉  zzzzzzzzzzzz
     "###);
-    // TODO: The command should be an error since there is no conflict after the
-    // current one
-    test_env.jj_cmd_ok(&repo_path, &["next", "--conflict"]);
-    // TODO: The command should be an error since there is no conflict after the
-    // current one
-    test_env.jj_cmd_ok(&repo_path, &["next", "--conflict", "--edit"]);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--conflict"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: No descendant found 1 commit forward
+    "###);
+    let stderr = test_env.jj_cmd_failure(&repo_path, &["next", "--conflict", "--edit"]);
+    insta::assert_snapshot!(stderr, @r###"
+    Error: No descendant found 1 commit forward
+    "###);
 }
 
 fn get_log_output(test_env: &TestEnvironment, cwd: &Path) -> String {