ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

next: support prompting for ambiguous commit targets

Browse source 
Resolves part of issue #2126
This commit is contained in:
Daniel Ploch 2024-01-19 12:49:13 -05:00 committed by Daniel Ploch
parent deaea0732a
commit 32b982e507
3 changed files with 112 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

45
cli/src/commands/next.rs
View file

@ -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.

3
cli/src/commands/prev.rs
View file

@ -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());

81
cli/tests/test_next_prev_commands.rs
View file

@ -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###"
Error: Ambiguous target commit
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
"###);
}
#[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
"###);
}