Skip to content

Commit

Permalink
cmd: Allow multiple -s for jj rebase
Browse files Browse the repository at this point in the history 
This addresses a portion of #1158
  • Loading branch information
@ilyagr
ilyagr committed Feb 19, 2023
1 parent 9827418 commit b309527
Show file tree
Hide file tree
Showing 3 changed files with 184 additions and 15 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

### New features

* `jj rebase` now accepts multiple `-s` arguments. Revsets with multiple commits
are allowed with `--allow-large-revsets`.

### Fixed bugs

* Modify/delete conflicts now include context lines
Expand Down
56 changes: 41 additions & 15 deletions src/commands/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -731,9 +731,9 @@ struct RebaseArgs {
/// Rebase the whole branch (relative to destination's ancestors)
#[arg(long, short)]
branch: Option<RevisionArg>,
/// Rebase this revision and its descendants
/// Rebase this revision and its descendants (can be repeated)
#[arg(long, short)]
source: Option<RevisionArg>,
source: Vec<RevisionArg>,
/// Rebase only this revision, rebasing descendants onto this revision's
/// parent(s)
#[arg(long, short)]
Expand Down Expand Up @@ -2757,13 +2757,18 @@ fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result
.collect_vec();
if let Some(rev_str) = &args.revision {
rebase_revision(ui, command, &mut workspace_command, &new_parents, rev_str)?;
} else if let Some(source_str) = &args.source {
} else if !args.source.is_empty() {
let source_commits = resolve_mutliple_nonempty_revsets_flag_guarded(
&workspace_command,
&args.source,
args.allow_large_revsets,
)?;
rebase_descendants(
ui,
command,
&mut workspace_command,
&new_parents,
source_str,
&source_commits,
)?;
} else {
let branch_str = args.branch.as_deref().unwrap_or("@");
Expand Down Expand Up @@ -2820,18 +2825,39 @@ fn rebase_descendants(
command: &CommandHelper,
workspace_command: &mut WorkspaceCommandHelper,
new_parents: &[Commit],
source_str: &str,
old_commits: &IndexSet<Commit>,
) -> Result<(), CommandError> {
let old_commit = workspace_command.resolve_single_rev(source_str)?;
workspace_command.check_rewritable(&old_commit)?;
check_rebase_destinations(workspace_command.repo(), new_parents, &old_commit)?;
let mut tx = workspace_command.start_transaction(&format!(
"rebase commit {} and descendants",
old_commit.id().hex()
));
rebase_commit(command.settings(), tx.mut_repo(), &old_commit, new_parents)?;
let num_rebased = tx.mut_repo().rebase_descendants(command.settings())? + 1;
writeln!(ui, "Rebased {num_rebased} commits")?;
for old_commit in old_commits.iter() {
workspace_command.check_rewritable(old_commit)?;
check_rebase_destinations(workspace_command.repo(), new_parents, old_commit)?;
}
let tx_message = if old_commits.len() == 1 {
format!(
"rebase commit {} and descendants",
old_commits.first().unwrap().id().hex()
)
} else {
format!("rebase {} commits and their descendants", old_commits.len())
};
let mut tx = workspace_command.start_transaction(&tx_message);
let index = tx.base_repo().readonly_index().clone();
let store = tx.base_repo().store().clone();
for old_commit_id in index
// `rebase_descendants` seems to work even without a topo_order, but it feels
// safer to have it.
.topo_order(&mut old_commits.iter().map(|c| c.id()))
.into_iter()
.rev()
.map(|index_entry| index_entry.commit_id())
{
let old_commit = store.get_commit(&old_commit_id).unwrap();
rebase_commit(command.settings(), tx.mut_repo(), &old_commit, new_parents)?;
}
writeln!(
ui,
"Rebased {num_rebased} commits",
num_rebased = old_commits.len() + tx.mut_repo().rebase_descendants(command.settings())?
)?;
tx.finish(ui)?;
Ok(())
}
Expand Down
140 changes: 140 additions & 0 deletions tests/test_rebase_command.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -426,6 +426,146 @@ fn test_rebase_with_descendants() {
o a
o
"###);

test_env.jj_cmd_success(&repo_path, &["undo"]);
let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-s", "b", "-s", "d", "-d", "a"]);
insta::assert_snapshot!(stdout, @r###"
Rebased 3 commits
Working copy now at: 92c2bc9a8623 d
Added 0 files, modified 0 files, removed 2 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o c
o b
│ @ d
├─╯
o a
o
"###);
test_env.jj_cmd_success(&repo_path, &["undo"]);
let stderr =
test_env.jj_cmd_failure(&repo_path, &["rebase", "-s", "b|d", "-s", "d", "-d", "a"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "b|d" resolved to more than one revision
Hint: The revset "b|d" resolved to these revisions:
df54a9fd85ae d
d370aee184ba b
If this was intentional, specify the `--allow-large-revsets` argument
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"rebase",
"-s",
"b|d",
"-s",
"d",
"-d",
"a",
"--allow-large-revsets",
],
);
insta::assert_snapshot!(stdout, @r###"
Rebased 3 commits
Working copy now at: f1e71cb78a06 d
Added 0 files, modified 0 files, removed 2 files
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o c
o b
│ @ d
├─╯
o a
o
"###);

test_env.jj_cmd_success(&repo_path, &["undo"]);
create_commit(&test_env, &repo_path, "e", &[]);
// Test the setup
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
@ e
│ o d
│ o c
│ ├─╮
│ o │ b
├─╯ │
│ o a
├───╯
o
"###);

// TODO: Simple case, will be deleted
let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-s", "a", "-s", "b", "-d", "e"]);
insta::assert_snapshot!(stdout, @r###"
Rebased 4 commits
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o d
o c
├─╮
o │ a
│ o b
├─╯
@ e
o
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&["rebase", "-s", "a", "-s", "c", "-s", "d", "-d", "e"],
);
insta::assert_snapshot!(stdout, @r###"
Rebased 3 commits
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o a
│ o c
├─╯
│ o d
├─╯
│ o b
├─╯
@ e
o
"###);

test_env.jj_cmd_success(&repo_path, &["undo"]);
let stderr =
test_env.jj_cmd_failure(&repo_path, &["rebase", "-s", "a|c|d", "-s", "d", "-d", "e"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "a|c|d" resolved to more than one revision
Hint: The revset "a|c|d" resolved to these revisions:
e1a883ee648c d
a3bab23dae21 c
2d49d96057da a
If this was intentional, specify the `--allow-large-revsets` argument
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"rebase",
"-s",
"a|c|d",
"-s",
"d",
"-d",
"e",
"--allow-large-revsets",
],
);
insta::assert_snapshot!(stdout, @r###"
Rebased 3 commits
"###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o a
│ o c
├─╯
│ o d
├─╯
│ o b
├─╯
@ e
o
"###);
}

fn get_log_output(test_env: &TestEnvironment, repo_path: &Path) -> String {
Expand Down

0 comments on commit b309527

Please sign in to comment.