From 550164209c8067f8c79ce60121dff31758e3662a Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk <martinvonz@google.com> Date: Fri, 24 Nov 2023 10:29:41 -0800 Subject: [PATCH] revset: add a `RevsetExpression::evaluate_programmatic()` We often resolve a programmatic revset and then immediately evaluate it. This patch adds a convenience method for those two steps. --- cli/src/commands/new.rs | 12 ++++-------- cli/src/commands/next.rs | 3 +-- cli/src/commands/prev.rs | 3 +-- cli/src/commands/rebase.rs | 9 +++------ lib/src/git.rs | 3 +-- lib/src/repo.rs | 16 ++++------------ lib/src/revset.rs | 12 ++++++++++-- lib/src/rewrite.rs | 8 ++------ lib/tests/test_revset.rs | 8 ++------ 9 files changed, 28 insertions(+), 46 deletions(-) diff --git a/cli/src/commands/new.rs b/cli/src/commands/new.rs index 9fb2abceb..af9afd136 100644 --- a/cli/src/commands/new.rs +++ b/cli/src/commands/new.rs @@ -116,8 +116,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", let new_parents = new_children.parents(); if let Some(commit_id) = new_children .dag_range_to(&new_parents) - .resolve_programmatic(tx.repo()) - .evaluate(tx.repo())? + .evaluate_programmatic(tx.repo())? .iter() .next() { @@ -128,8 +127,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", ))); } let mut new_parents_commits: Vec<Commit> = new_parents - .resolve_programmatic(tx.repo()) - .evaluate(tx.repo())? + .evaluate_programmatic(tx.repo())? .iter() .commits(tx.repo().store()) .try_collect()?; @@ -163,8 +161,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", // Exclude children that are ancestors of the new commit let to_rebase = old_parents.children().minus(&old_parents.ancestors()); to_rebase - .resolve_programmatic(tx.base_repo().as_ref()) - .evaluate(tx.base_repo().as_ref())? + .evaluate_programmatic(tx.base_repo().as_ref())? .iter() .commits(tx.base_repo().store()) .try_collect()? @@ -184,8 +181,7 @@ Please use `jj new 'all:x|y'` instead of `jj new --allow-large-revsets x y`.", let commit_parents = RevsetExpression::commits(child_commit.parent_ids().to_owned()); let new_parents = commit_parents.minus(&old_parents); let mut new_parent_commits: Vec<Commit> = new_parents - .resolve_programmatic(tx.base_repo().as_ref()) - .evaluate(tx.base_repo().as_ref())? + .evaluate_programmatic(tx.base_repo().as_ref())? .iter() .commits(tx.base_repo().store()) .try_collect()?; diff --git a/cli/src/commands/next.rs b/cli/src/commands/next.rs index 80fbd6d44..c4740bf23 100644 --- a/cli/src/commands/next.rs +++ b/cli/src/commands/next.rs @@ -90,8 +90,7 @@ pub(crate) fn cmd_next( descendant_expression.minus(&RevsetExpression::commit(current_wc_id.clone()).descendants()) }; let targets: Vec<Commit> = target_expression - .resolve_programmatic(workspace_command.repo().as_ref()) - .evaluate(workspace_command.repo().as_ref())? + .evaluate_programmatic(workspace_command.repo().as_ref())? .iter() .commits(workspace_command.repo().store()) .take(2) diff --git a/cli/src/commands/prev.rs b/cli/src/commands/prev.rs index 576f9cbe7..e5f2c027b 100644 --- a/cli/src/commands/prev.rs +++ b/cli/src/commands/prev.rs @@ -88,8 +88,7 @@ pub(crate) fn cmd_prev( ancestor_expression.minus(&RevsetExpression::commit(current_wc_id.clone())) }; let targets: Vec<_> = target_revset - .resolve_programmatic(workspace_command.repo().as_ref()) - .evaluate(workspace_command.repo().as_ref())? + .evaluate_programmatic(workspace_command.repo().as_ref())? .iter() .commits(workspace_command.repo().store()) .take(2) diff --git a/cli/src/commands/rebase.rs b/cli/src/commands/rebase.rs index 582ae3e1c..8a8281687 100644 --- a/cli/src/commands/rebase.rs +++ b/cli/src/commands/rebase.rs @@ -236,8 +236,7 @@ fn rebase_branch( .range(&RevsetExpression::commits(branch_commit_ids)) .roots(); let root_commits: IndexSet<_> = roots_expression - .resolve_programmatic(workspace_command.repo().as_ref()) - .evaluate(workspace_command.repo().as_ref()) + .evaluate_programmatic(workspace_command.repo().as_ref()) .unwrap() .iter() .commits(workspace_command.repo().store()) @@ -311,8 +310,7 @@ fn rebase_revision( let children_expression = RevsetExpression::commit(old_commit.id().clone()).children(); let child_commits: Vec<_> = children_expression - .resolve_programmatic(workspace_command.repo().as_ref()) - .evaluate(workspace_command.repo().as_ref()) + .evaluate_programmatic(workspace_command.repo().as_ref()) .unwrap() .iter() .commits(workspace_command.repo().store()) @@ -351,8 +349,7 @@ fn rebase_revision( .ancestors(), ); let new_child_parents: Vec<Commit> = new_child_parents_expression - .resolve_programmatic(tx.base_repo().as_ref()) - .evaluate(tx.base_repo().as_ref()) + .evaluate_programmatic(tx.base_repo().as_ref()) .unwrap() .iter() .commits(tx.base_repo().store()) diff --git a/lib/src/git.rs b/lib/src/git.rs index a9140c398..3b546a8df 100644 --- a/lib/src/git.rs +++ b/lib/src/git.rs @@ -371,8 +371,7 @@ fn abandon_unreachable_commits( .range(&RevsetExpression::commits(hidable_git_heads)) .intersection(&RevsetExpression::visible_heads().ancestors()); let abandoned_commits = revset::optimize(abandoned_expression) - .resolve_programmatic(mut_repo) - .evaluate(mut_repo) + .evaluate_programmatic(mut_repo) .unwrap() .iter() .collect_vec(); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 694d76586..f3391f4e7 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -264,10 +264,8 @@ impl ReadonlyRepo { let change_id_index: &'a (dyn ChangeIdIndex + 'a) = self .change_id_index .get_or_init(|| { - let revset: Box<dyn Revset<'a>> = RevsetExpression::all() - .resolve_programmatic(self) - .evaluate(self) - .unwrap(); + let revset: Box<dyn Revset<'a>> = + RevsetExpression::all().evaluate_programmatic(self).unwrap(); let change_id_index: Box<dyn ChangeIdIndex + 'a> = revset.change_id_index(); // evaluate() above only borrows the index, not the whole repo let change_id_index: Box<dyn ChangeIdIndex> = @@ -1324,19 +1322,13 @@ impl Repo for MutableRepo { } fn resolve_change_id_prefix(&self, prefix: &HexPrefix) -> PrefixResolution<Vec<CommitId>> { - let revset = RevsetExpression::all() - .resolve_programmatic(self) - .evaluate(self) - .unwrap(); + let revset = RevsetExpression::all().evaluate_programmatic(self).unwrap(); let change_id_index = revset.change_id_index(); change_id_index.resolve_prefix(prefix) } fn shortest_unique_change_id_prefix_len(&self, target_id: &ChangeId) -> usize { - let revset = RevsetExpression::all() - .resolve_programmatic(self) - .evaluate(self) - .unwrap(); + let revset = RevsetExpression::all().evaluate_programmatic(self).unwrap(); let change_id_index = revset.change_id_index(); change_id_index.shortest_unique_prefix_len(target_id) } diff --git a/lib/src/revset.rs b/lib/src/revset.rs index ed8e246aa..3270cb73f 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -657,6 +657,15 @@ impl RevsetExpression { resolve_symbols(repo, self, symbol_resolver) .map(|expression| resolve_visibility(repo, &expression)) } + + /// Resolve a programmatically created revset expression and evaluate it in + /// the repo. + pub fn evaluate_programmatic<'index>( + self: Rc<Self>, + repo: &'index dyn Repo, + ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetEvaluationError> { + self.resolve_programmatic(repo).evaluate(repo) + } } #[derive(Clone, Debug, Eq, PartialEq)] @@ -1939,8 +1948,7 @@ pub fn walk_revs<'index>( ) -> Result<Box<dyn Revset<'index> + 'index>, RevsetEvaluationError> { RevsetExpression::commits(unwanted.to_vec()) .range(&RevsetExpression::commits(wanted.to_vec())) - .resolve_programmatic(repo) - .evaluate(repo) + .evaluate_programmatic(repo) } fn resolve_git_ref(repo: &dyn Repo, symbol: &str) -> Option<Vec<CommitId>> { diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index d420f5121..6dca78d78 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -289,17 +289,13 @@ impl<'settings, 'repo> DescendantRebaser<'settings, 'repo> { .parents() .minus(&old_commits_expression); let heads_to_add = heads_to_add_expression - .resolve_programmatic(mut_repo) - .evaluate(mut_repo) + .evaluate_programmatic(mut_repo) .unwrap() .iter() .collect(); let to_visit_expression = old_commits_expression.descendants(); - let to_visit_revset = to_visit_expression - .resolve_programmatic(mut_repo) - .evaluate(mut_repo) - .unwrap(); + let to_visit_revset = to_visit_expression.evaluate_programmatic(mut_repo).unwrap(); let to_visit: Vec<_> = to_visit_revset.iter().commits(store).try_collect().unwrap(); drop(to_visit_revset); let to_visit_set: HashSet<CommitId> = diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs index b02923c2c..65f1fc983 100644 --- a/lib/tests/test_revset.rs +++ b/lib/tests/test_revset.rs @@ -364,8 +364,7 @@ fn test_resolve_working_copy() { .unwrap(); let resolve = |ws_id: WorkspaceId| -> Vec<CommitId> { RevsetExpression::working_copy(ws_id) - .resolve_programmatic(mut_repo) - .evaluate(mut_repo) + .evaluate_programmatic(mut_repo) .unwrap() .iter() .collect() @@ -2636,10 +2635,7 @@ fn test_evaluate_expression_file() { let mut_repo = &*mut_repo; let expression = RevsetExpression::filter(RevsetFilterPredicate::File(Some(vec![file_path.clone()]))); - let revset = expression - .resolve_programmatic(mut_repo) - .evaluate(mut_repo) - .unwrap(); + let revset = expression.evaluate_programmatic(mut_repo).unwrap(); revset.iter().collect() };