cli: replace --allow-large-revsets by use of revset function

The `--allow-large-revsets` flag we have on `jj rebase` and `jj new`
allows the user to do e.g. `jj rebase --allow-large-revsets -b
main.. -d main` to rebase all commits that are not in main onto
main. The reason we don't allow these revsets to resolve to multiple
commits by default is that we think users might specify multiple
commits by mistake. That's probably not much of a problem with `jj
rebase -b` (maybe we should always allow that to resolve to multiple
commits), but the user might want to know if `jj rebase -d @-`
resolves to multiple commits.

One problem with having a flag to allow multiple commits is that it
needs to be added to every command where we want to allow multiple
commits but default to one. Also, it should probably apply to each
revset argument those commands take. For example, even if the user
meant `-b main..` to resolve to multiple commits, they might not have
meant `-d main` to resolve to multiple commits (which it will in case
of a conflicted branch), so we might want separate
`--allow-large-revsets-in-destination` and
`--allow-large-revsets-in-source`, which gets quite cumbersome. It
seems better to have some syntax in the individual revsets for saying
that multiple commits are allowed.

I was initially thinking of extending the revset syntax by adding some
indicator outside the expression itself, because allowing multiple
commits only makes sense at the top level. We could use a prefix or
syffix of e.g. '%' or '*' for that, so you'd write `jj rebase -d
'x|y%'` or `jj rebase -d '*x|y'`. I don't like that syntax because we
already have other operators, so adding another operator makes it
looks like it's part of the syntax.

Another reason to use function syntax is that we may later want to add
a more generic `sized(x, 1..5)` function, which would error out if the
input revset resolves to an unexpected size. If we have that, then,
when the user runs `jj rebase -d foo..bar`, we can convert that to
`sized(foo..bar, 1..1)` unless there's already a top-level `sized()`
expression (which there isn't in this example).

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/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(),
@@ -978,6 +988,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 +1327,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 +2037,7 @@ impl VisibilityResolutionContext<'_> {
                 candidates: self.resolve(candidates).into(),
                 count: *count,
             },
+            RevsetExpression::Multiple(expression) => self.resolve(expression),
             RevsetExpression::Filter(_) | RevsetExpression::AsFilter(_) => {
                 // Top-level filter without intersection: e.g. "~author(_)" is represented as
                 // `AsFilter(NotIn(Filter(Author(_))))`.
@@ -2095,6 +2114,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,
@@ -2159,13 +2159,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"))
@@ -2175,18 +2170,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 'multiple(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,
-        &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 +3062,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",
         )));
@@ -3074,22 +3071,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 'multiple(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,
-        &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 +3097,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