Skip to content

Commit

Permalink
rebase: allow both --insert-after and --insert-before options to …
Browse files Browse the repository at this point in the history 
…be used simultaneously
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed Apr 26, 2024
1 parent 54ed28e commit 100a8c5
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 2 deletions.
54 changes: 52 additions & 2 deletions cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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)
Expand Down Expand Up @@ -169,6 +169,7 @@ pub(crate) struct RebaseArgs {
long,
short = 'A',
visible_alias = "after",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
Expand All @@ -181,6 +182,7 @@ pub(crate) struct RebaseArgs {
long,
short = 'B',
visible_alias = "before",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
Expand Down Expand Up @@ -239,7 +241,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_revision_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(
Expand Down Expand Up @@ -588,6 +603,41 @@ fn rebase_revisions_before(
)
}

fn rebase_revision_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(
ui,
settings,
workspace_command,
&new_parent_ids,
&new_children,
target_commits,
)
}

/// Extracts `target_commits` from the graph by rebasing its descendants onto
/// its parents. This assumes that `target_commits` can be rewritten.
/// `target_commits` should be in reverse topological order.
Expand Down
144 changes: 144 additions & 0 deletions cli/tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1849,6 +1849,150 @@ 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 setup_opid = test_env.current_operation_id(&repo_path);

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 1 commits onto destination
Rebased 3 descendant commits onto parent of rebased commits
Rebased 2 commits onto commits at destination
Working copy now at: lylxulpl 70c8b90c f | f
Parent commit : kmkuslsw 74fa3be9 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 70c8b90c
◉ e kmkuslsw 74fa3be9
◉ c vruxwmqv 962dda54
◉ d znkkpsqq f0349bcb
├─╮
│ ◉ 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 1 commits onto destination
Rebased 1 commits onto commits at destination
Working copy now at: lylxulpl cb9550d9 f | f
Parent commit : znkkpsqq f0349bcb 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 2349f0c3
├─╮
│ @ f lylxulpl cb9550d9
◉ │ c vruxwmqv 962dda54
├─╯
◉ d znkkpsqq f0349bcb
├─╮
│ ◉ b2 royxmykx 903ab0d6
◉ │ b1 zsuskuln 072d5ae1
├─╯
◉ a rlvkpnrz 2443ea76
◉ zzzzzzzz 00000000
"###);

// Test multiple revisions.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&[
"rebase", "-r", "c", "-r", "f", "-r", "e", "--before", "d", "--after", "a",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 3 commits onto destination
Rebased 1 commits onto commits at destination
Working copy now at: lylxulpl 41934576 f | f
Parent commit : rlvkpnrz 2443ea76 a | a
Added 0 files, modified 0 files, removed 3 files
"###);
insta::assert_snapshot!(get_long_log_output(&test_env, &repo_path), @r###"
◉ d znkkpsqq 4ecc060f
├─┬─╮
│ │ ◉ e kmkuslsw 40e71abb
│ │ ├─╮
│ │ │ @ f lylxulpl 41934576
│ │ ◉ │ c vruxwmqv 6a93c40e
│ │ ├─╯
│ ◉ │ b2 royxmykx 903ab0d6
│ ├─╯
◉ │ b1 zsuskuln 072d5ae1
├─╯
◉ a rlvkpnrz 2443ea76
◉ zzzzzzzz 00000000
"###);

// Should error if a loop will be created.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Working copy now at: lylxulpl 88f778c5 f | f
Parent commit : kmkuslsw 48dd9e3f e | e
Added 4 files, modified 0 files, removed 0 files
"###);
// Show 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 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 commit
"###);
}

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

0 comments on commit 100a8c5

Please sign in to comment.