From a115d676ef4b2e6f1f2d8a822e39d112d82d7787 Mon Sep 17 00:00:00 2001 From: Ilya Grigoriev Date: Sun, 29 Sep 2024 14:25:04 -0700 Subject: [PATCH] cli `new`: Allow `--before/--after` without a value to default to `@` Without this, I find it a bit jarring that `jj new` works but `jj new --before` does not. By contrast, since `jj rebase` does not currently work, I don't think `jj rebase --before` should either. Note that `jj new --before @ another_revision` is invalid, so `jj new --before another_revision` can only be parsed correctly in one way. I am slightly concerned that `clap` might forbids this in the future even in the cases where a human can tell there is no ambiguity, but I'm hoping for the best. --- CHANGELOG.md | 3 + cli/src/commands/new.rs | 16 +++-- cli/tests/cli-reference@.md.snap | 4 +- cli/tests/test_new_command.rs | 109 +++++++++++++++++++++++++++++-- 4 files changed, 119 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15ff115076..630aef2953 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -151,6 +151,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). * Author and committer names are now yellow by default. +* Without a specified revision, `jj new --insert-before` is now equivalent to +`jj new --insert-before @`; same for `--insert-after`. + ### Fixed bugs * Update working copy before reporting changes. This prevents errors during reporting diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 09ea51a7a0..00bd1441b9 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -64,20 +64,28 @@ pub(crate) struct NewArgs { /// No-op flag to pair with --no-edit #[arg(long, hide = true)] _edit: bool, - /// Insert the new change after the given commit(s) + /// Insert the new change after the given commit(s), or after `@` if no + /// commit is specified #[arg( long, short = 'A', visible_alias = "after", - conflicts_with = "revisions" + conflicts_with = "revisions", + default_missing_value = "@", + // At most one argument per --after, does not restrict repeating --after + num_args=0..=1, )] insert_after: Vec, - /// Insert the new change before the given commit(s) + /// Insert the new change before the given commit(s), or before `@` if no + /// commit is specified #[arg( long, short = 'B', visible_alias = "before", - conflicts_with = "revisions" + conflicts_with = "revisions", + default_missing_value = "@", + // At most one argument per --before, does not restrict repeating --before + num_args=0..=1, )] insert_before: Vec, } diff --git a/cli/tests/cli-reference@.md.snap b/cli/tests/cli-reference@.md.snap index 7c569c9e0c..c86e4a5039 100644 --- a/cli/tests/cli-reference@.md.snap +++ b/cli/tests/cli-reference@.md.snap @@ -1315,8 +1315,8 @@ For more information, see https://martinvonz.github.io/jj/latest/working-copy/. * `-m`, `--message ` — The change description to use * `--no-edit` — Do not edit the newly created change -* `-A`, `--insert-after ` — Insert the new change after the given commit(s) -* `-B`, `--insert-before ` — Insert the new change before the given commit(s) +* `-A`, `--insert-after ` — Insert the new change after the given commit(s), or after `@` if no commit is specified +* `-B`, `--insert-before ` — Insert the new change before the given commit(s), or before `@` if no commit is specified diff --git a/cli/tests/test_new_command.rs b/cli/tests/test_new_command.rs index 35f9ec87ee..5b00de3750 100644 --- a/cli/tests/test_new_command.rs +++ b/cli/tests/test_new_command.rs @@ -224,13 +224,108 @@ fn test_new_insert_after() { // --after cannot be used with revisions let stderr = test_env.jj_cmd_cli_error(&repo_path, &["new", "--after", "B", "D"]); - insta::assert_snapshot!(stderr, @r###" - error: the argument '--insert-after ' cannot be used with '[REVISIONS]...' + insta::assert_snapshot!(stderr, @r#" + error: the argument '--insert-after []' cannot be used with '[REVISIONS]...' - Usage: jj new --insert-after [REVISIONS]... + Usage: jj new --insert-after [] [REVISIONS]... For more information, try '--help'. + "#); +} + +#[test] +fn test_new_insert_before_after_no_arg() { + let test_env = TestEnvironment::default(); + test_env.jj_cmd_ok(test_env.env_root(), &["git", "init", "repo"]); + let repo_path = test_env.env_root().join("repo"); + setup_before_insertion(&test_env, &repo_path); + test_env.jj_cmd_ok(&repo_path, &["edit", "-r", "B"]); + insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###" + ○ F + ├─╮ + │ ○ E + ○ │ D + ├─╯ + │ ○ C + │ @ B + │ ○ A + ├─╯ + ◆ root "###); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["new", "--after", "-m", "G"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Rebased 1 descendant commits + Working copy now at: nkmrtpmo cf1ca757 (empty) G + Parent commit : kkmpptxz bfd4157e B | (empty) B + "#); + insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r#" + ○ C + @ G + ○ B + ○ A + │ ○ F + │ ├─╮ + │ │ ○ E + ├───╯ + │ ○ D + ├─╯ + ◆ root + "#); + + let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["new", "-m", "H", "--before"]); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Rebased 2 descendant commits + Working copy now at: xznxytkn f9f74f27 (empty) H + Parent commit : kkmpptxz bfd4157e B | (empty) B + "#); + insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r#" + ○ C + ○ G + @ H + ○ B + ○ A + │ ○ F + │ ├─╮ + │ │ ○ E + ├───╯ + │ ○ D + ├─╯ + ◆ root + "#); + + let (stdout, stderr) = test_env.jj_cmd_ok( + &repo_path, + &["new", "-m", "I", "--after", "--after", "D", "--no-edit"], + ); + insta::assert_snapshot!(stdout, @""); + insta::assert_snapshot!(stderr, @r#" + Created new commit nmzmmopx 56056dac (empty) I + Rebased 3 descendant commits + "#); + insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r#" + ○ C + ○ G + │ ○ F + ╭─┤ + ○ │ I + ├───╮ + │ │ ○ D + @ │ │ H + ○ │ │ B + ○ │ │ A + ├───╯ + │ ○ E + ├─╯ + ◆ root + "#); + + let stderr = test_env.jj_cmd_failure(&repo_path, &["new", "--before", "--after"]); + insta::assert_snapshot!(stderr, @r#" + Error: Refusing to create a loop: commit f9f74f27408e would be both an ancestor and a descendant of the new commit + "#); } #[test] @@ -346,13 +441,13 @@ fn test_new_insert_before() { // --before cannot be used with revisions let stderr = test_env.jj_cmd_cli_error(&repo_path, &["new", "--before", "B", "D"]); - insta::assert_snapshot!(stderr, @r###" - error: the argument '--insert-before ' cannot be used with '[REVISIONS]...' + insta::assert_snapshot!(stderr, @r#" + error: the argument '--insert-before []' cannot be used with '[REVISIONS]...' - Usage: jj new --insert-before [REVISIONS]... + Usage: jj new --insert-before [] [REVISIONS]... For more information, try '--help'. - "###); + "#); } #[test]