Skip to content

Commit

Permalink
jj rebase -d and jj new: Allow several commits per argument with …
Browse files Browse the repository at this point in the history
…`--allow-large-revsets`

Eventually, we should be able to rely on `jj op restore` and the `--allow-large-revsets`.
argument should likely be removed. This is a temporary measure until we figure
out #922 and the like.

Fixes #571
  • Loading branch information
ilyagr committed Feb 5, 2023
1 parent e13eaab commit 691bd66
Show file tree
Hide file tree
Showing 4 changed files with 99 additions and 14 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
and `jj split any-non-existent-path` inserts an empty commit between the
target commit and its parents.

* `jj rebase` now accepts a new `--allow-large-revsets` argument that allows the
revset in the `-d` argument to expand to several revisions. For example,
`jj rebase -s B -d B- -d C` now works even if `B` is a merge commit.

* `jj new` now also accepts a `--allow-large-revsets` argument that behaves
similarly to `jj rebase --allow-large-revsets`.

### Fixed bugs

* When sharing the working copy with a Git repo, we used to forget to export
Expand Down
19 changes: 12 additions & 7 deletions src/cli_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1412,15 +1412,20 @@ pub fn resolve_multiple_nonempty_revsets(
pub fn resolve_base_revs(
workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg],
allow_plural_revsets: bool,
) -> Result<IndexSet<Commit>, CommandError> {
let mut commits = IndexSet::new();
for revision_str in revisions {
let commit = workspace_command.resolve_single_rev(revision_str)?;
let commit_hash = short_commit_hash(commit.id());
if !commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
if allow_plural_revsets {
commits = resolve_multiple_nonempty_revsets(revisions, workspace_command)?;
} else {
for revision_str in revisions {
let commit = workspace_command.resolve_single_rev(revision_str)?;
let commit_hash = short_commit_hash(commit.id());
if !commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
}
}

Expand Down
28 changes: 21 additions & 7 deletions src/commands/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,9 @@ struct NewArgs {
/// The change description to use
#[arg(long, short, default_value = "")]
message: DescriptionArg,
/// Allow revsets expanding to multiple commits in a single argument
#[arg(long, short = 'L')]
allow_large_revsets: bool,
}

/// Move changes from one revision into another
Expand Down Expand Up @@ -731,6 +734,9 @@ struct RebaseArgs {
/// The revision(s) to rebase onto
#[arg(long, short, required = true)]
destination: Vec<RevisionArg>,
/// Allow revsets expanding to multiple commits in a single argument
#[arg(long, short = 'L')]
allow_large_revsets: bool,
}

/// Apply the reverse of a revision on top of another revision
Expand Down Expand Up @@ -1996,9 +2002,13 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
!args.revisions.is_empty(),
"expected a non-empty list from clap"
);
let commits = resolve_base_revs(&workspace_command, &args.revisions)?
.into_iter()
.collect_vec();
let commits = resolve_base_revs(
&workspace_command,
&args.revisions,
args.allow_large_revsets,
)?
.into_iter()
.collect_vec();
let parent_ids = commits.iter().map(|c| c.id().clone()).collect();
let mut tx = workspace_command.start_transaction("new empty commit");
let merged_tree = merge_commit_trees(tx.base_repo().as_repo_ref(), &commits);
Expand Down Expand Up @@ -2677,7 +2687,7 @@ don't make any changes, then the operation will be aborted.
}

fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), CommandError> {
if args.revisions.len() < 2 {
if !args.allow_large_revsets && args.revisions.len() < 2 {
return Err(CommandError::CliError(String::from(
"Merge requires at least two revisions",
)));
Expand All @@ -2687,9 +2697,13 @@ fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(),

fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = resolve_base_revs(&workspace_command, &args.destination)?
.into_iter()
.collect_vec();
let new_parents = resolve_base_revs(
&workspace_command,
&args.destination,
args.allow_large_revsets,
)?
.into_iter()
.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 {
Expand Down
59 changes: 59 additions & 0 deletions tests/test_rebase_command.rs
Original file line number Diff line number Diff line change
Expand Up @@ -321,12 +321,71 @@ fn test_rebase_multiple_destinations() {
fe2e8e8b50b3 c
d370aee184ba b
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&["rebase", "--allow-large-revsets", "-r", "a", "-d", "b|c"],
);
insta::assert_snapshot!(stdout, @r###""###);
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o a
|\
@ | c
| o b
|/
o
"###);

let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b", "-d", "b"]);
insta::assert_snapshot!(stderr, @r###"
Error: More than one revset resolved to revision d370aee184ba
"###);

// Adding --allow-large-revsets suppresses this error in addition to the large
// revsets error.
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"rebase",
"-r",
"a",
"-d",
"b",
"-d",
"b",
"--allow-large-revsets",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o a
| @ c
o | b
|/
o
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&[
"rebase",
"-r",
"a",
"-d",
"b|c",
"-d",
"b",
"--allow-large-revsets",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
o a
|\
@ | c
| o b
|/
o
"###);

let stderr =
test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b", "-d", "root"]);
insta::assert_snapshot!(stderr, @r###"
Expand Down

0 comments on commit 691bd66

Please sign in to comment.