From 99e9cd70d146402c698adaf07b54531c17b53fee Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Fri, 5 May 2023 14:01:25 -0700
Subject: [PATCH] cli: make `WorkspaceCommandHelper` create `SymbolResolver`

I would eventually want the `SymbolResolver` to be customizable (in
custom `jj` binaries), so we want to make sure we always use the
customized version of it.

I left `RevsetExpression::resolve()` unchanged. I consider that to be
for programmatically created expressions.
---
 lib/src/revset.rs        |  7 +++----
 lib/tests/test_revset.rs |  9 +++++----
 src/cli_util.rs          | 13 +++++++++----
 src/commands/mod.rs      |  3 ++-
 4 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 93afac975..076338b85 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -441,13 +441,12 @@ impl RevsetExpression {
             .map(|expression| resolve_visibility(repo, &expression))
     }
 
-    pub fn resolve_in_workspace(
+    pub fn resolve_user_expression(
         self: Rc<Self>,
         repo: &dyn Repo,
-        workspace_ctx: &RevsetWorkspaceContext,
+        symbol_resolver: &dyn SymbolResolver,
     ) -> Result<ResolvedExpression, RevsetResolutionError> {
-        let symbol_resolver = DefaultSymbolResolver::new(repo, Some(workspace_ctx.workspace_id));
-        resolve_symbols(repo, self, &symbol_resolver)
+        resolve_symbols(repo, self, symbol_resolver)
             .map(|expression| resolve_visibility(repo, &expression))
     }
 }
diff --git a/lib/tests/test_revset.rs b/lib/tests/test_revset.rs
index d5923b70b..3899b9f5a 100644
--- a/lib/tests/test_revset.rs
+++ b/lib/tests/test_revset.rs
@@ -24,9 +24,9 @@ use jujutsu_lib::op_store::{RefTarget, WorkspaceId};
 use jujutsu_lib::repo::Repo;
 use jujutsu_lib::repo_path::RepoPath;
 use jujutsu_lib::revset::{
-    optimize, parse, resolve_symbol, ReverseRevsetGraphIterator, Revset, RevsetAliasesMap,
-    RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge, RevsetResolutionError,
-    RevsetWorkspaceContext,
+    optimize, parse, resolve_symbol, DefaultSymbolResolver, ReverseRevsetGraphIterator, Revset,
+    RevsetAliasesMap, RevsetExpression, RevsetFilterPredicate, RevsetGraphEdge,
+    RevsetResolutionError, RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::GitSettings;
 use jujutsu_lib::tree::merge_trees;
@@ -521,8 +521,9 @@ fn resolve_commit_ids_in_workspace(
     };
     let expression =
         optimize(parse(revset_str, &RevsetAliasesMap::new(), Some(&workspace_ctx)).unwrap());
+    let symbol_resolver = DefaultSymbolResolver::new(repo, Some(workspace_ctx.workspace_id));
     let expression = expression
-        .resolve_in_workspace(repo, &workspace_ctx)
+        .resolve_user_expression(repo, &symbol_resolver)
         .unwrap();
     expression.evaluate(repo).unwrap().iter().collect()
 }
diff --git a/src/cli_util.rs b/src/cli_util.rs
index 61c30df99..efbb9b6d4 100644
--- a/src/cli_util.rs
+++ b/src/cli_util.rs
@@ -44,8 +44,9 @@ use jujutsu_lib::repo::{
 };
 use jujutsu_lib::repo_path::{FsPathParseError, RepoPath};
 use jujutsu_lib::revset::{
-    Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression, RevsetIteratorExt,
-    RevsetParseError, RevsetParseErrorKind, RevsetResolutionError, RevsetWorkspaceContext,
+    DefaultSymbolResolver, Revset, RevsetAliasesMap, RevsetEvaluationError, RevsetExpression,
+    RevsetIteratorExt, RevsetParseError, RevsetParseErrorKind, RevsetResolutionError,
+    RevsetWorkspaceContext,
 };
 use jujutsu_lib::settings::UserSettings;
 use jujutsu_lib::transaction::Transaction;
@@ -883,8 +884,8 @@ impl WorkspaceCommandHelper {
         &'repo self,
         revset_expression: Rc<RevsetExpression>,
     ) -> Result<Box<dyn Revset<'repo> + 'repo>, CommandError> {
-        let revset_expression =
-            revset_expression.resolve_in_workspace(self.repo.as_ref(), &self.revset_context())?;
+        let revset_expression = revset_expression
+            .resolve_user_expression(self.repo.as_ref(), &self.revset_symbol_resolver())?;
         Ok(revset_expression.evaluate(self.repo.as_ref())?)
     }
 
@@ -900,6 +901,10 @@ impl WorkspaceCommandHelper {
         }
     }
 
+    pub(crate) fn revset_symbol_resolver(&self) -> DefaultSymbolResolver<'_> {
+        DefaultSymbolResolver::new(self.repo().as_ref(), Some(self.workspace_id()))
+    }
+
     pub fn template_aliases_map(&self) -> &TemplateAliasesMap {
         &self.template_aliases_map
     }
diff --git a/src/commands/mod.rs b/src/commands/mod.rs
index 0b2289425..112e2a053 100644
--- a/src/commands/mod.rs
+++ b/src/commands/mod.rs
@@ -3224,7 +3224,8 @@ fn cmd_debug_revset(
     writeln!(ui, "{expression:#?}")?;
     writeln!(ui)?;
 
-    let expression = expression.resolve_in_workspace(repo, &workspace_ctx)?;
+    let expression =
+        expression.resolve_user_expression(repo, &workspace_command.revset_symbol_resolver())?;
     writeln!(ui, "-- Resolved:")?;
     writeln!(ui, "{expression:#?}")?;
     writeln!(ui)?;