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

This commit is contained in:
Benjamin Tan 2024-03-30 04:57:06 +08:00
parent f75461efc1
commit 0e2e09a593
2 changed files with 216 additions and 2 deletions

View file

@ -127,7 +127,7 @@ use crate::ui::Ui;
#[derive(clap::Args, Clone, Debug)]
#[command(verbatim_doc_comment)]
#[command(group(ArgGroup::new("to_rebase").args(&["branch", "source", "revisions"])))]
#[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)
@ -242,7 +242,20 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
.parse_union_revsets(&args.revisions)?
.evaluate_to_commits()?
.try_collect()?; // in reverse topological order
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_revisions_after_before(
ui,
command.settings(),
&mut workspace_command,
&after_commits,
&before_commits,
&target_commits,
)?;
} else if !args.insert_after.is_empty() {
let after_commits =
workspace_command.resolve_some_revsets_default_single(&args.insert_after)?;
rebase_revisions_after(
@ -521,6 +534,41 @@ fn rebase_revisions_before(
)
}
fn rebase_revisions_after_before(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
after_commits: &IndexSet<Commit>,
before_commits: &IndexSet<Commit>,
target_commits: &[Commit],
) -> Result<(), CommandError> {
workspace_command.check_rewritable(target_commits.iter().ids())?;
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_commits_transaction(
ui,
settings,
workspace_command,
&new_parent_ids,
&new_children,
target_commits,
)
}
/// Wraps `move_commits` in a transaction.
fn move_commits_transaction(
ui: &mut Ui,

View file

@ -2085,6 +2085,172 @@ fn test_rebase_revisions_before() {
"###);
}
#[test]
fn test_rebase_revisions_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 setup_opid = test_env.current_operation_id(&repo_path);
// Rebase a commit after another commit and before that commit's child to
// insert directly between the two commits.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-r", "d", "--after", "e", "--before", "f"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits onto destination
Rebased 1 descendant commits
Working copy now at: lylxulpl fe3d8c30 f | f
Parent commit : znkkpsqq cca70ee1 d | d
Added 1 files, modified 0 files, removed 0 files
"###);
insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
@ f lylxulpl fe3d8c30
d znkkpsqq cca70ee1
e kmkuslsw 48dd9e3f
c vruxwmqv c41e416e
b2 royxmykx 903ab0d6
b1 zsuskuln 072d5ae1
a rlvkpnrz 2443ea76
zzzzzzzz 00000000
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
// Rebase a commit after another commit and before that commit's descendant to
// create a new merge commit.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-r", "d", "--after", "a", "--before", "f"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits onto destination
Rebased 1 descendant commits
Working copy now at: lylxulpl 22f0323c f | f
Parent commit : kmkuslsw 48dd9e3f e | e
Parent commit : znkkpsqq 61388bb6 d | d
Added 1 files, modified 0 files, removed 0 files
"###);
insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
@ f lylxulpl 22f0323c
d znkkpsqq 61388bb6
e kmkuslsw 48dd9e3f
c vruxwmqv c41e416e
b2 royxmykx 903ab0d6
b1 zsuskuln 072d5ae1
a rlvkpnrz 2443ea76
zzzzzzzz 00000000
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
// "c" has parents "b1" and "b2", so when it is rebased, its children "d" and
// "e" should have "b1" and "b2" as parents as well. "c" is then inserted in
// between "d" and "e", making "e" a merge commit with 3 parents "b1", "b2",
// and "c".
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-r", "c", "--after", "d", "--before", "e"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 1 commits onto destination
Rebased 3 descendant commits
Working copy now at: lylxulpl e37682c5 f | f
Parent commit : kmkuslsw 9bbc9e53 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 e37682c5
e kmkuslsw 9bbc9e53
c vruxwmqv e11c7c95
d znkkpsqq 37869bd5
b2 royxmykx 903ab0d6
b1 zsuskuln 072d5ae1
a rlvkpnrz 2443ea76
zzzzzzzz 00000000
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
// Rebase multiple commits and preserve their ancestry. Apart from the heads of
// the target commits ("d" and "e"), "f" also has commits "b1" and "b2" as
// parents since its parents "d" and "e" were in the target set and were
// replaced by their closest ancestors outside the target set.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&[
"rebase", "-r", "c", "-r", "d", "-r", "e", "--after", "a", "--before", "f",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits onto destination
Rebased 1 descendant commits
Working copy now at: lylxulpl 868f6c61 f | f
Parent commit : zsuskuln 072d5ae1 b1 | b1
Parent commit : royxmykx 903ab0d6 b2 | b2
Parent commit : znkkpsqq ae6181e6 d | d
Parent commit : kmkuslsw a55a6779 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 868f6c61
e kmkuslsw a55a6779
d znkkpsqq ae6181e6
c vruxwmqv 22540859
b2 royxmykx 903ab0d6
b1 zsuskuln 072d5ae1
a rlvkpnrz 2443ea76
zzzzzzzz 00000000
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
// Should error if a loop will be created.
let stderr = test_env.jj_cmd_failure(
&repo_path,
&["rebase", "-r", "e", "--after", "c", "--before", "a"],
);
insta::assert_snapshot!(stderr, @r###"
Error: Refusing to create a loop: commit c41e416ee4cf would be both an ancestor and a descendant of the rebased commits
"###);
}
#[test]
fn test_rebase_skip_empty() {
let test_env = TestEnvironment::default();