next: support prompting for ambiguous commit targets

Resolves part of issue jj-vcs#2126

cli/src/commands/next.rs

@@ -12,12 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+use std::io::Write;
+
 use itertools::Itertools;
 use jj_lib::commit::Commit;
 use jj_lib::repo::Repo;
 use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 
-use crate::cli_util::{short_commit_hash, user_error, CommandError, CommandHelper};
+use crate::cli_util::{
+    short_commit_hash, user_error, CommandError, CommandHelper, WorkspaceCommandHelper,
+};
 use crate::ui::Ui;
 
 /// Move the current working copy commit to the next child revision in the
@@ -45,7 +49,6 @@ use crate::ui::Ui;
 /// B => @
 /// |    |
 /// @    A
-// TODO(#2126): Handle multiple child revisions properly.
 #[derive(clap::Args, Clone, Debug)]
 #[command(verbatim_doc_comment)]
 pub(crate) struct NextArgs {
@@ -60,6 +63,38 @@ pub(crate) struct NextArgs {
     edit: bool,
 }
 
+pub fn choose_commit<'a>(
+    ui: &mut Ui,
+    workspace_command: &WorkspaceCommandHelper,
+    cmd: &str,
+    commits: &'a [Commit],
+) -> Result<&'a Commit, CommandError> {
+    writeln!(ui.stdout(), "ambiguous {cmd} commit, choose one to target:")?;
+    let mut choices: Vec<String> = Default::default();
+    for (i, commit) in commits.iter().enumerate() {
+        writeln!(
+            ui.stdout(),
+            "{}: {}",
+            i + 1,
+            workspace_command.format_commit_summary(commit)
+        )?;
+        choices.push(format!("{}", i + 1));
+    }
+    writeln!(ui.stdout(), "q: quit the prompt")?;
+    choices.push("q".to_string());
+
+    let choice = ui.prompt_choice(
+        "enter the index of the commit you want to target",
+        &choices,
+        None,
+    )?;
+    if choice == "q" {
+        return Err(user_error("ambiguous target commit"));
+    }
+
+    Ok(&commits[choice.parse::<usize>().unwrap() - 1])
+}
+
 pub(crate) fn cmd_next(
     ui: &mut Ui,
     command: &CommandHelper,
@@ -104,11 +139,7 @@ pub(crate) fn cmd_next(
                 if amount > 1 { "s" } else { "" }
             )));
         }
-        _ => {
-            // TODO(#2126) We currently cannot deal with multiple children, which result
-            // from branches. Prompt the user for resolution.
-            return Err(user_error("Ambiguous target commit"));
-        }
+        commits => choose_commit(ui, &workspace_command, "next", commits)?,
     };
     let target_short = short_commit_hash(target.id());
     // We're editing, just move to the target commit.

cli/src/commands/prev.rs

@@ -17,6 +17,7 @@ use jj_lib::repo::Repo;
 use jj_lib::revset::{RevsetExpression, RevsetIteratorExt};
 
 use crate::cli_util::{short_commit_hash, user_error, CommandError, CommandHelper};
+use crate::commands::next::choose_commit;
 use crate::ui::Ui;
 
 /// Move the working copy commit to the parent of the current revision.
@@ -101,7 +102,7 @@ pub(crate) fn cmd_prev(
                 if amount > 1 { "s" } else { "" }
             )))
         }
-        _ => return Err(user_error("Ambiguous target commit")),
+        commits => choose_commit(ui, &workspace_command, "prev", commits)?,
     };
     // Generate a short commit hash, to make it readable in the op log.
     let target_short = short_commit_hash(target.id());

cli/tests/test_next_prev_commands.rs

@@ -13,7 +13,7 @@
 // limitations under the License.
 //
 
-use crate::common::TestEnvironment;
+use crate::common::{get_stderr_string, get_stdout_string, TestEnvironment};
 
 pub mod common;
 
@@ -135,8 +135,7 @@ fn test_next_fails_on_merge_commit() {
 }
 
 #[test]
-fn test_next_fails_on_branching_children() {
-    // TODO(#2126): Fix this behavior
+fn test_next_fails_on_branching_children_no_stdin() {
     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");
@@ -145,10 +144,67 @@ fn test_next_fails_on_branching_children() {
     test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
     test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
     test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
+
     // Try to advance the working copy commit.
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["next"]);
+    let assert = test_env.jj_cmd(&repo_path, &["next"]).assert().code(255);
+    let stderr = test_env.normalize_output(&get_stderr_string(&assert));
+    insta::assert_snapshot!(stderr,@r###"
+    Internal error: I/O error: Cannot prompt for input since the output is not connected to a terminal
+    "###);
+}
+
+#[test]
+fn test_next_fails_on_branching_children_quit_prompt() {
+    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, &["commit", "-m", "first"]);
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
+    test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
+    test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
+
+    // Try to advance the working copy commit.
+    let assert = test_env
+        .jj_cmd_stdin(&repo_path, &["next"], "q\n")
+        .assert()
+        .code(1);
+    let stdout = test_env.normalize_output(&get_stdout_string(&assert));
+    let stderr = test_env.normalize_output(&get_stderr_string(&assert));
+    insta::assert_snapshot!(stdout,@r###"
+    ambiguous next commit, choose one to target:
+    1: zsuskuln 40a959a0 (empty) third
+    2: rlvkpnrz 5c52832c (empty) second
+    q: quit the prompt
+    enter the index of the commit you want to target: 
+    "###);
     insta::assert_snapshot!(stderr,@r###"
-    Error: Ambiguous target commit
+    Error: ambiguous target commit
+    "###);
+}
+
+#[test]
+fn test_next_choose_branching_child() {
+    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, &["commit", "-m", "first"]);
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "second"]);
+    test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
+    test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "third"]);
+    test_env.jj_cmd_ok(&repo_path, &["co", "@--"]);
+    // Advance the working copy commit.
+    let (stdout, stderr) = test_env.jj_cmd_stdin_ok(&repo_path, &["next"], "1\n");
+    insta::assert_snapshot!(stdout,@r###"
+    ambiguous next commit, choose one to target:
+    1: zsuskuln 40a959a0 (empty) third
+    2: rlvkpnrz 5c52832c (empty) second
+    q: quit the prompt
+    enter the index of the commit you want to target: 
+    "###);
+    insta::assert_snapshot!(stderr,@r###"
+    Working copy now at: yqosqzyt f9f7101a (empty) (no description set)
+    Parent commit      : zsuskuln 40a959a0 (empty) third
     "###);
 }
 
@@ -172,7 +228,6 @@ fn test_prev_fails_on_merge_commit() {
 
 #[test]
 fn test_prev_fails_on_multiple_parents() {
-    // TODO(#2126): Fix this behavior
     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");
@@ -184,10 +239,18 @@ fn test_prev_fails_on_multiple_parents() {
     // Create a merge commit, which has two parents.
     test_env.jj_cmd_ok(&repo_path, &["new", "left", "right"]);
     test_env.jj_cmd_ok(&repo_path, &["commit", "-m", "merge"]);
-    // We have more than one parent, prev fails.
-    let stderr = test_env.jj_cmd_failure(&repo_path, &["prev"]);
+    // Advance the working copy commit.
+    let (stdout, stderr) = test_env.jj_cmd_stdin_ok(&repo_path, &["prev"], "2\n");
+    insta::assert_snapshot!(stdout,@r###"
+    ambiguous prev commit, choose one to target:
+    1: zsuskuln edad76e9 right | (empty) second
+    2: qpvuntsm 5ae1a6a5 left | (empty) first
+    q: quit the prompt
+    enter the index of the commit you want to target: 
+    "###);
     insta::assert_snapshot!(stderr,@r###"
-    Error: Ambiguous target commit
+    Working copy now at: yostqsxw a9de0711 (empty) (no description set)
+    Parent commit      : qpvuntsm 5ae1a6a5 left | (empty) first
     "###);
 }