ok/jj
1
0
Fork 0
forked from mirrors/jj
Code Pull requests Activity

rebase/new: make --allow-large-revset no longer also allow duplicates

Browse source 
The `--allow-large-revset` option for `jj rebase` and `jj new` is used
for allowing a single revset to resolve to more than one destination
commit. It also means that duplicate commits between individual
revsets are allowed (e.g. `jj rebase -d x -d 'x|y'`). I'm about to
replace the first meaning of the flag by a revset function. I don't
think it's worth keeping the flag only for the second meaning, so I'm
just removing the feature instead. We can add it back under a
different name (`--allow-duplicate-destinations`?) if people care
about it.
This commit is contained in:
Martin von Zweigbergk 2023-07-25 10:17:36 -07:00 committed by Martin von Zweigbergk
parent c183b94aef
commit aaf5df95a8
3 changed files with 30 additions and 47 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
CHANGELOG.md
View file

@ -17,6 +17,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* The `:` revset operator is deprecated. Use `::` instead. We plan to delete the
`:` form in jj 0.15+.
* The `--allow-large-revsets` flag for `jj rebase` and `jj new` can no longer be
used for allowing duplicate destinations. Include the potential duplicates
in a single expression instead (e.g. `jj new --allow-large-revsets 'x|y'`).
### New features
* `jj config edit --user` and `jj config set --user` will now pick a default

23
src/cli_util.rs
View file

@ -1845,23 +1845,32 @@ pub fn resolve_multiple_nonempty_revsets_flag_guarded(
revisions: &[RevisionArg],
allow_plural_revsets: bool,
) -> Result<IndexSet<Commit>, CommandError> {
if allow_plural_revsets {
resolve_multiple_nonempty_revsets(revisions, workspace_command, ui)
} else {
let mut commits = IndexSet::new();
for revision_str in revisions {
let mut all_commits = IndexSet::new();
for revision_str in revisions {
if allow_plural_revsets {
let commits = workspace_command.resolve_revset(revision_str, ui)?;
workspace_command.check_non_empty(&commits)?;
for commit in commits {
let commit_hash = short_commit_hash(commit.id());
if !all_commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
}
} else {
let commit = workspace_command
.resolve_single_rev(revision_str, ui)
.map_err(append_large_revsets_hint_if_multiple_revisions)?;
let commit_hash = short_commit_hash(commit.id());
if !commits.insert(commit) {
if !all_commits.insert(commit) {
return Err(user_error(format!(
r#"More than one revset resolved to revision {commit_hash}"#,
)));
}
}
Ok(commits)
}
Ok(all_commits)
}
fn append_large_revsets_hint_if_multiple_revisions(err: CommandError) -> CommandError {

50
tests/test_rebase_command.rs
View file

@ -142,10 +142,9 @@ fn test_rebase_branch() {
◉
"###);
// Same test but with more than one revision per argument and same revision
// repeated in more than one argument
// Same test but with more than one revision per argument
test_env.jj_cmd_success(&repo_path, &["undo"]);
let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-b=e|d", "-b=d", "-d=b"]);
let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-b=e|d", "-d=b"]);
insta::assert_snapshot!(stderr, @r###"
Error: Revset "e|d" resolved to more than one revision
Hint: The revset "e|d" resolved to these revisions:
@ -155,7 +154,7 @@ fn test_rebase_branch() {
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&["rebase", "-b=e|d", "-b=d", "-d=b", "--allow-large-revsets"],
&["rebase", "-b=e|d", "-d=b", "--allow-large-revsets"],
);
insta::assert_snapshot!(stdout, @r###"
Rebased 2 commits
@ -399,30 +398,8 @@ fn test_rebase_multiple_destinations() {
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###"
◉ a
◉ b
│ @ c
├─╯
◉
"###);
let stdout = test_env.jj_cmd_success(
// Same error with --allow-large-revsets if there is overlap.
let stderr = test_env.jj_cmd_failure(
&repo_path,
&[
"rebase",
@ -435,14 +412,8 @@ fn test_rebase_multiple_destinations() {
"--allow-large-revsets",
],
);
insta::assert_snapshot!(stdout, @"");
insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###"
◉ a
├─╮
│ ◉ b
@ │ c
├─╯
◉
insta::assert_snapshot!(stderr, @r###"
Error: More than one revset resolved to revision d370aee184ba
"###);
let stderr =
@ -536,10 +507,9 @@ fn test_rebase_with_descendants() {
◉
"###);
// Same test as above, but with duplicate commits and multiple commits per
// argument
// Same test as above, but with multiple commits per argument
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"]);
let stderr = test_env.jj_cmd_failure(&repo_path, &["rebase", "-s=b|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:
@ -549,7 +519,7 @@ fn test_rebase_with_descendants() {
"###);
let stdout = test_env.jj_cmd_success(
&repo_path,
&["rebase", "-s=b|d", "-s=d", "-d=a", "--allow-large-revsets"],
&["rebase", "-s=b|d", "-d=a", "--allow-large-revsets"],
);
insta::assert_snapshot!(stdout, @r###"
Rebased 3 commits