cli: add convenient wrapper for user revset evaluation

cli/src/cli_util.rs

@@ -52,8 +52,8 @@ use jj_lib::repo::{
 };
 use jj_lib::repo_path::{FsPathParseError, RepoPath, RepoPathBuf};
 use jj_lib::revset::{
-    Revset, RevsetAliasesMap, RevsetCommitRef, RevsetExpression, RevsetFilterPredicate,
-    RevsetIteratorExt, RevsetParseContext, RevsetParseError, RevsetWorkspaceContext,
+    RevsetAliasesMap, RevsetCommitRef, RevsetExpression, RevsetFilterPredicate, RevsetIteratorExt,
+    RevsetParseContext, RevsetWorkspaceContext,
 };
 use jj_lib::rewrite::restore_tree;
 use jj_lib::settings::{ConfigResultExt as _, UserSettings};
@@ -88,6 +88,7 @@ use crate::git_util::{
 };
 use crate::merge_tools::{DiffEditor, MergeEditor, MergeToolConfigError};
 use crate::operation_templater::OperationTemplateLanguageExtension;
+use crate::revset_util::RevsetExpressionEvaluator;
 use crate::template_builder::TemplateLanguage;
 use crate::template_parser::TemplateAliasesMap;
 use crate::templater::{PropertyPlaceholder, TemplateRenderer};
@@ -754,9 +755,10 @@ 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()?)
+        Ok(self
+            .parse_revset(revision_str)?
+            .evaluate_to_commits()?
+            .try_collect()?)
     }
 
     /// Resolve a revset any number of revisions (including 0), but require the
@@ -781,8 +783,7 @@ impl WorkspaceCommandHelper {
         should_hint_about_all_prefix: bool,
     ) -> Result<Commit, CommandError> {
         let revset_expression = self.parse_revset(revision_str)?;
-        let revset = self.evaluate_revset(revset_expression.clone())?;
-        let mut iter = revset.iter().commits(self.repo().store()).fuse();
+        let mut iter = revset_expression.evaluate_to_commits()?.fuse();
         match (iter.next(), iter.next()) {
             (Some(commit), None) => Ok(commit?),
             (None, _) => Err(user_error(format!(
@@ -821,7 +822,7 @@ impl WorkspaceCommandHelper {
                          `jj abandon -r <REVISION>`.",
                     );
                 } else if let RevsetExpression::CommitRef(RevsetCommitRef::Symbol(branch_name)) =
-                    revset_expression.as_ref()
+                    revset_expression.expression().as_ref()
                 {
                     // Separate hint if there's a conflicted branch
                     cmd_err.add_formatted_hint_with(|formatter| {
@@ -860,30 +861,34 @@ impl WorkspaceCommandHelper {
     pub fn parse_revset(
         &self,
         revision_str: &str,
-    ) -> Result<Rc<RevsetExpression>, RevsetParseError> {
-        revset::parse(revision_str, &self.revset_parse_context())
+    ) -> Result<RevsetExpressionEvaluator<'_>, CommandError> {
+        let expression = revset::parse(revision_str, &self.revset_parse_context())?;
+        self.attach_revset_evaluator(expression)
     }
 
     /// Parses the given revset expressions and concatenates them all.
     pub fn parse_union_revsets(
         &self,
         revision_args: &[RevisionArg],
-    ) -> Result<Rc<RevsetExpression>, RevsetParseError> {
+    ) -> Result<RevsetExpressionEvaluator<'_>, CommandError> {
         let context = self.revset_parse_context();
         let expressions: Vec<_> = revision_args
             .iter()
             .map(|s| revset::parse(s, &context))
             .try_collect()?;
-        Ok(RevsetExpression::union_all(&expressions))
+        let expression = RevsetExpression::union_all(&expressions);
+        self.attach_revset_evaluator(expression)
     }
 
-    pub fn evaluate_revset<'repo>(
-        &'repo self,
+    fn attach_revset_evaluator(
+        &self,
         expression: Rc<RevsetExpression>,
-    ) -> Result<Box<dyn Revset + 'repo>, CommandError> {
-        let repo = self.repo().as_ref();
-        let symbol_resolver = revset_util::default_symbol_resolver(repo, self.id_prefix_context()?);
-        Ok(revset_util::evaluate(repo, &symbol_resolver, expression)?)
+    ) -> Result<RevsetExpressionEvaluator<'_>, CommandError> {
+        Ok(RevsetExpressionEvaluator::new(
+            self.repo().as_ref(),
+            self.id_prefix_context()?,
+            expression,
+        ))
     }
 
     pub(crate) fn revset_parse_context(&self) -> RevsetParseContext {
@@ -908,9 +913,10 @@ impl WorkspaceCommandHelper {
                 .get_string("revsets.short-prefixes")
                 .unwrap_or_else(|_| self.settings.default_revset());
             if !revset_string.is_empty() {
-                let disambiguation_revset = self.parse_revset(&revset_string).map_err(|err| {
-                    config_error_with_message("Invalid `revsets.short-prefixes`", err)
-                })?;
+                let disambiguation_revset =
+                    revset::parse(&revset_string, &self.revset_parse_context()).map_err(|err| {
+                        config_error_with_message("Invalid `revsets.short-prefixes`", err)
+                    })?;
                 context = context.disambiguate_within(revset::optimize(disambiguation_revset));
             }
             Ok(context)
@@ -1003,19 +1009,18 @@ impl WorkspaceCommandHelper {
                 .map(|commit| commit.id().clone())
                 .collect(),
         );
-        let immutable_revset =
-            revset_util::parse_immutable_expression(&self.revset_parse_context())?;
-        let revset = self.evaluate_revset(to_rewrite_revset.intersection(&immutable_revset))?;
-        if let Some(commit) = revset.iter().commits(self.repo().store()).next() {
-            let commit = commit?;
-            let error = if commit.id() == self.repo().store().root_commit_id() {
+        let immutable = revset_util::parse_immutable_expression(&self.revset_parse_context())?;
+        let mut expression = self.attach_revset_evaluator(immutable)?;
+        expression.intersect_with(&to_rewrite_revset);
+        if let Some(commit_id) = expression.evaluate_to_commit_ids()?.next() {
+            let error = if &commit_id == self.repo().store().root_commit_id() {
                 user_error(format!(
                     "The root commit {} is immutable",
-                    short_commit_hash(commit.id()),
+                    short_commit_hash(&commit_id),
                 ))
             } else {
                 user_error_with_hint(
-                    format!("Commit {} is immutable", short_commit_hash(commit.id()),),
+                    format!("Commit {} is immutable", short_commit_hash(&commit_id)),
                     "Configure the set of immutable commits via \
                      `revset-aliases.immutable_heads()`.",
                 )

cli/src/commands/abandon.rs

@@ -16,8 +16,6 @@ use std::io::Write;
 
 use itertools::Itertools as _;
 use jj_lib::object_id::ObjectId;
-use jj_lib::repo::Repo as _;
-use jj_lib::revset::RevsetIteratorExt as _;
 use tracing::instrument;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
@@ -52,12 +50,10 @@ pub(crate) fn cmd_abandon(
     args: &AbandonArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let to_abandon: Vec<_> = {
-        let repo = workspace_command.repo();
-        let expression = workspace_command.parse_union_revsets(&args.revisions)?;
-        let revset = workspace_command.evaluate_revset(expression)?;
-        revset.iter().commits(repo.store()).try_collect()?
-    };
+    let to_abandon: Vec<_> = workspace_command
+        .parse_union_revsets(&args.revisions)?
+        .evaluate_to_commits()?
+        .try_collect()?;
     if to_abandon.is_empty() {
         writeln!(ui.stderr(), "No revisions to abandon.")?;
         return Ok(());

cli/src/commands/bench.rs

@@ -204,7 +204,7 @@ fn bench_revset<M: Measurement>(
     revset: &str,
 ) -> Result<(), CommandError> {
     writeln!(ui.stderr(), "----------Testing revset: {revset}----------")?;
-    let expression = revset::optimize(workspace_command.parse_revset(revset)?);
+    let expression = revset::optimize(workspace_command.parse_revset(revset)?.expression().clone());
     // Time both evaluation and iteration.
     let routine = |workspace_command: &WorkspaceCommandHelper, expression: Rc<RevsetExpression>| {
         // Evaluate the expression without parsing/evaluating short-prefixes.

cli/src/commands/branch.rs

@@ -18,7 +18,6 @@ use std::io::Write as _;
 
 use clap::builder::NonEmptyStringValueParser;
 use itertools::Itertools;
-use jj_lib::backend::CommitId;
 use jj_lib::git;
 use jj_lib::object_id::ObjectId;
 use jj_lib::op_store::{RefTarget, RemoteRef};
@@ -624,12 +623,10 @@ fn cmd_branch_list(
         }
         if !args.revisions.is_empty() {
             // Match against local targets only, which is consistent with "jj git push".
-            let filter_expression = workspace_command.parse_union_revsets(&args.revisions)?;
+            let mut expression = workspace_command.parse_union_revsets(&args.revisions)?;
             // Intersects with the set of local branch targets to minimize the lookup space.
-            let revset_expression = RevsetExpression::branches(StringPattern::everything())
-                .intersection(&filter_expression);
-            let revset = workspace_command.evaluate_revset(revset_expression)?;
-            let filtered_targets: HashSet<CommitId> = revset.iter().collect();
+            expression.intersect_with(&RevsetExpression::branches(StringPattern::everything()));
+            let filtered_targets: HashSet<_> = expression.evaluate_to_commit_ids()?.collect();
             branch_names.extend(
                 view.local_branches()
                     .filter(|(_, target)| {

cli/src/commands/duplicate.rs

@@ -42,11 +42,10 @@ pub(crate) fn cmd_duplicate(
     args: &DuplicateArgs,
 ) -> Result<(), CommandError> {
     let mut workspace_command = command.workspace_helper(ui)?;
-    let to_duplicate: Vec<CommitId> = {
-        let expression = workspace_command.parse_union_revsets(&args.revisions)?;
-        let revset = workspace_command.evaluate_revset(expression)?;
-        revset.iter().collect() // in reverse topological order
-    };
+    let to_duplicate: Vec<CommitId> = workspace_command
+        .parse_union_revsets(&args.revisions)?
+        .evaluate_to_commit_ids()?
+        .collect(); // in reverse topological order
     if to_duplicate.is_empty() {
         writeln!(ui.stderr(), "No revisions to duplicate.")?;
         return Ok(());

cli/src/commands/git.rs

@@ -1192,7 +1192,7 @@ fn find_branches_targeted_by_revisions<'a>(
         .range(&RevsetExpression::commit(wc_commit_id))
         .intersection(&RevsetExpression::branches(StringPattern::everything()));
         let current_branches_revset =
-            workspace_command.evaluate_revset(current_branches_expression)?;
+            current_branches_expression.evaluate_programmatic(workspace_command.repo().as_ref())?;
         revision_commit_ids.extend(current_branches_revset.iter());
         if revision_commit_ids.is_empty() {
             writeln!(
@@ -1203,11 +1203,9 @@ fn find_branches_targeted_by_revisions<'a>(
         }
     }
     for rev_str in revisions {
-        let expression = workspace_command
-            .parse_revset(rev_str)?
-            .intersection(&RevsetExpression::branches(StringPattern::everything()));
-        let revset = workspace_command.evaluate_revset(expression)?;
-        let mut commit_ids = revset.iter().peekable();
+        let mut expression = workspace_command.parse_revset(rev_str)?;
+        expression.intersect_with(&RevsetExpression::branches(StringPattern::everything()));
+        let mut commit_ids = expression.evaluate_to_commit_ids()?.peekable();
         if commit_ids.peek().is_none() {
             let rev_str: &str = rev_str;
             writeln!(

cli/src/commands/log.rs

@@ -88,15 +88,15 @@ pub(crate) fn cmd_log(
                 .iter()
                 .map(|path_arg| workspace_command.parse_file_path(path_arg))
                 .try_collect()?;
-            expression = expression.intersection(&RevsetExpression::filter(
-                RevsetFilterPredicate::File(Some(repo_paths)),
-            ));
+            expression.intersect_with(&RevsetExpression::filter(RevsetFilterPredicate::File(
+                Some(repo_paths),
+            )));
         }
         expression
     };
     let repo = workspace_command.repo();
     let matcher = workspace_command.matcher_from_values(&args.paths)?;
-    let revset = workspace_command.evaluate_revset(revset_expression)?;
+    let revset = revset_expression.evaluate()?;
 
     let store = repo.store();
     let diff_formats =

cli/src/commands/run.rs

@@ -15,8 +15,6 @@
 //! This file contains the internal implementation of `run`.
 
 use itertools::Itertools as _;
-use jj_lib::repo::Repo as _;
-use jj_lib::revset::RevsetIteratorExt as _;
 
 use crate::cli_util::{CommandHelper, RevisionArg};
 use crate::command_error::{user_error, CommandError};
@@ -52,12 +50,10 @@ pub struct RunArgs {
 
 pub fn cmd_run(ui: &mut Ui, command: &CommandHelper, args: &RunArgs) -> Result<(), CommandError> {
     let workspace_command = command.workspace_helper(ui)?;
-    let _resolved_commits: Vec<_> = {
-        let repo = workspace_command.repo();
-        let expression = workspace_command.parse_union_revsets(&args.revisions)?;
-        let revset = workspace_command.evaluate_revset(expression)?;
-        revset.iter().commits(repo.store()).try_collect()?
-    };
+    let _resolved_commits: Vec<_> = workspace_command
+        .parse_union_revsets(&args.revisions)?
+        .evaluate_to_commits()?
+        .try_collect()?;
     // Jobs are resolved in this order:
     // 1. Commandline argument iff > 0.
     // 2. the amount of cores available.

cli/src/revset_util.rs

@@ -17,12 +17,13 @@
 use std::rc::Rc;
 
 use itertools::Itertools as _;
-use jj_lib::backend::CommitId;
+use jj_lib::backend::{BackendResult, CommitId};
+use jj_lib::commit::Commit;
 use jj_lib::id_prefix::IdPrefixContext;
 use jj_lib::repo::Repo;
 use jj_lib::revset::{
     self, DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression,
-    RevsetParseContext, RevsetParseError, RevsetResolutionError,
+    RevsetIteratorExt as _, RevsetParseContext, RevsetParseError, RevsetResolutionError,
 };
 use jj_lib::settings::ConfigResultExt as _;
 use thiserror::Error;
@@ -41,6 +42,60 @@ pub enum UserRevsetEvaluationError {
     Evaluation(RevsetEvaluationError),
 }
 
+/// Wrapper around `RevsetExpression` to provide convenient methods.
+pub struct RevsetExpressionEvaluator<'repo> {
+    repo: &'repo dyn Repo,
+    id_prefix_context: &'repo IdPrefixContext,
+    expression: Rc<RevsetExpression>,
+}
+
+impl<'repo> RevsetExpressionEvaluator<'repo> {
+    pub fn new(
+        repo: &'repo dyn Repo,
+        id_prefix_context: &'repo IdPrefixContext,
+        expression: Rc<RevsetExpression>,
+    ) -> Self {
+        RevsetExpressionEvaluator {
+            repo,
+            id_prefix_context,
+            expression,
+        }
+    }
+
+    /// Returns the underlying expression.
+    pub fn expression(&self) -> &Rc<RevsetExpression> {
+        &self.expression
+    }
+
+    /// Intersects the underlying expression with the `other` expression.
+    pub fn intersect_with(&mut self, other: &Rc<RevsetExpression>) {
+        self.expression = self.expression.intersection(other);
+    }
+
+    /// Evaluates the expression.
+    pub fn evaluate(&self) -> Result<Box<dyn Revset + 'repo>, UserRevsetEvaluationError> {
+        let symbol_resolver = default_symbol_resolver(self.repo, self.id_prefix_context);
+        evaluate(self.repo, &symbol_resolver, self.expression.clone())
+    }
+
+    /// Evaluates the expression to an iterator over commit ids. Entries are
+    /// sorted in reverse topological order.
+    pub fn evaluate_to_commit_ids(
+        &self,
+    ) -> Result<Box<dyn Iterator<Item = CommitId> + 'repo>, UserRevsetEvaluationError> {
+        Ok(self.evaluate()?.iter())
+    }
+
+    /// Evaluates the expression to an iterator over commit objects. Entries are
+    /// sorted in reverse topological order.
+    pub fn evaluate_to_commits(
+        &self,
+    ) -> Result<impl Iterator<Item = BackendResult<Commit>> + 'repo, UserRevsetEvaluationError>
+    {
+        Ok(self.evaluate()?.iter().commits(self.repo.store()))
+    }
+}
+
 pub fn load_revset_aliases(
     ui: &Ui,
     layered_configs: &LayeredConfigs,