Skip to content

Commit

Permalink
cli new: Allow --before/--after without a value to default to @
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ilyagr committed Nov 16, 2024
1 parent 4729c77 commit 602254c
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 13 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,9 @@ Thanks to the people who made this release happen!

* 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
Expand Down
16 changes: 12 additions & 4 deletions cli/src/commands/new.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,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<RevisionArg>,
/// 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<RevisionArg>,
}
Expand Down
4 changes: 2 additions & 2 deletions cli/tests/[email protected]
Original file line number Diff line number Diff line change
Expand Up @@ -1403,8 +1403,8 @@ For more information, see https://martinvonz.github.io/jj/latest/working-copy/.
* `-m`, `--message <MESSAGE>` — The change description to use
* `--no-edit` — Do not edit the newly created change
* `-A`, `--insert-after <INSERT_AFTER>` — Insert the new change after the given commit(s)
* `-B`, `--insert-before <INSERT_BEFORE>` — Insert the new change before the given commit(s)
* `-A`, `--insert-after <INSERT_AFTER>` — Insert the new change after the given commit(s), or after `@` if no commit is specified
* `-B`, `--insert-before <INSERT_BEFORE>` — Insert the new change before the given commit(s), or before `@` if no commit is specified
Expand Down
109 changes: 102 additions & 7 deletions cli/tests/test_new_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <INSERT_AFTER>' cannot be used with '[REVISIONS]...'
insta::assert_snapshot!(stderr, @r#"
error: the argument '--insert-after [<INSERT_AFTER>]' cannot be used with '[REVISIONS]...'
Usage: jj new --insert-after <INSERT_AFTER> [REVISIONS]...
Usage: jj new --insert-after [<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]
Expand Down Expand Up @@ -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 <INSERT_BEFORE>' cannot be used with '[REVISIONS]...'
insta::assert_snapshot!(stderr, @r#"
error: the argument '--insert-before [<INSERT_BEFORE>]' cannot be used with '[REVISIONS]...'
Usage: jj new --insert-before <INSERT_BEFORE> [REVISIONS]...
Usage: jj new --insert-before [<INSERT_BEFORE>] [REVISIONS]...
For more information, try '--help'.
"###);
"#);
}

#[test]
Expand Down

0 comments on commit 602254c

Please sign in to comment.