revset: convert commit and change prefix resolvers into partial symbol resolvers

cli/src/revset_util.rs

@@ -159,13 +159,7 @@ pub fn default_symbol_resolver<'a>(
     repo: &'a dyn Repo,
     id_prefix_context: &'a IdPrefixContext,
 ) -> DefaultSymbolResolver<'a> {
-    let commit_id_resolver: revset::PrefixResolver<CommitId> =
+    DefaultSymbolResolver::new(repo).with_id_prefix_context(id_prefix_context)
-        Box::new(|repo, prefix| id_prefix_context.resolve_commit_prefix(repo, prefix));
-    let change_id_resolver: revset::PrefixResolver<Vec<CommitId>> =
-        Box::new(|repo, prefix| id_prefix_context.resolve_change_prefix(repo, prefix));
-    DefaultSymbolResolver::new(repo)
-        .with_commit_id_resolver(commit_id_resolver)
-        .with_change_id_resolver(change_id_resolver)
 }
 
 /// Parses user-configured expression defining the immutable set.

lib/src/revset.rs

@@ -37,6 +37,7 @@ use crate::dsl_util::{collect_similar, StringLiteralParser};
 use crate::fileset::{FilePattern, FilesetExpression, FilesetParseContext};
 use crate::git;
 use crate::hex_util::to_forward_hex;
+use crate::id_prefix::IdPrefixContext;
 use crate::object_id::{HexPrefix, PrefixResolution};
 use crate::op_store::WorkspaceId;
 use crate::repo::Repo;
@@ -2148,45 +2149,93 @@ impl PartialSymbolResolver for GitRefResolver {
 const DEFAULT_RESOLVERS: &[&'static dyn PartialSymbolResolver] =
     &[&TagResolver, &BranchResolver, &GitRefResolver];
 
-pub type PrefixResolver<'a, T> = Box<dyn Fn(&dyn Repo, &HexPrefix) -> PrefixResolution<T> + 'a>;
+#[derive(Default)]
+struct CommitPrefixResolver<'a> {
+    context: Option<&'a IdPrefixContext>,
+}
+
+impl PartialSymbolResolver for CommitPrefixResolver<'_> {
+    fn resolve_symbol(
+        &self,
+        repo: &dyn Repo,
+        symbol: &str,
+    ) -> Result<Option<Vec<CommitId>>, RevsetResolutionError> {
+        if let Some(prefix) = HexPrefix::new(symbol) {
+            let resolution = self
+                .context
+                .as_ref()
+                .map(|ctx| ctx.resolve_commit_prefix(repo, &prefix))
+                .unwrap_or_else(|| repo.index().resolve_commit_id_prefix(&prefix));
+            match resolution {
+                PrefixResolution::AmbiguousMatch => Err(
+                    RevsetResolutionError::AmbiguousCommitIdPrefix(symbol.to_owned()),
+                ),
+                PrefixResolution::SingleMatch(id) => Ok(Some(vec![id])),
+                PrefixResolution::NoMatch => Ok(None),
+            }
+        } else {
+            Ok(None)
+        }
+    }
+}
+
+#[derive(Default)]
+struct ChangePrefixResolver<'a> {
+    context: Option<&'a IdPrefixContext>,
+}
+
+impl PartialSymbolResolver for ChangePrefixResolver<'_> {
+    fn resolve_symbol(
+        &self,
+        repo: &dyn Repo,
+        symbol: &str,
+    ) -> Result<Option<Vec<CommitId>>, RevsetResolutionError> {
+        if let Some(prefix) = to_forward_hex(symbol).as_deref().and_then(HexPrefix::new) {
+            let resolution = self
+                .context
+                .as_ref()
+                .map(|ctx| ctx.resolve_change_prefix(repo, &prefix))
+                .unwrap_or_else(|| repo.resolve_change_id_prefix(&prefix));
+            match resolution {
+                PrefixResolution::AmbiguousMatch => Err(
+                    RevsetResolutionError::AmbiguousChangeIdPrefix(symbol.to_owned()),
+                ),
+                PrefixResolution::SingleMatch(ids) => Ok(Some(ids)),
+                PrefixResolution::NoMatch => Ok(None),
+            }
+        } else {
+            Ok(None)
+        }
+    }
+}
 
 /// Resolves branches, remote branches, tags, git refs, and full and abbreviated
 /// commit and change ids.
 pub struct DefaultSymbolResolver<'a> {
     repo: &'a dyn Repo,
-    commit_id_resolver: PrefixResolver<'a, CommitId>,
+    commit_id_resolver: CommitPrefixResolver<'a>,
-    change_id_resolver: PrefixResolver<'a, Vec<CommitId>>,
+    change_id_resolver: ChangePrefixResolver<'a>,
 }
 
 impl<'a> DefaultSymbolResolver<'a> {
     pub fn new(repo: &'a dyn Repo) -> Self {
         DefaultSymbolResolver {
             repo,
-            commit_id_resolver: Box::new(|repo, prefix| {
+            commit_id_resolver: Default::default(),
-                repo.index().resolve_commit_id_prefix(prefix)
+            change_id_resolver: Default::default(),
-            }),
-            change_id_resolver: Box::new(|repo, prefix| repo.resolve_change_id_prefix(prefix)),
         }
     }
 
-    pub fn with_commit_id_resolver(
+    pub fn with_id_prefix_context(mut self, id_prefix_context: &'a IdPrefixContext) -> Self {
-        mut self,
+        self.commit_id_resolver.context = Some(id_prefix_context);
-        commit_id_resolver: PrefixResolver<'a, CommitId>,
+        self.change_id_resolver.context = Some(id_prefix_context);
-    ) -> Self {
-        self.commit_id_resolver = commit_id_resolver;
-        self
-    }
-
-    pub fn with_change_id_resolver(
-        mut self,
-        change_id_resolver: PrefixResolver<'a, Vec<CommitId>>,
-    ) -> Self {
-        self.change_id_resolver = change_id_resolver;
         self
     }
 
     fn partial_resolvers(&self) -> impl Iterator<Item = &dyn PartialSymbolResolver> {
-        DEFAULT_RESOLVERS.iter().copied()
+        let prefix_resolvers: [&dyn PartialSymbolResolver; 2] =
+            [&self.commit_id_resolver, &self.change_id_resolver];
+        itertools::chain!(DEFAULT_RESOLVERS.iter().copied(), prefix_resolvers)
     }
 }
 
@@ -2201,41 +2250,6 @@ impl SymbolResolver for DefaultSymbolResolver<'_> {
                 return Ok(ids);
             }
         }
-        // TODO: Convert prefix resolvers
-
-        // Try to resolve as a commit id.
-        if let Some(prefix) = HexPrefix::new(symbol) {
-            match (self.commit_id_resolver)(self.repo, &prefix) {
-                PrefixResolution::AmbiguousMatch => {
-                    return Err(RevsetResolutionError::AmbiguousCommitIdPrefix(
-                        symbol.to_owned(),
-                    ));
-                }
-                PrefixResolution::SingleMatch(id) => {
-                    return Ok(vec![id]);
-                }
-                PrefixResolution::NoMatch => {
-                    // Fall through
-                }
-            }
-        }
-
-        // Try to resolve as a change id.
-        if let Some(prefix) = to_forward_hex(symbol).as_deref().and_then(HexPrefix::new) {
-            match (self.change_id_resolver)(self.repo, &prefix) {
-                PrefixResolution::AmbiguousMatch => {
-                    return Err(RevsetResolutionError::AmbiguousChangeIdPrefix(
-                        symbol.to_owned(),
-                    ));
-                }
-                PrefixResolution::SingleMatch(ids) => {
-                    return Ok(ids);
-                }
-                PrefixResolution::NoMatch => {
-                    // Fall through
-                }
-            }
-        }
 
         Err(make_no_such_symbol_error(self.repo, symbol))
     }