diff --git a/cli/src/cli_util.rs b/cli/src/cli_util.rs index 1c37c99fbb..71af456307 100644 --- a/cli/src/cli_util.rs +++ b/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, 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 { 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 `.", ); } 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, RevsetParseError> { - revset::parse(revision_str, &self.revset_parse_context()) + ) -> Result, 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, RevsetParseError> { + ) -> Result, 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, - ) -> Result, 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, 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()`.", ) diff --git a/cli/src/commands/abandon.rs b/cli/src/commands/abandon.rs index 3b00e2853a..be0cb71db9 100644 --- a/cli/src/commands/abandon.rs +++ b/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(()); diff --git a/cli/src/commands/bench.rs b/cli/src/commands/bench.rs index 5c0e805f42..9503479fa7 100644 --- a/cli/src/commands/bench.rs +++ b/cli/src/commands/bench.rs @@ -204,7 +204,7 @@ fn bench_revset( 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| { // Evaluate the expression without parsing/evaluating short-prefixes. diff --git a/cli/src/commands/branch.rs b/cli/src/commands/branch.rs index 7417abd5ac..db12e0175d 100644 --- a/cli/src/commands/branch.rs +++ b/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 = 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)| { diff --git a/cli/src/commands/duplicate.rs b/cli/src/commands/duplicate.rs index 58efcd2866..1457490386 100644 --- a/cli/src/commands/duplicate.rs +++ b/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 = { - 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 = 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(()); diff --git a/cli/src/commands/git.rs b/cli/src/commands/git.rs index 82c035bfc7..205e6258bd 100644 --- a/cli/src/commands/git.rs +++ b/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!( diff --git a/cli/src/commands/log.rs b/cli/src/commands/log.rs index 90cb9b5e80..93ca0f43e4 100644 --- a/cli/src/commands/log.rs +++ b/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 = diff --git a/cli/src/commands/run.rs b/cli/src/commands/run.rs index 4d8cca15c0..cbcab0296f 100644 --- a/cli/src/commands/run.rs +++ b/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. diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index c74efebe80..c1dd62f268 100644 --- a/cli/src/revset_util.rs +++ b/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, +} + +impl<'repo> RevsetExpressionEvaluator<'repo> { + pub fn new( + repo: &'repo dyn Repo, + id_prefix_context: &'repo IdPrefixContext, + expression: Rc, + ) -> Self { + RevsetExpressionEvaluator { + repo, + id_prefix_context, + expression, + } + } + + /// Returns the underlying expression. + pub fn expression(&self) -> &Rc { + &self.expression + } + + /// Intersects the underlying expression with the `other` expression. + pub fn intersect_with(&mut self, other: &Rc) { + self.expression = self.expression.intersection(other); + } + + /// Evaluates the expression. + pub fn evaluate(&self) -> Result, 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 + '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> + 'repo, UserRevsetEvaluationError> + { + Ok(self.evaluate()?.iter().commits(self.repo.store())) + } +} + pub fn load_revset_aliases( ui: &Ui, layered_configs: &LayeredConfigs,