merge: move non-consuming Merge<Option<TreeValue>> methods to generic…

… type

The next patch will add .is_tree() callers, and the other methods don't
required owned type.

lib/src/merge.rs

@@ -539,21 +539,29 @@ impl MergedTreeValue {
             .collect();
         backend::Conflict { removes, adds }
     }
+}
 
+impl<T> Merge<Option<T>>
+where
+    T: Borrow<TreeValue>,
+{
     /// Whether this merge should be recursed into when doing directory walks.
     pub fn is_tree(&self) -> bool {
         self.is_present()
-            && self
-                .iter()
-                .all(|value| matches!(value, Some(TreeValue::Tree(_)) | None))
+            && self.iter().all(|value| {
+                matches!(
+                    borrow_tree_value(value.as_ref()),
+                    Some(TreeValue::Tree(_)) | None
+                )
+            })
     }
 
     /// If this merge contains only files or absent entries, returns a merge of
     /// the `FileId`s`. The executable bits will be ignored. Use
     /// `Merge::with_new_file_ids()` to produce a new merge with the original
     /// executable bits preserved.
     pub fn to_file_merge(&self) -> Option<Merge<Option<FileId>>> {
-        self.maybe_map(|term| match term {
+        self.maybe_map(|term| match borrow_tree_value(term.as_ref()) {
             None => Some(None),
             Some(TreeValue::File { id, executable: _ }) => Some(Some(id.clone())),
             _ => None,
@@ -563,20 +571,50 @@ impl MergedTreeValue {
     /// If this merge contains only files or absent entries, returns a merge of
     /// the files' executable bits.
     pub fn to_executable_merge(&self) -> Option<Merge<bool>> {
-        self.maybe_map(|term| match term {
+        self.maybe_map(|term| match borrow_tree_value(term.as_ref()) {
             None => Some(false),
             Some(TreeValue::File { id: _, executable }) => Some(*executable),
             _ => None,
         })
     }
 
+    /// If every non-`None` term of a `MergedTreeValue`
+    /// is a `TreeValue::Tree`, this converts it to
+    /// a `Merge<Tree>`, with empty trees instead of
+    /// any `None` terms. Otherwise, returns `None`.
+    pub fn to_tree_merge(
+        &self,
+        store: &Arc<Store>,
+        dir: &RepoPath,
+    ) -> BackendResult<Option<Merge<Tree>>> {
+        let tree_id_merge = self.maybe_map(|term| match borrow_tree_value(term.as_ref()) {
+            None => Some(None),
+            Some(TreeValue::Tree(id)) => Some(Some(id)),
+            Some(_) => None,
+        });
+        if let Some(tree_id_merge) = tree_id_merge {
+            let get_tree = |id: &Option<&TreeId>| -> BackendResult<Tree> {
+                if let Some(id) = id {
+                    store.get_tree(dir, id)
+                } else {
+                    Ok(Tree::empty(store.clone(), dir.to_owned()))
+                }
+            };
+            Ok(Some(tree_id_merge.try_map(get_tree)?))
+        } else {
+            Ok(None)
+        }
+    }
+
     /// Creates a new merge with the file ids from the given merge. In other
     /// words, only the executable bits from `self` will be preserved.
-    pub fn with_new_file_ids(&self, file_ids: &Merge<Option<FileId>>) -> Self {
+    pub fn with_new_file_ids(&self, file_ids: &Merge<Option<FileId>>) -> Merge<Option<TreeValue>> {
         assert_eq!(self.values.len(), file_ids.values.len());
         let values = zip(self.iter(), file_ids.iter())
             .map(|(tree_value, file_id)| {
-                if let Some(TreeValue::File { id: _, executable }) = tree_value {
+                if let Some(TreeValue::File { id: _, executable }) =
+                    borrow_tree_value(tree_value.as_ref())
+                {
                     Some(TreeValue::File {
                         id: file_id.as_ref().unwrap().clone(),
                         executable: *executable,
@@ -595,48 +633,19 @@ impl MergedTreeValue {
     pub fn describe(&self, file: &mut dyn Write) -> std::io::Result<()> {
         file.write_all(b"Conflict:\n")?;
         for term in self.removes().flatten() {
-            file.write_all(format!("  Removing {}\n", describe_conflict_term(term)).as_bytes())?;
+            file.write_all(
+                format!("  Removing {}\n", describe_conflict_term(term.borrow())).as_bytes(),
+            )?;
         }
         for term in self.adds().flatten() {
-            file.write_all(format!("  Adding {}\n", describe_conflict_term(term)).as_bytes())?;
+            file.write_all(
+                format!("  Adding {}\n", describe_conflict_term(term.borrow())).as_bytes(),
+            )?;
         }
         Ok(())
     }
 }
 
-impl<T> Merge<Option<T>>
-where
-    T: Borrow<TreeValue>,
-{
-    /// If every non-`None` term of a `MergedTreeValue`
-    /// is a `TreeValue::Tree`, this converts it to
-    /// a `Merge<Tree>`, with empty trees instead of
-    /// any `None` terms. Otherwise, returns `None`.
-    pub fn to_tree_merge(
-        &self,
-        store: &Arc<Store>,
-        dir: &RepoPath,
-    ) -> BackendResult<Option<Merge<Tree>>> {
-        let tree_id_merge = self.maybe_map(|term| match borrow_tree_value(term.as_ref()) {
-            None => Some(None),
-            Some(TreeValue::Tree(id)) => Some(Some(id)),
-            Some(_) => None,
-        });
-        if let Some(tree_id_merge) = tree_id_merge {
-            let get_tree = |id: &Option<&TreeId>| -> BackendResult<Tree> {
-                if let Some(id) = id {
-                    store.get_tree(dir, id)
-                } else {
-                    Ok(Tree::empty(store.clone(), dir.to_owned()))
-                }
-            };
-            Ok(Some(tree_id_merge.try_map(get_tree)?))
-        } else {
-            Ok(None)
-        }
-    }
-}
-
 fn borrow_tree_value<T: Borrow<TreeValue> + ?Sized>(term: Option<&T>) -> Option<&TreeValue> {
     term.map(|value| value.borrow())
 }