From fc65b000200f1dab814b085cb9b52c2fad2b918b Mon Sep 17 00:00:00 2001
From: Yuya Nishihara <yuya@tcha.org>
Date: Wed, 5 Apr 2023 21:55:30 +0900
Subject: [PATCH] revset: extract CommitId resolution to function

I'm going to merge unresolved variants as RevsetExpression::CommitRef(_).
This prepares for the change.
---
 lib/src/revset.rs | 124 ++++++++++++++++++++++++----------------------
 1 file changed, 66 insertions(+), 58 deletions(-)

diff --git a/lib/src/revset.rs b/lib/src/revset.rs
index 3d659eb6a..86a824506 100644
--- a/lib/src/revset.rs
+++ b/lib/src/revset.rs
@@ -1614,6 +1614,71 @@ pub fn resolve_symbol(
     }
 }
 
+fn resolve_commit_ref(
+    repo: &dyn Repo,
+    expression: &RevsetExpression,
+    workspace_ctx: Option<&RevsetWorkspaceContext>,
+) -> Result<Option<Vec<CommitId>>, RevsetResolutionError> {
+    match expression {
+        RevsetExpression::Symbol(symbol) => {
+            let commit_ids =
+                resolve_symbol(repo, symbol, workspace_ctx.map(|ctx| ctx.workspace_id))?;
+            Ok(Some(commit_ids))
+        }
+        RevsetExpression::Branches(needle) => {
+            let mut commit_ids = vec![];
+            for (branch_name, branch_target) in repo.view().branches() {
+                if !branch_name.contains(needle) {
+                    continue;
+                }
+                if let Some(local_target) = &branch_target.local_target {
+                    commit_ids.extend(local_target.adds());
+                }
+            }
+            Ok(Some(commit_ids))
+        }
+        RevsetExpression::RemoteBranches {
+            branch_needle,
+            remote_needle,
+        } => {
+            let mut commit_ids = vec![];
+            for (branch_name, branch_target) in repo.view().branches() {
+                if !branch_name.contains(branch_needle) {
+                    continue;
+                }
+                for (remote_name, remote_target) in branch_target.remote_targets.iter() {
+                    if remote_name.contains(remote_needle) {
+                        commit_ids.extend(remote_target.adds());
+                    }
+                }
+            }
+            Ok(Some(commit_ids))
+        }
+        RevsetExpression::Tags => {
+            let mut commit_ids = vec![];
+            for ref_target in repo.view().tags().values() {
+                commit_ids.extend(ref_target.adds());
+            }
+            Ok(Some(commit_ids))
+        }
+        RevsetExpression::GitRefs => {
+            let mut commit_ids = vec![];
+            for ref_target in repo.view().git_refs().values() {
+                commit_ids.extend(ref_target.adds());
+            }
+            Ok(Some(commit_ids))
+        }
+        RevsetExpression::GitHead => {
+            let mut commit_ids = vec![];
+            if let Some(ref_target) = repo.view().git_head() {
+                commit_ids.extend(ref_target.adds());
+            }
+            Ok(Some(commit_ids))
+        }
+        _ => Ok(None),
+    }
+}
+
 // TODO: Maybe return a new type (RevsetParameters?) instead of
 // RevsetExpression. Then pass that to evaluate(), so it's clear which variants
 // are allowed.
@@ -1639,64 +1704,7 @@ pub fn resolve_symbols(
             _ => Ok(None),
         },
         |expression| {
-            Ok(match expression.as_ref() {
-                RevsetExpression::Symbol(symbol) => {
-                    let commit_ids =
-                        resolve_symbol(repo, symbol, workspace_ctx.map(|ctx| ctx.workspace_id))?;
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                RevsetExpression::Branches(needle) => {
-                    let mut commit_ids = vec![];
-                    for (branch_name, branch_target) in repo.view().branches() {
-                        if !branch_name.contains(needle) {
-                            continue;
-                        }
-                        if let Some(local_target) = &branch_target.local_target {
-                            commit_ids.extend(local_target.adds());
-                        }
-                    }
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                RevsetExpression::RemoteBranches {
-                    branch_needle,
-                    remote_needle,
-                } => {
-                    let mut commit_ids = vec![];
-                    for (branch_name, branch_target) in repo.view().branches() {
-                        if !branch_name.contains(branch_needle) {
-                            continue;
-                        }
-                        for (remote_name, remote_target) in branch_target.remote_targets.iter() {
-                            if remote_name.contains(remote_needle) {
-                                commit_ids.extend(remote_target.adds());
-                            }
-                        }
-                    }
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                RevsetExpression::Tags => {
-                    let mut commit_ids = vec![];
-                    for ref_target in repo.view().tags().values() {
-                        commit_ids.extend(ref_target.adds());
-                    }
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                RevsetExpression::GitRefs => {
-                    let mut commit_ids = vec![];
-                    for ref_target in repo.view().git_refs().values() {
-                        commit_ids.extend(ref_target.adds());
-                    }
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                RevsetExpression::GitHead => {
-                    let mut commit_ids = vec![];
-                    if let Some(ref_target) = repo.view().git_head() {
-                        commit_ids.extend(ref_target.adds());
-                    }
-                    Some(RevsetExpression::commits(commit_ids))
-                }
-                _ => None,
-            })
+            Ok(resolve_commit_ref(repo, expression, workspace_ctx)?.map(RevsetExpression::commits))
         },
     )?
     .unwrap_or(expression))