diff --git a/lib/src/conflicts.rs b/lib/src/conflicts.rs index a34c5b315..542278645 100644 --- a/lib/src/conflicts.rs +++ b/lib/src/conflicts.rs @@ -114,6 +114,14 @@ impl Conflict { { trivial_merge(&self.removes, &self.adds) } + + /// Creates a new conflict by applying `f` to each remove and add, returning + /// `None if `f` returns `None` for any of them. + pub fn try_map<'a, U>(&'a self, mut f: impl FnMut(&'a T) -> Option) -> Option> { + let removes = self.removes.iter().map(&mut f).collect::>()?; + let adds = self.adds.iter().map(&mut f).collect::>()?; + Some(Conflict { removes, adds }) + } } impl Conflict> { @@ -174,29 +182,14 @@ impl Conflict> { } pub fn to_file_conflict(&self) -> Option>> { - fn collect_file_terms(terms: &[Option]) -> Option>> { - let mut file_terms = vec![]; - for term in terms { - match term { - None => { - file_terms.push(None); - } - Some(TreeValue::File { - id, - executable: false, - }) => { - file_terms.push(Some(id.clone())); - } - _ => { - return None; - } - } - } - Some(file_terms) - } - let file_removes = collect_file_terms(self.removes())?; - let file_adds = collect_file_terms(self.adds())?; - Some(Conflict::new(file_removes, file_adds)) + self.try_map(|term| match term { + None => Some(None), + Some(TreeValue::File { + id, + executable: false, + }) => Some(Some(id.clone())), + _ => None, + }) } /// Give a summary description of the conflict's "removes" and "adds" @@ -752,4 +745,25 @@ mod tests { } } } + + #[test] + fn test_try_map() { + fn sqrt(i: &i32) -> Option { + if *i >= 0 { + Some((*i as f64).sqrt() as i32) + } else { + None + } + } + fn c(removes: &[i32], adds: &[i32]) -> Conflict { + Conflict::new(removes.to_vec(), adds.to_vec()) + } + // 1-way conflict + assert_eq!(c(&[], &[1]).try_map(sqrt), Some(c(&[], &[1]))); + assert_eq!(c(&[], &[-1]).try_map(sqrt), None); + // 3-way conflict + assert_eq!(c(&[1], &[4, 9]).try_map(sqrt), Some(c(&[1], &[2, 3]))); + assert_eq!(c(&[-1], &[4, 9]).try_map(sqrt), None); + assert_eq!(c(&[1], &[-4, 9]).try_map(sqrt), None); + } } diff --git a/lib/src/tree.rs b/lib/src/tree.rs index 311dda62a..4e91e6ae6 100644 --- a/lib/src/tree.rs +++ b/lib/src/tree.rs @@ -637,40 +637,31 @@ fn try_resolve_file_conflict( // merge it. We check early so we don't waste time reading file contents if // we can't merge them anyway. At the same time we determine whether the // resulting file should be executable. - let mut executable_removes = vec![]; - let mut executable_adds = vec![]; - let mut removed_file_ids = vec![]; - let mut added_file_ids = vec![]; - for term in conflict.removes() { - match term { - Some(TreeValue::File { id, executable }) => { - executable_removes.push(*executable); - removed_file_ids.push(id.clone()); - } - _ => { - return Ok(None); - } - } - } - for term in conflict.adds() { - match term { - Some(TreeValue::File { id, executable }) => { - executable_adds.push(*executable); - added_file_ids.push(id.clone()); - } - _ => { - return Ok(None); - } - } - } - let executable = if let Some(executable) = trivial_merge(&executable_removes, &executable_adds) - { + // TODO: Change to let-else once our MSRV is above 1.65 + let file_id_conflict = if let Some(conflict) = conflict.try_map(|term| match term { + Some(TreeValue::File { id, executable: _ }) => Some(id), + _ => None, + }) { + conflict + } else { + return Ok(None); + }; + // TODO: Change to let-else once our MSRV is above 1.65 + let executable_conflict = if let Some(conflict) = conflict.try_map(|term| match term { + Some(TreeValue::File { id: _, executable }) => Some(executable), + _ => None, + }) { + conflict + } else { + return Ok(None); + }; + let executable = if let Some(&executable) = executable_conflict.resolve_trivial() { *executable } else { // We're unable to determine whether the result should be executable return Ok(None); }; - if let Some(resolved_file_id) = trivial_merge(&removed_file_ids, &added_file_ids) { + if let Some(&resolved_file_id) = file_id_conflict.resolve_trivial() { // Don't bother reading the file contents if the conflict can be trivially // resolved. return Ok(Some(TreeValue::File { @@ -680,25 +671,25 @@ fn try_resolve_file_conflict( } let mut removed_contents = vec![]; let mut added_contents = vec![]; - for file_id in removed_file_ids { + for &file_id in file_id_conflict.removes() { let mut content = vec![]; store - .read_file(filename, &file_id)? + .read_file(filename, file_id)? .read_to_end(&mut content) .map_err(|err| TreeMergeError::ReadError { source: err, - file_id, + file_id: file_id.clone(), })?; removed_contents.push(content); } - for file_id in added_file_ids { + for &file_id in file_id_conflict.adds() { let mut content = vec![]; store - .read_file(filename, &file_id)? + .read_file(filename, file_id)? .read_to_end(&mut content) .map_err(|err| TreeMergeError::ReadError { source: err, - file_id, + file_id: file_id.clone(), })?; added_contents.push(content); }