diff --git a/CHANGELOG.md b/CHANGELOG.md index 23b69f168f..ac1a2d72b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,9 +16,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * The `:` revset operator is deprecated. Use `::` instead. +* The `--allow-large-revsets` flag for `jj rebase` and `jj new` was replaced by + a `all:` before the revset. For example, use `jj rebase -d 'all:foo-'` + instead of `jj rebase --allow-large-revsets -d 'foo-'`. + * 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'`). + in a single expression instead (e.g. `jj new 'all:x|y'`). ### New features diff --git a/src/cli_util.rs b/src/cli_util.rs index 00f0a8e537..495dc95bd5 100644 --- a/src/cli_util.rs +++ b/src/cli_util.rs @@ -945,6 +945,8 @@ impl WorkspaceCommandHelper { ) } + /// Resolve a revset to a single revision. Return an error if the revset is + /// empty or has multiple revisions. pub fn resolve_single_rev( &self, revision_str: &str, @@ -979,6 +981,7 @@ impl WorkspaceCommandHelper { } } + /// Resolve a revset any number of revisions (including 0). pub fn resolve_revset( &self, revision_str: &str, @@ -989,6 +992,34 @@ impl WorkspaceCommandHelper { Ok(revset.iter().commits(self.repo().store()).try_collect()?) } + /// Resolve a revset any number of revisions (including 0), but require the + /// user to indicate if they allow multiple revisions by prefixing the + /// expression with `all:`. + pub fn resolve_revset_default_single( + &self, + revision_str: &str, + ui: &mut Ui, + ) -> Result, CommandError> { + if let Some(revision_str) = revision_str.strip_prefix("all:") { + self.resolve_revset(revision_str, ui) + } else { + self.resolve_single_rev(revision_str, ui) + .map_err(|err| match err { + CommandError::UserError { message, hint } => user_error_with_hint( + message, + format!( + "{old_hint}Prefix the expression with 'all' to allow any number of \ + revisions (i.e. 'all:{}').", + revision_str, + old_hint = hint.map(|hint| format!("{hint}\n")).unwrap_or_default() + ), + ), + err => err, + }) + .map(|commit| vec![commit]) + } + } + pub fn parse_revset( &self, revision_str: &str, @@ -1826,29 +1857,16 @@ pub fn resolve_multiple_nonempty_revsets( Ok(acc) } -pub fn resolve_multiple_nonempty_revsets_flag_guarded( +pub fn resolve_multiple_nonempty_revsets_default_single( workspace_command: &WorkspaceCommandHelper, ui: &mut Ui, revisions: &[RevisionArg], - allow_plural_revsets: bool, ) -> Result, CommandError> { 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 commits = workspace_command.resolve_revset_default_single(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!( @@ -1860,24 +1878,6 @@ pub fn resolve_multiple_nonempty_revsets_flag_guarded( Ok(all_commits) } -fn append_large_revsets_hint_if_multiple_revisions(err: CommandError) -> CommandError { - match err { - CommandError::UserError { message, hint } if message.contains("more than one revision") => { - CommandError::UserError { - message, - hint: { - Some(format!( - "{old_hint}If this was intentional, specify the `--allow-large-revsets` \ - argument", - old_hint = hint.map(|h| format!("{h}\n")).unwrap_or_default() - )) - }, - } - } - _ => err, - } -} - pub fn update_working_copy( repo: &Arc, wc: &mut WorkingCopy, diff --git a/src/commands/mod.rs b/src/commands/mod.rs index cfa2b298bb..fb0bdd27ae 100644 --- a/src/commands/mod.rs +++ b/src/commands/mod.rs @@ -56,7 +56,7 @@ use maplit::{hashmap, hashset}; use crate::cli_util::{ check_stale_working_copy, get_new_config_file_path, print_checkout_stats, - resolve_multiple_nonempty_revsets, resolve_multiple_nonempty_revsets_flag_guarded, + resolve_multiple_nonempty_revsets, resolve_multiple_nonempty_revsets_default_single, run_ui_editor, serialize_config_value, short_commit_hash, user_error, user_error_with_hint, write_config_value_to_file, Args, CommandError, CommandHelper, DescriptionArg, LogContentFormat, RevisionArg, WorkspaceCommandHelper, @@ -2160,14 +2160,9 @@ fn resolve_destination_revs( workspace_command: &WorkspaceCommandHelper, ui: &mut Ui, revisions: &[RevisionArg], - allow_plural_revsets: bool, ) -> Result, CommandError> { - let commits = resolve_multiple_nonempty_revsets_flag_guarded( - workspace_command, - ui, - revisions, - allow_plural_revsets, - )?; + let commits = + resolve_multiple_nonempty_revsets_default_single(workspace_command, ui, revisions)?; let root_commit_id = workspace_command.repo().store().root_commit_id(); if commits.len() >= 2 && commits.iter().any(|c| c.id() == root_commit_id) { Err(user_error("Cannot merge with root revision")) @@ -2177,19 +2172,20 @@ fn resolve_destination_revs( } fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), CommandError> { + if args.allow_large_revsets { + return Err(user_error( + "--allow-large-revsets has been deprecated. +Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", + )); + } let mut workspace_command = command.workspace_helper(ui)?; assert!( !args.revisions.is_empty(), "expected a non-empty list from clap" ); - let target_commits = resolve_destination_revs( - &workspace_command, - ui, - &args.revisions, - args.allow_large_revsets, - )? - .into_iter() - .collect_vec(); + let target_commits = resolve_destination_revs(&workspace_command, ui, &args.revisions)? + .into_iter() + .collect_vec(); let target_ids = target_commits.iter().map(|c| c.id().clone()).collect_vec(); let mut tx = workspace_command.start_transaction("new empty commit"); let mut num_rebased = 0; @@ -3069,7 +3065,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.allow_large_revsets && args.revisions.len() < 2 { + if args.revisions.len() < 2 { return Err(CommandError::CliError(String::from( "Merge requires at least two revisions", ))); @@ -3078,24 +3074,21 @@ fn cmd_merge(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), } fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result<(), CommandError> { + if args.allow_large_revsets { + return Err(user_error( + "--allow-large-revsets has been deprecated. +Please use `jj rebase -d 'all:x|y'` instead of `jj rebase --allow-large-revsets -d x -d y`.", + )); + } let mut workspace_command = command.workspace_helper(ui)?; - let new_parents = resolve_destination_revs( - &workspace_command, - ui, - &args.destination, - args.allow_large_revsets, - )? - .into_iter() - .collect_vec(); + let new_parents = resolve_destination_revs(&workspace_command, ui, &args.destination)? + .into_iter() + .collect_vec(); if let Some(rev_str) = &args.revision { rebase_revision(ui, command, &mut workspace_command, &new_parents, rev_str)?; } else if !args.source.is_empty() { - let source_commits = resolve_multiple_nonempty_revsets_flag_guarded( - &workspace_command, - ui, - &args.source, - args.allow_large_revsets, - )?; + let source_commits = + resolve_multiple_nonempty_revsets_default_single(&workspace_command, ui, &args.source)?; rebase_descendants( ui, command, @@ -3107,12 +3100,7 @@ fn cmd_rebase(ui: &mut Ui, command: &CommandHelper, args: &RebaseArgs) -> Result let branch_commits = if args.branch.is_empty() { IndexSet::from([workspace_command.resolve_single_rev("@", ui)?]) } else { - resolve_multiple_nonempty_revsets_flag_guarded( - &workspace_command, - ui, - &args.branch, - args.allow_large_revsets, - )? + resolve_multiple_nonempty_revsets_default_single(&workspace_command, ui, &args.branch)? }; rebase_branch( ui, diff --git a/tests/test_rebase_command.rs b/tests/test_rebase_command.rs index 62d766d0d4..f2203d4926 100644 --- a/tests/test_rebase_command.rs +++ b/tests/test_rebase_command.rs @@ -150,12 +150,9 @@ fn test_rebase_branch() { Hint: The revset "e|d" resolved to these revisions: e52756c82985 e 514fa6b265d4 d - If this was intentional, specify the `--allow-large-revsets` argument + Prefix the expression with 'all' to allow any number of revisions (i.e. 'all:e|d'). "###); - let stdout = test_env.jj_cmd_success( - &repo_path, - &["rebase", "-b=e|d", "-d=b", "--allow-large-revsets"], - ); + let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-b=all:e|d", "-d=b"]); insta::assert_snapshot!(stdout, @r###" Rebased 2 commits Working copy now at: 817e3fb0dc64 e @@ -377,12 +374,9 @@ fn test_rebase_multiple_destinations() { Hint: The revset "b|c" resolved to these revisions: fe2e8e8b50b3 c d370aee184ba b - If this was intentional, specify the `--allow-large-revsets` argument + Prefix the expression with 'all' to allow any number of revisions (i.e. 'all:b|c'). "###); - let stdout = test_env.jj_cmd_success( - &repo_path, - &["rebase", "--allow-large-revsets", "-r", "a", "-d", "b|c"], - ); + let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-r", "a", "-d", "all:b|c"]); insta::assert_snapshot!(stdout, @r###""###); insta::assert_snapshot!(get_log_output(&test_env, &repo_path), @r###" ◉ a @@ -398,19 +392,10 @@ fn test_rebase_multiple_destinations() { Error: More than one revset resolved to revision d370aee184ba "###); - // Same error with --allow-large-revsets if there is overlap. + // Same error with 'all:' if there is overlap. let stderr = test_env.jj_cmd_failure( &repo_path, - &[ - "rebase", - "-r", - "a", - "-d", - "b|c", - "-d", - "b", - "--allow-large-revsets", - ], + &["rebase", "-r", "a", "-d", "all:b|c", "-d", "b"], ); insta::assert_snapshot!(stderr, @r###" Error: More than one revset resolved to revision d370aee184ba @@ -515,12 +500,9 @@ fn test_rebase_with_descendants() { Hint: The revset "b|d" resolved to these revisions: df54a9fd85ae d d370aee184ba b - If this was intentional, specify the `--allow-large-revsets` argument + Prefix the expression with 'all' to allow any number of revisions (i.e. 'all:b|d'). "###); - let stdout = test_env.jj_cmd_success( - &repo_path, - &["rebase", "-s=b|d", "-d=a", "--allow-large-revsets"], - ); + let stdout = test_env.jj_cmd_success(&repo_path, &["rebase", "-s=all:b|d", "-d=a"]); insta::assert_snapshot!(stdout, @r###" Rebased 3 commits Working copy now at: d17539f7ea7c d