op_heads: let caller decide resolve_op_heads() error type

The resolver callback usually returns wider error type, which I don't think
is a variant of OpHeadResolutionError.

To help type inference, resolver's error type is E, not E1 where E: From<E1>.

cli/src/cli_util.rs

@@ -203,14 +203,12 @@ impl From<WorkspaceInitError> for CommandError {
     }
 }
 
-impl From<OpHeadResolutionError<CommandError>> for CommandError {
+impl From<OpHeadResolutionError> for CommandError {
-    fn from(err: OpHeadResolutionError<CommandError>) -> Self {
+    fn from(err: OpHeadResolutionError) -> Self {
         match err {
             OpHeadResolutionError::NoHeads => CommandError::InternalError(
                 "Corrupt repository: there are no operations".to_string(),
             ),
-            OpHeadResolutionError::OpStore(err) => err.into(),
-            OpHeadResolutionError::Err(e) => e,
         }
     }
 }
@@ -617,7 +615,7 @@ impl CommandHelper {
         &self,
         ui: &mut Ui,
         repo_loader: &RepoLoader,
-    ) -> Result<Operation, OpHeadResolutionError<CommandError>> {
+    ) -> Result<Operation, CommandError> {
         if self.global_args.at_operation == "@" {
             op_heads_store::resolve_op_heads(
                 repo_loader.op_heads_store().as_ref(),
@@ -2013,7 +2011,7 @@ pub fn resolve_op_for_load(
     op_store: &Arc<dyn OpStore>,
     op_heads_store: &Arc<dyn OpHeadsStore>,
     op_str: &str,
-) -> Result<Operation, OpHeadResolutionError<CommandError>> {
+) -> Result<Operation, CommandError> {
     let get_current_op = || {
         op_heads_store::resolve_op_heads(op_heads_store.as_ref(), op_store, |_| {
             Err(user_error(format!(
@@ -2021,23 +2019,20 @@ pub fn resolve_op_for_load(
             )))
         })
     };
-    let operation = resolve_single_op(op_store, op_heads_store, get_current_op, op_str)
+    resolve_single_op(op_store, op_heads_store, get_current_op, op_str)
-        .map_err(OpHeadResolutionError::Err)?;
-    Ok(operation)
 }
 
 fn resolve_single_op(
     op_store: &Arc<dyn OpStore>,
     op_heads_store: &Arc<dyn OpHeadsStore>,
-    get_current_op: impl FnOnce() -> Result<Operation, OpHeadResolutionError<CommandError>>,
+    get_current_op: impl FnOnce() -> Result<Operation, CommandError>,
     op_str: &str,
 ) -> Result<Operation, CommandError> {
     let op_symbol = op_str.trim_end_matches('-');
     let op_postfix = &op_str[op_symbol.len()..];
     let mut operation = match op_symbol {
         "@" => get_current_op(),
-        s => resolve_single_op_from_store(op_store, op_heads_store, s)
+        s => resolve_single_op_from_store(op_store, op_heads_store, s),
-            .map_err(OpHeadResolutionError::Err),
     }?;
     for _ in op_postfix.chars() {
         let mut parent_ops = operation.parents();

lib/src/op_heads_store.rs

@@ -26,13 +26,9 @@ use crate::op_store::{OpStore, OpStoreError, OperationId};
 use crate::operation::Operation;
 
 #[derive(Debug, Error)]
-pub enum OpHeadResolutionError<E> {
+pub enum OpHeadResolutionError {
     #[error("Operation log has no heads")]
     NoHeads,
-    #[error(transparent)]
-    OpStore(#[from] OpStoreError),
-    #[error("Op resolution error: {0}")]
-    Err(#[source] E),
 }
 
 pub trait OpHeadsStoreLock {}
@@ -63,12 +59,15 @@ pub fn resolve_op_heads<E>(
     op_heads_store: &dyn OpHeadsStore,
     op_store: &Arc<dyn OpStore>,
     resolver: impl FnOnce(Vec<Operation>) -> Result<Operation, E>,
-) -> Result<Operation, OpHeadResolutionError<E>> {
+) -> Result<Operation, E>
+where
+    E: From<OpHeadResolutionError> + From<OpStoreError>,
+{
     let mut op_heads = op_heads_store.get_op_heads();
 
     // TODO: De-duplicate this 'simple-resolution' code.
     if op_heads.is_empty() {
-        return Err(OpHeadResolutionError::NoHeads);
+        return Err(OpHeadResolutionError::NoHeads.into());
     }
 
     if op_heads.len() == 1 {
@@ -89,7 +88,7 @@ pub fn resolve_op_heads<E>(
     let op_head_ids = op_heads_store.get_op_heads();
 
     if op_head_ids.is_empty() {
-        return Err(OpHeadResolutionError::NoHeads);
+        return Err(OpHeadResolutionError::NoHeads.into());
     }
 
     if op_head_ids.len() == 1 {
@@ -128,13 +127,9 @@ pub fn resolve_op_heads<E>(
     }
 
     op_heads.sort_by_key(|op| op.store_operation().metadata.end_time.timestamp.clone());
-    match resolver(op_heads) {
+    let new_op = resolver(op_heads)?;
-        Ok(new_op) => {
+    let mut old_op_heads = ancestor_op_heads;
-            let mut old_op_heads = ancestor_op_heads;
+    old_op_heads.extend_from_slice(new_op.parent_ids());
-            old_op_heads.extend_from_slice(new_op.parent_ids());
+    op_heads_store.update_op_heads(&old_op_heads, new_op.id());
-            op_heads_store.update_op_heads(&old_op_heads, new_op.id());
+    Ok(new_op)
-            Ok(new_op)
-        }
-        Err(e) => Err(OpHeadResolutionError::Err(e)),
-    }
 }

lib/src/repo.rs

@@ -300,7 +300,7 @@ impl ReadonlyRepo {
     pub fn reload_at_head(
         &self,
         user_settings: &UserSettings,
-    ) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<RepoLoaderError>> {
+    ) -> Result<Arc<ReadonlyRepo>, RepoLoaderError> {
         self.loader().load_at_head(user_settings)
     }
 
@@ -591,6 +591,8 @@ pub enum RepoLoaderError {
     #[error(transparent)]
     TreeMerge(#[from] TreeMergeError),
     #[error(transparent)]
+    OpHeadResolution(#[from] OpHeadResolutionError),
+    #[error(transparent)]
     OpStore(#[from] OpStoreError),
 }
 
@@ -662,7 +664,7 @@ impl RepoLoader {
     pub fn load_at_head(
         &self,
         user_settings: &UserSettings,
-    ) -> Result<Arc<ReadonlyRepo>, OpHeadResolutionError<RepoLoaderError>> {
+    ) -> Result<Arc<ReadonlyRepo>, RepoLoaderError> {
         let op = op_heads_store::resolve_op_heads(
             self.op_heads_store.as_ref(),
             &self.op_store,