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 Apr 2, 2024
1 parent 61e5c3e commit f59623b
Show file tree
Hide file tree
Showing 2 changed files with 141 additions and 3 deletions.
55 changes: 53 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", "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 @@ -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 @@ -244,7 +246,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_str,
)?;
} else if !args.insert_after.is_empty() {
let after_commits =
workspace_command.resolve_some_revsets_default_single(&args.insert_after)?;
rebase_revision_after(
Expand Down Expand Up @@ -565,6 +580,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_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.iter())?;

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;

move_commit(
ui,
settings,
workspace_command,
new_parents,
new_children,
old_commit,
)
}

/// Extracts `commit` from the graph by rebasing its children on its parents.
/// This assumes that it can be rewritten.
fn extract_commit(
Expand Down
89 changes: 88 additions & 1 deletion cli/tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1250,7 +1250,7 @@ fn test_rebase_revision_before() {
let (stdout, stderr) = test_env.jj_cmd_ok(&repo_path, &["rebase", "-r", "b2", "--before", "a"]);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(stderr, @r###"
Rebased 11 commits
Rebased 7 commits
Working copy now at: lylxulpl de47a5a1 f | f
Parent commit : kmkuslsw 838e25e4 e | e
"###);
Expand Down Expand Up @@ -1322,6 +1322,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 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 f59623b

Please sign in to comment.