new: allow --insert-before and --insert-after simultaneously

CHANGELOG.md

@@ -69,6 +69,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   [#3800](https://github.com/martinvonz/jj/issues/3800) and
   [#3801](https://github.com/martinvonz/jj/issues/3801)).
 
+* `jj new`'s `--insert-before` and `--insert-after` options can now be used
+  simultaneously.
+
 ### Fixed bugs
 
 * Previously, `jj git push` only made sure that the branch is in the expected

cli/src/commands/new.rs

@@ -60,22 +60,20 @@ pub(crate) struct NewArgs {
     /// No-op flag to pair with --no-edit
     #[arg(long, hide = true)]
     _edit: bool,
-    /// Insert the new change between the given commit(s) and their children
+    /// Insert the new change after the given commit(s)
     #[arg(
         long,
         short = 'A',
         visible_alias = "after",
-        conflicts_with = "revisions",
-        conflicts_with = "insert_before"
+        conflicts_with = "revisions"
     )]
     insert_after: Vec<RevisionArg>,
-    /// Insert the new change between the given commit(s) and their parents
+    /// Insert the new change before the given commit(s)
     #[arg(
         long,
         short = 'B',
         visible_alias = "before",
-        conflicts_with = "revisions",
-        conflicts_with = "insert_after"
+        conflicts_with = "revisions"
     )]
     insert_before: Vec<RevisionArg>,
 }
@@ -95,12 +93,30 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
     let mut workspace_command = command.workspace_helper(ui)?;
 
     let parent_commits;
-    let parent_commit_ids;
+    let parent_commit_ids: Vec<CommitId>;
     let children_commits;
     let mut advance_branches_target = None;
     let mut advanceable_branches = vec![];
 
-    if !args.insert_before.is_empty() {
+    if !args.insert_before.is_empty() && !args.insert_after.is_empty() {
+        parent_commits = workspace_command
+            .resolve_some_revsets_default_single(&args.insert_after)?
+            .into_iter()
+            .collect_vec();
+        parent_commit_ids = parent_commits.iter().ids().cloned().collect();
+        children_commits = workspace_command
+            .resolve_some_revsets_default_single(&args.insert_before)?
+            .into_iter()
+            .collect_vec();
+        let children_commit_ids = children_commits.iter().ids().cloned().collect();
+        let children_expression = RevsetExpression::commits(children_commit_ids);
+        let parents_expression = RevsetExpression::commits(parent_commit_ids.clone());
+        ensure_no_commit_loop(
+            workspace_command.repo(),
+            &children_expression,
+            &parents_expression,
+        )?;
+    } else if !args.insert_before.is_empty() {
         // Instead of having the new commit as a child of the changes given on the
         // command line, add it between the changes' parents and the changes.
         // The parents of the new commit will be the parents of the target commits
@@ -110,6 +126,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.",
             .into_iter()
             .collect_vec();
         let children_commit_ids = children_commits.iter().ids().cloned().collect();
+        workspace_command.check_rewritable(&children_commit_ids)?;
         let children_expression = RevsetExpression::commits(children_commit_ids);
         let parents_expression = children_expression.parents();
         ensure_no_commit_loop(

cli/tests/cli-reference@.md.snap

@@ -1158,8 +1158,8 @@ For more information, see https://github.com/martinvonz/jj/blob/main/docs/workin
 
   Possible values: `true`, `false`
 
-* `-A`, `--insert-after <INSERT_AFTER>` — Insert the new change between the given commit(s) and their children
-* `-B`, `--insert-before <INSERT_BEFORE>` — Insert the new change between the given commit(s) and their parents
+* `-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)
 
 
 

cli/tests/test_new_command.rs

@@ -231,16 +231,6 @@ fn test_new_insert_after() {
 
     For more information, try '--help'.
     "###);
-
-    // --after cannot be used with --before
-    let stderr = test_env.jj_cmd_cli_error(&repo_path, &["new", "--after", "B", "--before", "D"]);
-    insta::assert_snapshot!(stderr, @r###"
-    error: the argument '--insert-after <INSERT_AFTER>' cannot be used with '--insert-before <INSERT_BEFORE>'
-
-    Usage: jj new --insert-after <INSERT_AFTER> [REVISIONS]...
-
-    For more information, try '--help'.
-    "###);
 }
 
 #[test]
@@ -517,6 +507,115 @@ fn test_new_insert_before_root() {
     "###);
 }
 
+#[test]
+fn test_new_insert_after_before() {
+    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);
+    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", "-m", "G", "--after", "C", "--before", "F"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 1 descendant commits
+    Working copy now at: kxryzmor 33be1218 (empty) G
+    Parent commit      : mzvwutvl ec18c57d C | (empty) C
+    "###);
+    insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
+    ◉      F
+    ├─┬─╮
+    │ │ @  G
+    │ │ ◉  C
+    │ │ ◉  B
+    │ │ ◉  A
+    │ ◉ │  E
+    │ ├─╯
+    ◉ │  D
+    ├─╯
+    ◉  root
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["new", "-m", "H", "--after", "D", "--before", "B"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 4 descendant commits
+    Working copy now at: uyznsvlq fcf8281b (empty) H
+    Parent commit      : vruxwmqv c9257eff D | (empty) D
+    "###);
+    insta::assert_snapshot!(get_short_log_output(&test_env, &repo_path), @r###"
+    ◉      F
+    ├─┬─╮
+    │ │ ◉  G
+    │ │ ◉  C
+    │ │ ◉    B
+    │ │ ├─╮
+    │ │ │ @  H
+    ├─────╯
+    ◉ │ │  D
+    │ │ ◉  A
+    ├───╯
+    │ ◉  E
+    ├─╯
+    ◉  root
+    "###);
+}
+
+#[test]
+fn test_new_insert_after_before_no_loop() {
+    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);
+    let template = r#"commit_id.short() ++ " " ++ if(description, description, "root")"#;
+    let stdout = test_env.jj_cmd_success(&repo_path, &["log", "-T", template]);
+    insta::assert_snapshot!(stdout, @r###"
+    @    7705d353bf5d F
+    ├─╮
+    │ ◉  41a89ffcbba2 E
+    ◉ │  c9257eff5bf9 D
+    ├─╯
+    │ ◉  ec18c57d72d8 C
+    │ ◉  6041917ceeb5 B
+    │ ◉  65b1ef43c737 A
+    ├─╯
+    ◉  000000000000 root
+    "###);
+
+    let stderr = test_env.jj_cmd_failure(
+        &repo_path,
+        &[
+            "new",
+            "-m",
+            "G",
+            "--insert-before",
+            "A",
+            "--insert-after",
+            "C",
+        ],
+    );
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Refusing to create a loop: commit ec18c57d72d8 would be both an ancestor and a descendant of the new commit
+    "###);
+}
+
 #[test]
 fn test_new_conflicting_branches() {
     let test_env = TestEnvironment::default();