jj rebase -d and jj new: Allow several commits per argument with --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 https://github.com/martinvonz/jj/issues/922 and the like.

Fixes https://github.com/martinvonz/jj/issues/571
This commit is contained in:
Ilya Grigoriev 2023-01-30 22:32:18 -08:00
parent d153ced8ba
commit 1c6c6dbccc
4 changed files with 99 additions and 14 deletions

View file

@ -120,6 +120,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* Command arguments to `ui.diff-editor`/`ui.merge-editor` can now be specified * Command arguments to `ui.diff-editor`/`ui.merge-editor` can now be specified
inline without referring to `[merge-tools]` table. inline without referring to `[merge-tools]` table.
* `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 ### Fixed bugs
* When sharing the working copy with a Git repo, we used to forget to export * When sharing the working copy with a Git repo, we used to forget to export

View file

@ -1412,15 +1412,20 @@ pub fn resolve_multiple_nonempty_revsets(
pub fn resolve_base_revs( pub fn resolve_base_revs(
workspace_command: &WorkspaceCommandHelper, workspace_command: &WorkspaceCommandHelper,
revisions: &[RevisionArg], revisions: &[RevisionArg],
allow_plural_revsets: bool,
) -> Result<IndexSet<Commit>, CommandError> { ) -> Result<IndexSet<Commit>, CommandError> {
let mut commits = IndexSet::new(); let mut commits = IndexSet::new();
for revision_str in revisions { if allow_plural_revsets {
let commit = workspace_command.resolve_single_rev(revision_str)?; commits = resolve_multiple_nonempty_revsets(revisions, workspace_command)?;
let commit_hash = short_commit_hash(commit.id()); } else {
if !commits.insert(commit) { for revision_str in revisions {
return Err(user_error(format!( let commit = workspace_command.resolve_single_rev(revision_str)?;
r#"More than one revset resolved to revision {commit_hash}"#, 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}"#,
)));
}
} }
} }

View file

@ -456,6 +456,9 @@ struct NewArgs {
/// The change description to use /// The change description to use
#[arg(long, short, default_value = "")] #[arg(long, short, default_value = "")]
message: DescriptionArg, 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 /// Move changes from one revision into another
@ -731,6 +734,9 @@ struct RebaseArgs {
/// The revision(s) to rebase onto /// The revision(s) to rebase onto
#[arg(long, short, required = true)] #[arg(long, short, required = true)]
destination: Vec<RevisionArg>, 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 /// Apply the reverse of a revision on top of another revision
@ -1997,9 +2003,13 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
!args.revisions.is_empty(), !args.revisions.is_empty(),
"expected a non-empty list from clap" "expected a non-empty list from clap"
); );
let commits = resolve_base_revs(&workspace_command, &args.revisions)? let commits = resolve_base_revs(
.into_iter() &workspace_command,
.collect_vec(); &args.revisions,
args.allow_large_revsets,
)?
.into_iter()
.collect_vec();
let parent_ids = commits.iter().map(|c| c.id().clone()).collect(); let parent_ids = commits.iter().map(|c| c.id().clone()).collect();
let mut tx = workspace_command.start_transaction("new empty commit"); let mut tx = workspace_command.start_transaction("new empty commit");
let merged_tree = merge_commit_trees(tx.base_repo().as_repo_ref(), &commits); let merged_tree = merge_commit_trees(tx.base_repo().as_repo_ref(), &commits);
@ -2678,7 +2688,7 @@ don't make any changes, then the operation will be aborted.
} }
fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), CommandError> { 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( return Err(CommandError::CliError(String::from(
"Merge requires at least two revisions", "Merge requires at least two revisions",
))); )));
@ -2688,9 +2698,13 @@ fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(),
fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result<(), CommandError> { fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result<(), CommandError> {
let mut workspace_command = command.workspace_helper(ui)?; let mut workspace_command = command.workspace_helper(ui)?;
let new_parents = resolve_base_revs(&workspace_command, &args.destination)? let new_parents = resolve_base_revs(
.into_iter() &workspace_command,
.collect_vec(); &args.destination,
args.allow_large_revsets,
)?
.into_iter()
.collect_vec();
if let Some(rev_str) = &args.revision { if let Some(rev_str) = &args.revision {
rebase_revision(ui, command, &mut workspace_command, &new_parents, rev_str)?; rebase_revision(ui, command, &mut workspace_command, &new_parents, rev_str)?;
} else if let Some(source_str) = &args.source { } else if let Some(source_str) = &args.source {

View file

@ -321,12 +321,71 @@ fn test_rebase_multiple_destinations() {
fe2e8e8b50b3 c fe2e8e8b50b3 c
d370aee184ba b 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"]); let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b", "-d", "b"]);
insta::assert_snapshot!(stderr, @r###" insta::assert_snapshot!(stderr, @r###"
Error: More than one revset resolved to revision d370aee184ba 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 = let stderr =
test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b", "-d", "root"]); test_env.jj_cmd_failure(&repo_path, &["rebase", "-r", "a", "-d", "b", "-d", "root"]);
insta::assert_snapshot!(stderr, @r###" insta::assert_snapshot!(stderr, @r###"