diff --git a/cli/src/revset_util.rs b/cli/src/revset_util.rs index 9391bc4e0..397cf07ed 100644 --- a/cli/src/revset_util.rs +++ b/cli/src/revset_util.rs @@ -19,7 +19,6 @@ use std::rc::Rc; use std::sync::Arc; use itertools::Itertools as _; -use jj_lib::backend::BackendResult; use jj_lib::backend::CommitId; use jj_lib::commit::Commit; use jj_lib::id_prefix::IdPrefixContext; @@ -114,8 +113,10 @@ impl<'repo> RevsetExpressionEvaluator<'repo> { /// sorted in reverse topological order. pub fn evaluate_to_commits( &self, - ) -> Result> + 'repo, UserRevsetEvaluationError> - { + ) -> Result< + impl Iterator> + 'repo, + UserRevsetEvaluationError, + > { Ok(self.evaluate()?.iter().commits(self.repo.store())) } } diff --git a/lib/src/annotate.rs b/lib/src/annotate.rs index 6048ece91..8d8bc276e 100644 --- a/lib/src/annotate.rs +++ b/lib/src/annotate.rs @@ -36,7 +36,6 @@ use crate::graph::GraphEdgeType; use crate::merged_tree::MergedTree; use crate::repo::Repo; use crate::repo_path::RepoPath; -use crate::revset::RevsetEvaluationError; use crate::revset::RevsetExpression; use crate::revset::RevsetFilterPredicate; use crate::store::Store; @@ -164,12 +163,7 @@ fn process_commits( .filtered(predicate), ) .evaluate_programmatic(repo) - .map_err(|e| match e { - RevsetEvaluationError::StoreError(backend_error) => backend_error, - RevsetEvaluationError::Other(_) => { - panic!("Unable to evaluate internal revset") - } - })?; + .map_err(|e| e.expect_backend_error())?; let mut commit_line_map = get_initial_commit_line_map(starting_commit_id, num_lines); let mut original_line_map = HashMap::new(); diff --git a/lib/src/repo.rs b/lib/src/repo.rs index 6a0398248..382ec96f5 100644 --- a/lib/src/repo.rs +++ b/lib/src/repo.rs @@ -78,7 +78,6 @@ use crate::refs::diff_named_remote_refs; use crate::refs::merge_ref_targets; use crate::refs::merge_remote_refs; use crate::revset; -use crate::revset::RevsetEvaluationError; use crate::revset::RevsetExpression; use crate::revset::RevsetIteratorExt; use crate::rewrite::merge_commit_trees; @@ -1194,11 +1193,13 @@ impl MutableRepo { )); let to_visit_revset = to_visit_expression .evaluate_programmatic(self) - .map_err(|err| match err { - RevsetEvaluationError::StoreError(err) => err, - RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"), - })?; - let to_visit: Vec<_> = to_visit_revset.iter().commits(store).try_collect()?; + .map_err(|err| err.expect_backend_error())?; + let to_visit: Vec<_> = to_visit_revset + .iter() + .commits(store) + .try_collect() + // TODO: Return evaluation error to caller + .map_err(|err| err.expect_backend_error())?; drop(to_visit_revset); let to_visit_set: HashSet = to_visit.iter().map(|commit| commit.id().clone()).collect(); diff --git a/lib/src/revset.rs b/lib/src/revset.rs index d09bfcb90..568c56e82 100644 --- a/lib/src/revset.rs +++ b/lib/src/revset.rs @@ -28,7 +28,6 @@ use once_cell::sync::Lazy; use thiserror::Error; use crate::backend::BackendError; -use crate::backend::BackendResult; use crate::backend::ChangeId; use crate::backend::CommitId; use crate::commit::Commit; @@ -98,6 +97,15 @@ pub enum RevsetEvaluationError { Other(Box), } +impl RevsetEvaluationError { + pub fn expect_backend_error(self) -> BackendError { + match self { + Self::StoreError(err) => err, + Self::Other(err) => panic!("Unexpected revset error: {err}"), + } + } +} + // assumes index has less than u64::MAX entries. pub const GENERATION_RANGE_FULL: Range = 0..u64::MAX; pub const GENERATION_RANGE_EMPTY: Range = 0..0; @@ -2246,12 +2254,14 @@ pub struct RevsetCommitIterator { } impl> Iterator for RevsetCommitIterator { - type Item = BackendResult; + type Item = Result; fn next(&mut self) -> Option { - self.iter - .next() - .map(|commit_id| self.store.get_commit(&commit_id)) + self.iter.next().map(|commit_id| { + self.store + .get_commit(&commit_id) + .map_err(RevsetEvaluationError::StoreError) + }) } } diff --git a/lib/src/rewrite.rs b/lib/src/rewrite.rs index 9f4434638..10d4bb70b 100644 --- a/lib/src/rewrite.rs +++ b/lib/src/rewrite.rs @@ -42,7 +42,6 @@ use crate::merged_tree::TreeDiffEntry; use crate::repo::MutableRepo; use crate::repo::Repo; use crate::repo_path::RepoPath; -use crate::revset::RevsetEvaluationError; use crate::revset::RevsetExpression; use crate::revset::RevsetIteratorExt; use crate::settings::UserSettings; @@ -500,13 +499,12 @@ pub fn move_commits( RevsetExpression::commits(target_commits.iter().ids().cloned().collect_vec()) .connected() .evaluate_programmatic(mut_repo) - .map_err(|err| match err { - RevsetEvaluationError::StoreError(err) => err, - RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"), - })? + .map_err(|err| err.expect_backend_error())? .iter() .commits(mut_repo.store()) - .try_collect()?; + .try_collect() + // TODO: Return evaluation error to caller + .map_err(|err| err.expect_backend_error())?; // Compute the parents of all commits in the connected target set, allowing only // commits in the target set as parents. The parents of each commit are @@ -588,13 +586,12 @@ pub fn move_commits( .children(), ) .evaluate_programmatic(mut_repo) - .map_err(|err| match err { - RevsetEvaluationError::StoreError(err) => err, - RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"), - })? + .map_err(|err| err.expect_backend_error())? .iter() .commits(mut_repo.store()) - .try_collect()?; + .try_collect() + // TODO: Return evaluation error to caller + .map_err(|err| err.expect_backend_error())?; // For all commits in the target set, compute its transitive descendant commits // which are outside of the target set by up to 1 generation. @@ -717,13 +714,12 @@ pub fn move_commits( let to_visit_expression = RevsetExpression::commits(roots).descendants(); let to_visit: Vec<_> = to_visit_expression .evaluate_programmatic(mut_repo) - .map_err(|err| match err { - RevsetEvaluationError::StoreError(err) => err, - RevsetEvaluationError::Other(_) => panic!("Unexpected revset error: {err}"), - })? + .map_err(|err| err.expect_backend_error())? .iter() .commits(mut_repo.store()) - .try_collect()?; + .try_collect() + // TODO: Return evaluation error to caller + .map_err(|err| err.expect_backend_error())?; let to_visit_commits: IndexMap<_, _> = to_visit .into_iter() .map(|commit| (commit.id().clone(), commit))