Skip to content

Commit

Permalink
rebase -s: add support for --insert-after and --insert-before opt…
Browse files Browse the repository at this point in the history 
…ions
  • Loading branch information
@bnjmnt4n
bnjmnt4n committed May 1, 2024
1 parent 49e797b commit cac8365
Show file tree
Hide file tree
Showing 2 changed files with 200 additions and 46 deletions.
82 changes: 59 additions & 23 deletions cli/src/commands/rebase.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,26 +162,24 @@ pub(crate) struct RebaseArgs {
/// The revision(s) to insert after (can be repeated to create a merge
/// commit)
///
/// Only works with `-r`.
/// Only works with `-r` and `-s`.
#[arg(
long,
short = 'A',
visible_alias = "after",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
insert_after: Vec<RevisionArg>,
/// The revision(s) to insert before (can be repeated to create a merge
/// commit)
///
/// Only works with `-r`.
/// Only works with `-r` and `-s`.
#[arg(
long,
short = 'B',
visible_alias = "before",
conflicts_with = "destination",
conflicts_with = "source",
conflicts_with = "branch"
)]
insert_before: Vec<RevisionArg>,
Expand Down Expand Up @@ -247,17 +245,14 @@ Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets
&rebase_options,
)?;
} else if !args.source.is_empty() {
let new_parents = workspace_command
.resolve_some_revsets_default_single(&args.destination)?
.into_iter()
.collect_vec();
let source_commits = workspace_command.resolve_some_revsets_default_single(&args.source)?;
rebase_descendants_transaction(
rebase_source(
ui,
command.settings(),
&mut workspace_command,
new_parents,
&source_commits,
&args.source,
&args.destination,
&args.insert_after,
&args.insert_before,
&rebase_options,
)?;
} else {
Expand Down Expand Up @@ -318,6 +313,47 @@ fn rebase_revisions(
)
}

fn rebase_source(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
source: &[RevisionArg],
destination: &[RevisionArg],
insert_after: &[RevisionArg],
insert_before: &[RevisionArg],
rebase_options: &RebaseOptions,
) -> Result<(), CommandError> {
let source_commits = workspace_command
.resolve_some_revsets_default_single(source)?
.into_iter()
.collect_vec();
workspace_command.check_rewritable(source_commits.iter().ids())?;

let (new_parents, new_children) = compute_destination(
workspace_command,
&source_commits,
destination,
insert_after,
insert_before,
true,
)?;
if !new_children.is_empty() {
for commit in source_commits.iter() {
check_rebase_destinations(workspace_command.repo(), &new_parents, commit)?;
}
}

rebase_descendants_transaction(
ui,
settings,
workspace_command,
&new_parents.iter().ids().cloned().collect_vec(),
&new_children,
&source_commits,
rebase_options,
)
}

fn rebase_branch(
ui: &mut Ui,
settings: &UserSettings,
Expand All @@ -337,17 +373,23 @@ fn rebase_branch(
let roots_expression = RevsetExpression::commits(parent_ids)
.range(&RevsetExpression::commits(branch_commit_ids))
.roots();
let root_commits: IndexSet<_> = roots_expression
let root_commits: Vec<_> = roots_expression
.evaluate_programmatic(workspace_command.repo().as_ref())
.unwrap()
.iter()
.commits(workspace_command.repo().store())
.try_collect()?;
workspace_command.check_rewritable(root_commits.iter().ids())?;
for commit in root_commits.iter() {
check_rebase_destinations(workspace_command.repo(), &new_parents, commit)?;
}

rebase_descendants_transaction(
ui,
settings,
workspace_command,
new_parents,
&new_parents.iter().ids().cloned().collect_vec(),
&[],
&root_commits,
&rebase_options,
)
Expand All @@ -357,19 +399,15 @@ fn rebase_descendants_transaction(
ui: &mut Ui,
settings: &UserSettings,
workspace_command: &mut WorkspaceCommandHelper,
new_parents: Vec<Commit>,
target_roots: &IndexSet<Commit>,
new_parent_ids: &[CommitId],
new_children: &[Commit],
target_roots: &[Commit],
rebase_options: &RebaseOptions,
) -> Result<(), CommandError> {
if target_roots.is_empty() {
return Ok(());
}

workspace_command.check_rewritable(target_roots.iter().ids())?;
for commit in target_roots.iter() {
check_rebase_destinations(workspace_command.repo(), &new_parents, commit)?;
}

let mut tx = workspace_command.start_transaction();
let tx_description = if target_roots.len() == 1 {
format!(
Expand All @@ -390,8 +428,6 @@ fn rebase_descendants_transaction(
.iter()
.commits(tx.repo().store())
.try_collect()?;
let new_parent_ids = new_parents.iter().ids().cloned().collect_vec();
let new_children: [Commit; 0] = [];
let target_roots = target_roots.iter().ids().cloned().collect_vec();

let MoveCommitsStats {
Expand Down
164 changes: 141 additions & 23 deletions cli/tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,6 @@ fn test_rebase_invalid() {
For more information, try '--help'.
"###);

// -s with --after
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-s", "a", "--after", "b"]);
insta::assert_snapshot!(stderr, @r###"
error: the argument '--source <SOURCE>' cannot be used with '--insert-after <INSERT_AFTER>'
Usage: jj rebase --source <SOURCE> <--destination <DESTINATION>|--insert-after <INSERT_AFTER>|--insert-before <INSERT_BEFORE>>
For more information, try '--help'.
"###);

// -b with --after
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-b", "a", "--after", "b"]);
insta::assert_snapshot!(stderr, @r###"
Expand All @@ -129,16 +119,6 @@ fn test_rebase_invalid() {
For more information, try '--help'.
"###);

// -s with --before
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-s", "a", "--before", "b"]);
insta::assert_snapshot!(stderr, @r###"
error: the argument '--source <SOURCE>' cannot be used with '--insert-before <INSERT_BEFORE>'
Usage: jj rebase --source <SOURCE> <--destination <DESTINATION>|--insert-after <INSERT_AFTER>|--insert-before <INSERT_BEFORE>>
For more information, try '--help'.
"###);

// -b with --before
let stderr = test_env.jj_cmd_cli_error(&repo_path, &["rebase", "-b", "a", "--before", "b"]);
insta::assert_snapshot!(stderr, @r###"
Expand Down Expand Up @@ -1298,7 +1278,7 @@ fn test_rebase_with_child_and_descendant_bug_2600() {
}

#[test]
fn test_rebase_revisions_after() {
fn test_rebase_after() {
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");
Expand Down Expand Up @@ -1699,6 +1679,61 @@ fn test_rebase_revisions_after() {
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);

// `rebase -s` of commit "c" and its descendants after itself should be a no-op.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "c", "--after", "c"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Skipped rebase of 4 commits that were already in place
Nothing changed.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ f
◉ e
│ ◉ d
├─╯
◉ c
├─╮
│ ◉ b4
│ ◉ b3
◉ │ b2
◉ │ b1
├─╯
◉ a
◉
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);

// `rebase -s` of a commit and its descendants after multiple commits.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-s", "c", "--after", "b1", "--after", "b3"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: xznxytkn a4ace41c f | f
Parent commit : nkmrtpmo c7744d08 e | e
Added 0 files, modified 0 files, removed 2 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ b4
├─╮
│ │ ◉ b2
╭─┬─╯
│ @ f
│ ◉ e
◉ │ d
├─╯
◉ c
├─╮
│ ◉ b3
◉ │ b1
├─╯
◉ a
◉
"###);
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,
Expand All @@ -1710,7 +1745,7 @@ fn test_rebase_revisions_after() {
}

#[test]
fn test_rebase_revisions_before() {
fn test_rebase_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");
Expand Down Expand Up @@ -2120,6 +2155,62 @@ fn test_rebase_revisions_before() {
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);

// `rebase -s` of commit "c" and its descendants before itself should be a
// no-op.
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-s", "c", "--before", "c"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Skipped rebase of 4 commits that were already in place
Nothing changed.
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ f
◉ e
│ ◉ d
├─╯
◉ c
├─╮
│ ◉ b4
│ ◉ b3
◉ │ b2
◉ │ b1
├─╯
◉ a
◉
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);

// `rebase -s` of a commit and its descendants before multiple commits.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-s", "c", "--before", "b2", "--before", "b4"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: xznxytkn 1ca449cc f | f
Parent commit : nkmrtpmo c75237e2 e | e
Added 0 files, modified 0 files, removed 2 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ b4
├─╮
│ │ ◉ b2
╭─┬─╯
│ @ f
│ ◉ e
◉ │ d
├─╯
◉ c
├─╮
│ ◉ b3
◉ │ b1
├─╯
◉ a
◉
"###);
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,
Expand All @@ -2131,7 +2222,7 @@ fn test_rebase_revisions_before() {
}

#[test]
fn test_rebase_revisions_after_before() {
fn test_rebase_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");
Expand Down Expand Up @@ -2286,6 +2377,33 @@ fn test_rebase_revisions_after_before() {
"###);
test_env.jj_cmd_ok(&repo_path, &["op", "restore", &setup_opid]);

// `rebase -s` of a commit and its descendants.
let (stdout, stderr) = test_env.jj_cmd_ok(
&repo_path,
&["rebase", "-s", "c", "--before", "b1", "--after", "b2"],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 4 commits
Working copy now at: lylxulpl 108f0202 f | f
Parent commit : kmkuslsw 52245d71 e | e
Added 0 files, modified 0 files, removed 1 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ b1
├─┬─╮
│ │ @ f
│ │ ◉ e
│ ◉ │ d
│ ├─╯
│ ◉ c
│ ◉ b2
├─╯
◉ a
◉
"###);
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,
Expand Down

0 comments on commit cac8365

Please sign in to comment.