cli: replace --allow-large-revsets by use of multiple() function

CHANGELOG.md

@@ -11,9 +11,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 * The minimum supported Rust version (MSRV) is now 1.71.0.
 
+* The `--allow-large-revsets` flag for `jj rebase` and `jj new` was replaced by
+  the `multiple()` revset function. For example, use
+  `jj rebase -d 'multiple(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 'multiple(x|y)'`).
 
 ### New features
 

docs/revsets.md

@@ -113,6 +113,9 @@ revsets (expressions) as arguments.
   Note that this is different from
   [Mercurial's](https://repo.mercurial-scm.org/hg/help/revsets) `roots(x)`
   function, which is equivalent to `x ~ x+`.
+* `multiple(x)`: Indicates that you want to allow `x` to resolve to multiple
+  commits even when exactly 1 commit is normally expected. It has no effect in 
+  contexts where multiple commits are allowed by default.
 * `latest(x[, count])`: Latest `count` commits in `x`, based on committer
   timestamp. The default `count` is 1.
 * `merges()`: Merge commits.

lib/src/default_revset_engine.rs

@@ -625,6 +625,7 @@ impl<'index> EvaluationContext<'index> {
                     self.take_latest_revset(candidate_set.as_ref(), *count),
                 ))
             }
+            ResolvedExpression::Multiple(expression) => self.evaluate(expression),
             ResolvedExpression::Union(expression1, expression2) => {
                 let set1 = self.evaluate(expression1)?;
                 let set2 = self.evaluate(expression2)?;

lib/src/revset.rs

@@ -265,6 +265,12 @@ pub enum RevsetExpression {
     },
     Heads(Rc<RevsetExpression>),
     Roots(Rc<RevsetExpression>),
+    /// Marker to indicate that multiple revisions are allowed. For use at the
+    /// top level in contexts where otherwise a single revision is expected,
+    /// such as in `jj rebase -d`.
+    // TODO: Convert this into a more generic `Sized {expression, range}`, or maybe make that its
+    // own variant.
+    Multiple(Rc<RevsetExpression>),
     Latest {
         candidates: Rc<RevsetExpression>,
         count: usize,
@@ -331,6 +337,10 @@ impl RevsetExpression {
         Rc::new(RevsetExpression::CommitRef(RevsetCommitRef::GitHead))
     }
 
+    pub fn multiple(self: &Rc<RevsetExpression>) -> Rc<RevsetExpression> {
+        Rc::new(RevsetExpression::Multiple(self.clone()))
+    }
+
     pub fn latest(self: &Rc<RevsetExpression>, count: usize) -> Rc<RevsetExpression> {
         Rc::new(RevsetExpression::Latest {
             candidates: self.clone(),
@@ -516,6 +526,7 @@ pub enum ResolvedExpression {
     },
     Heads(Box<ResolvedExpression>),
     Roots(Box<ResolvedExpression>),
+    Multiple(Box<ResolvedExpression>),
     Latest {
         candidates: Box<ResolvedExpression>,
         count: usize,
@@ -978,6 +989,11 @@ static BUILTIN_FUNCTION_MAP: Lazy<HashMap<&'static str, RevsetFunction>> = Lazy:
         expect_no_arguments(name, arguments_pair)?;
         Ok(RevsetExpression::git_head())
     });
+    map.insert("multiple", |name, arguments_pair, state| {
+        let arg = expect_one_argument(name, arguments_pair)?;
+        let expression = parse_expression_rule(arg.into_inner(), state)?;
+        Ok(expression.multiple())
+    });
     map.insert("latest", |name, arguments_pair, state| {
         let ([candidates_arg], [count_opt_arg]) = expect_arguments(name, arguments_pair)?;
         let candidates = parse_expression_rule(candidates_arg.into_inner(), state)?;
@@ -1312,6 +1328,9 @@ fn try_transform_expression<E>(
                     candidates,
                     count: *count,
                 }),
+            RevsetExpression::Multiple(expression) => {
+                transform_rec(expression, pre, post)?.map(RevsetExpression::Multiple)
+            }
             RevsetExpression::Filter(_) => None,
             RevsetExpression::AsFilter(candidates) => {
                 transform_rec(candidates, pre, post)?.map(RevsetExpression::AsFilter)
@@ -2019,6 +2038,9 @@ impl VisibilityResolutionContext<'_> {
                 candidates: self.resolve(candidates).into(),
                 count: *count,
             },
+            RevsetExpression::Multiple(expression) => {
+                ResolvedExpression::Multiple(self.resolve(expression).into())
+            }
             RevsetExpression::Filter(_) | RevsetExpression::AsFilter(_) => {
                 // Top-level filter without intersection: e.g. "~author(_)" is represented as
                 // `AsFilter(NotIn(Filter(Author(_))))`.
@@ -2095,6 +2117,7 @@ impl VisibilityResolutionContext<'_> {
             | RevsetExpression::DagRange { .. }
             | RevsetExpression::Heads(_)
             | RevsetExpression::Roots(_)
+            | RevsetExpression::Multiple(_)
             | RevsetExpression::Latest { .. } => {
                 ResolvedPredicateExpression::Set(self.resolve(expression).into())
             }

src/cli_util.rs

@@ -932,6 +932,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) -> Result<Commit, CommandError> {
         let revset_expression = self.parse_revset(revision_str)?;
         let revset = self.evaluate_revset(revset_expression)?;
@@ -962,12 +964,41 @@ impl WorkspaceCommandHelper {
         }
     }
 
+    /// Resolve a revset any number of revisions (including 0).
     pub fn resolve_revset(&self, revision_str: &str) -> Result<Vec<Commit>, CommandError> {
         let revset_expression = self.parse_revset(revision_str)?;
         let revset = self.evaluate_revset(revset_expression)?;
         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 wrapping the
+    /// expression in `multiple()`.
+    pub fn resolve_revset_default_single(
+        &self,
+        revision_str: &str,
+    ) -> Result<Vec<Commit>, CommandError> {
+        let revset_expression = self.parse_revset(revision_str)?;
+        if matches!(revset_expression.as_ref(), RevsetExpression::Multiple(_)) {
+            self.resolve_revset(revision_str)
+        } else {
+            self.resolve_single_rev(revision_str)
+                .map_err(|err| match err {
+                    CommandError::UserError { message, hint } => user_error_with_hint(
+                        message,
+                        format!(
+                            "{old_hint}Wrap the expression in 'multiple()' to allow any number of \
+                             revisions (i.e. 'multiple({})').",
+                            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,
@@ -1746,28 +1777,15 @@ 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,
     revisions: &[RevisionArg],
-    allow_plural_revsets: bool,
 ) -> Result<IndexSet<Commit>, CommandError> {
     let mut all_commits = IndexSet::new();
     for revision_str in revisions {
-        if allow_plural_revsets {
-            let commits = workspace_command.resolve_revset(revision_str)?;
-            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)
-                .map_err(append_large_revsets_hint_if_multiple_revisions)?;
+        let commits = workspace_command.resolve_revset_default_single(revision_str)?;
+        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!(
@@ -1779,24 +1797,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<ReadonlyRepo>,
     wc: &mut WorkingCopy,

src/commands/mod.rs

@@ -56,7 +56,7 @@ use maplit::{hashmap, hashset};
 
 use crate::cli_util::{
     check_stale_working_copy, get_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,
@@ -527,9 +527,6 @@ 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,
     /// Insert the new change between the target commit(s) and their children
     #[arg(long, short = 'A', visible_alias = "after")]
     insert_after: bool,
@@ -857,10 +854,6 @@ struct RebaseArgs {
     /// commit)
     #[arg(long, short, required = true)]
     destination: Vec<RevisionArg>,
-    /// Allow revsets expanding to multiple commits in a single argument (for
-    /// those options that can be repeated)
-    #[arg(long, short = 'L')]
-    allow_large_revsets: bool,
 }
 
 /// Apply the reverse of a revision on top of another revision
@@ -2159,13 +2152,8 @@ fn cmd_edit(ui: &mut Ui, command: &CommandHelper, args: &EditArgs) -> Result<(),
 fn resolve_destination_revs(
     workspace_command: &WorkspaceCommandHelper,
     revisions: &[RevisionArg],
-    allow_plural_revsets: bool,
 ) -> Result<IndexSet<Commit>, CommandError> {
-    let commits = resolve_multiple_nonempty_revsets_flag_guarded(
-        workspace_command,
-        revisions,
-        allow_plural_revsets,
-    )?;
+    let commits = resolve_multiple_nonempty_revsets_default_single(workspace_command, 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"))
@@ -2180,13 +2168,9 @@ fn cmd_new(ui: &mut Ui, command: &CommandHelper, args: &NewArgs) -> Result<(), C
         !args.revisions.is_empty(),
         "expected a non-empty list from clap"
     );
-    let target_commits = resolve_destination_revs(
-        &workspace_command,
-        &args.revisions,
-        args.allow_large_revsets,
-    )?
-    .into_iter()
-    .collect_vec();
+    let target_commits = resolve_destination_revs(&workspace_command, &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;
@@ -3065,7 +3049,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",
         )));
@@ -3075,21 +3059,14 @@ 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_destination_revs(
-        &workspace_command,
-        &args.destination,
-        args.allow_large_revsets,
-    )?
-    .into_iter()
-    .collect_vec();
+    let new_parents = resolve_destination_revs(&workspace_command, &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,
-            &args.source,
-            args.allow_large_revsets,
-        )?;
+        let source_commits =
+            resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.source)?;
         rebase_descendants(
             ui,
             command,
@@ -3101,11 +3078,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("@")?])
         } else {
-            resolve_multiple_nonempty_revsets_flag_guarded(
-                &workspace_command,
-                &args.branch,
-                args.allow_large_revsets,
-            )?
+            resolve_multiple_nonempty_revsets_default_single(&workspace_command, &args.branch)?
         };
         rebase_branch(
             ui,

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
+    Wrap the expression in 'multiple()' to allow any number of revisions (i.e. 'multiple(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=multiple(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
+    Wrap the expression in 'multiple()' to allow any number of revisions (i.e. 'multiple(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", "multiple(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 multiple() 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", "multiple(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
+    Wrap the expression in 'multiple()' to allow any number of revisions (i.e. 'multiple(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=multiple(b|d)", "-d=a"]);
     insta::assert_snapshot!(stdout, @r###"
     Rebased 3 commits
     Working copy now at: d17539f7ea7c d