Skip to content

Commit

Permalink
rebase: allow both --insert-after and --insert-before to be used …
Browse files Browse the repository at this point in the history 
…simultaneously
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Mar 30, 2024
1 parent 1a4f7df commit 8bf886c
Show file tree
Hide file tree
Showing 2 changed files with 156 additions and 2 deletions.
71 changes: 69 additions & 2 deletions cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,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("order").args(&["destination", "insert_after", "insert_before"]).required(true)))]
#[command(group(ArgGroup::new("order").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)
Expand Down Expand Up @@ -167,6 +167,7 @@ pub(crate) struct RebaseArgs {
long,
short = 'A',
visible_alias = "after",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
Expand All @@ -179,6 +180,7 @@ pub(crate) struct RebaseArgs {
long,
short = 'B',
visible_alias = "before",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
Expand Down Expand Up @@ -237,8 +239,29 @@ 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() {
// TODO: Use `IndexSet`?
let after_commits = resolve_multiple_nonempty_revsets_default_single(
&workspace_command,
&args.insert_after,
)?
.into_iter()
.collect_vec();
let before_commits = resolve_multiple_nonempty_revsets_default_single(
&workspace_command,
&args.insert_before,
)?
.into_iter()
.collect_vec();
rebase_revision_after_before(
ui,
command.settings(),
&mut workspace_command,
after_commits,
before_commits,
rev_str,
)?;
} else if !args.insert_after.is_empty() {
let after_commits = resolve_multiple_nonempty_revsets_default_single(
&workspace_command,
&args.insert_after,
Expand Down Expand Up @@ -574,6 +597,50 @@ fn rebase_revision_before(
)
}

fn rebase_revision_after_before(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
after_commits: Vec<Commit>,
before_commits: Vec<Commit>,
rev_str: &str,
) -> Result<(), CommandError> {
let old_commit = workspace_command.resolve_single_rev(rev_str)?;
workspace_command.check_rewritable([&old_commit])?;
workspace_command.check_rewritable(&before_commits)?;

let after_commit_ids = after_commits.iter().map(|c| c.id().clone()).collect_vec();
let before_commit_ids = before_commits.iter().map(|c| c.id().clone()).collect_vec();
let new_children_expression = RevsetExpression::commits(before_commit_ids);
let new_parents_expression = RevsetExpression::commits(after_commit_ids);

ensure_no_commit_loop(
workspace_command.repo().as_ref(),
&new_children_expression,
&new_parents_expression,
)?;

let new_parents = after_commits;
let new_children = before_commits;

let (new_parents, new_children_with_parents) = move_revision_compute_parent_children(
workspace_command,
&old_commit,
&new_parents_expression,
new_parents,
new_children,
)?;

move_commit(
ui,
settings,
workspace_command,
&new_parents,
&new_children_with_parents,
old_commit,
)
}

type NewParentsAndChildren = (Vec<Commit>, Vec<(Commit, Vec<Commit>)>);

fn move_revision_compute_parent_children(
Expand Down
87 changes: 87 additions & 0 deletions cli/tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1321,6 +1321,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 6 commits
Working copy now at: lylxulpl cef45f4f f | f
Parent commit : kmkuslsw 0accb3c3 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 cef45f4f
◉ e kmkuslsw 0accb3c3
◉ c vruxwmqv 80233f8e
◉ d znkkpsqq f7326458
├─╮
│ ◉ b1 zsuskuln 072d5ae1
◉ │ b2 royxmykx 903ab0d6
├─╯
◉ 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 715fd8c6 f | f
Parent commit : znkkpsqq f7326458 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 50b57734
├─╮
│ @ f lylxulpl 715fd8c6
◉ │ c vruxwmqv 80233f8e
├─╯
◉ d znkkpsqq f7326458
├─╮
│ ◉ b1 zsuskuln 072d5ae1
◉ │ b2 royxmykx 903ab0d6
├─╯
◉ 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 f7326458ae0e would be both an ancestor and a descendant of the rebased commit
"###);
}

#[test]
fn test_rebase_skip_empty() {
let test_env = TestEnvironment::default();
Expand Down

0 comments on commit 8bf886c

Please sign in to comment.