From f1e2124a64b9ebc7c9bca4029755077f085e1b55 Mon Sep 17 00:00:00 2001
From: Martin von Zweigbergk <martinvonz@google.com>
Date: Wed, 20 Oct 2021 22:00:52 -0700
Subject: [PATCH] trees: make `simplify_conflict()` always return a `Conflict`

The interface is much cleaner this way. The function no longer makes
any writes to the store.
---
 lib/src/tree.rs | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/lib/src/tree.rs b/lib/src/tree.rs
index f70fc2fc0..865d96276 100644
--- a/lib/src/tree.rs
+++ b/lib/src/tree.rs
@@ -634,7 +634,17 @@ fn merge_tree_value(
                             value: side2.clone(),
                         });
                     }
-                    simplify_conflict(store, &conflict)?
+                    let conflict = simplify_conflict(store, &conflict)?;
+                    if conflict.adds.is_empty() {
+                        // If there are no values to add, then the path doesn't exist
+                        None
+                    } else if conflict.removes.is_empty() && conflict.adds.len() == 1 {
+                        // A single add means that the current state is that state.
+                        Some(conflict.adds[0].value.clone())
+                    } else {
+                        let conflict_id = store.write_conflict(&conflict)?;
+                        Some(TreeValue::Conflict(conflict_id))
+                    }
                 }
             }
         }
@@ -659,7 +669,7 @@ fn conflict_part_to_conflict(store: &Store, part: &ConflictPart) -> Result<Confl
 fn simplify_conflict(
     store: &Store,
     conflict: &Conflict,
-) -> Result<Option<TreeValue>, BackendError> {
+) -> Result<Conflict, BackendError> {
     // Important cases to simplify:
     //
     // D
@@ -738,20 +748,8 @@ fn simplify_conflict(
     // {+A+A}, that would become just {+A}. Similarly {+B-A+B} would be just
     // {+B-A}.
 
-    if new_adds.is_empty() {
-        // If there are no values to add, then the path doesn't exist (so return None to
-        // indicate that).
-        return Ok(None);
-    }
-
-    if new_removes.is_empty() && new_adds.len() == 1 {
-        // A single add means that the current state is that state.
-        return Ok(Some(new_adds[0].value.clone()));
-    }
-
-    let conflict_id = store.write_conflict(&Conflict {
+    Ok(Conflict {
         adds: new_adds,
         removes: new_removes,
-    })?;
-    Ok(Some(TreeValue::Conflict(conflict_id)))
+    })
 }