rebase: allow both --insert-after and --insert-before to be used …

…simultaneously

cli/src/commands/rebase.rs

@@ -128,7 +128,7 @@ use crate::ui::Ui;
 #[derive(clap::Args, Clone, Debug)]
 #[command(verbatim_doc_comment)]
 #[command(group(ArgGroup::new("to_rebase").args(&["branch", "source", "revision"])))]
-#[command(group(ArgGroup::new("target").args(&["destination", "insert_after", "insert_before"]).required(true)))]
+#[command(group(ArgGroup::new("target").args(&["destination", "insert_after", "insert_before"]).multiple(true).required(true)))]
 pub(crate) struct RebaseArgs {
     /// Rebase the whole branch relative to destination's ancestors (can be
     /// repeated)
@@ -171,6 +171,7 @@ pub(crate) struct RebaseArgs {
         long,
         short = 'A',
         visible_alias = "after",
+        conflicts_with = "destination",
         conflicts_with = "source",
         conflicts_with = "branch"
     )]
@@ -183,6 +184,7 @@ pub(crate) struct RebaseArgs {
         long,
         short = 'B',
         visible_alias = "before",
+        conflicts_with = "destination",
         conflicts_with = "source",
         conflicts_with = "branch"
     )]
@@ -237,7 +239,20 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
             EmptyBehaviour::Keep,
             "clap should forbid `-r --skip-empty`"
         );
-        if !args.insert_after.is_empty() {
+        if !args.insert_after.is_empty() && !args.insert_before.is_empty() {
+            let after_commits =
+                workspace_command.resolve_some_revsets_default_single(&args.insert_after)?;
+            let before_commits =
+                workspace_command.resolve_some_revsets_default_single(&args.insert_before)?;
+            rebase_revision_after_before(
+                ui,
+                command.settings(),
+                &mut workspace_command,
+                &after_commits,
+                &before_commits,
+                rev_arg,
+            )?;
+        } else if !args.insert_after.is_empty() {
             let after_commits =
                 workspace_command.resolve_some_revsets_default_single(&args.insert_after)?;
             rebase_revision_after(
@@ -553,6 +568,42 @@ fn rebase_revision_before(
     )
 }
 
+fn rebase_revision_after_before(
+    ui: &mut Ui,
+    settings: &UserSettings,
+    workspace_command: &mut WorkspaceCommandHelper,
+    after_commits: &IndexSet<Commit>,
+    before_commits: &IndexSet<Commit>,
+    rev_arg: &RevisionArg,
+) -> Result<(), CommandError> {
+    let old_commit = workspace_command.resolve_single_rev(rev_arg)?;
+    workspace_command.check_rewritable([old_commit.id()])?;
+    let before_commit_ids = before_commits.iter().ids().cloned().collect_vec();
+    workspace_command.check_rewritable(&before_commit_ids)?;
+
+    let after_commit_ids = after_commits.iter().ids().cloned().collect_vec();
+    let new_children_expression = RevsetExpression::commits(before_commit_ids);
+    let new_parents_expression = RevsetExpression::commits(after_commit_ids.clone());
+
+    ensure_no_commit_loop(
+        workspace_command.repo().as_ref(),
+        &new_children_expression,
+        &new_parents_expression,
+    )?;
+
+    let new_parent_ids = after_commit_ids;
+    let new_children = before_commits.iter().cloned().collect_vec();
+
+    move_commit(
+        ui,
+        settings,
+        workspace_command,
+        &new_parent_ids,
+        &new_children,
+        old_commit,
+    )
+}
+
 /// Extracts `commit` from the graph by rebasing its children onto its parents.
 /// This assumes that `commit` can be rewritten.
 fn extract_commit(

cli/tests/test_rebase_command.rs

@@ -1350,6 +1350,93 @@ fn test_rebase_revision_before() {
     "###);
 }
 
+#[test]
+fn test_rebase_revision_after_before() {
+    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");
+
+    create_commit(&test_env, &repo_path, "a", &[]);
+    create_commit(&test_env, &repo_path, "b1", &["a"]);
+    create_commit(&test_env, &repo_path, "b2", &["a"]);
+    create_commit(&test_env, &repo_path, "c", &["b1", "b2"]);
+    create_commit(&test_env, &repo_path, "d", &["c"]);
+    create_commit(&test_env, &repo_path, "e", &["c"]);
+    create_commit(&test_env, &repo_path, "f", &["e"]);
+    // Test the setup
+    insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
+    @  f  lylxulpl  88f778c5
+    ◉  e  kmkuslsw  48dd9e3f
+    │ ◉  d  znkkpsqq  92438fc9
+    ├─╯
+    ◉    c  vruxwmqv  c41e416e
+    ├─╮
+    │ ◉  b2  royxmykx  903ab0d6
+    ◉ │  b1  zsuskuln  072d5ae1
+    ├─╯
+    ◉  a  rlvkpnrz  2443ea76
+    ◉    zzzzzzzz  00000000
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["rebase", "-r", "c", "--before", "e", "--after", "d"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 4 commits
+    Working copy now at: lylxulpl 4aa9c561 f | f
+    Parent commit      : kmkuslsw 288ca227 e | e
+    Added 1 files, modified 0 files, removed 0 files
+    "###);
+    insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
+    @  f  lylxulpl  4aa9c561
+    ◉  e  kmkuslsw  288ca227
+    ◉  c  vruxwmqv  583a5164
+    ◉    d  znkkpsqq  1ca86c56
+    ├─╮
+    │ ◉  b2  royxmykx  903ab0d6
+    ◉ │  b1  zsuskuln  072d5ae1
+    ├─╯
+    ◉  a  rlvkpnrz  2443ea76
+    ◉    zzzzzzzz  00000000
+    "###);
+
+    let (stdout, stderr) = test_env.jj_cmd_ok(
+        &repo_path,
+        &["rebase", "-r", "f", "--before", "e", "--after", "d"],
+    );
+    insta::assert_snapshot!(stdout, @"");
+    insta::assert_snapshot!(stderr, @r###"
+    Rebased 2 commits
+    Working copy now at: lylxulpl 044de9f6 f | f
+    Parent commit      : znkkpsqq 1ca86c56 d | d
+    Added 0 files, modified 0 files, removed 2 files
+    "###);
+    insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
+    ◉    e  kmkuslsw  c3183134
+    ├─╮
+    │ @  f  lylxulpl  044de9f6
+    ◉ │  c  vruxwmqv  583a5164
+    ├─╯
+    ◉    d  znkkpsqq  1ca86c56
+    ├─╮
+    │ ◉  b2  royxmykx  903ab0d6
+    ◉ │  b1  zsuskuln  072d5ae1
+    ├─╯
+    ◉  a  rlvkpnrz  2443ea76
+    ◉    zzzzzzzz  00000000
+    "###);
+
+    let stderr = test_env.jj_cmd_failure(
+        &repo_path,
+        &["rebase", "-r", "e", "--before", "b2", "--before", "c"],
+    );
+    insta::assert_snapshot!(stderr, @r###"
+    Error: Refusing to create a loop: commit 1ca86c56c601 would be both an ancestor and a descendant of the rebased commit
+    "###);
+}
+
 #[test]
 fn test_rebase_skip_empty() {
     let test_env = TestEnvironment::default();