rust-dirstate: split DirsMultiset constructor per input type
authorYuya Nishihara <yuya@tcha.org>
Sat, 17 Aug 2019 18:25:29 +0900
changeset 42802 2e1f74cc3350
parent 42801 1a535313ad1b
child 42803 8ab1ae7f1cf4
rust-dirstate: split DirsMultiset constructor per input type Since skip_state only applies to dirstate, it doesn't make sense to unify these constructors and dispatch by enum.
rust/hg-core/src/dirstate.rs
rust/hg-core/src/dirstate/dirs_multiset.rs
rust/hg-core/src/dirstate/dirstate_map.rs
rust/hg-core/src/lib.rs
rust/hg-cpython/src/dirstate/dirs_multiset.rs
rust/hg-cpython/src/dirstate/dirstate_map.rs
rust/hg-cpython/src/parsers.rs
--- a/rust/hg-core/src/dirstate.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-core/src/dirstate.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -33,13 +33,6 @@
 pub type StateMap = HashMap<Vec<u8>, DirstateEntry>;
 pub type CopyMap = HashMap<Vec<u8>, Vec<u8>>;
 
-/// The Python implementation passes either a mapping (dirstate) or a flat
-/// iterable (manifest)
-pub enum DirsIterable<'a> {
-    Dirstate(&'a HashMap<Vec<u8>, DirstateEntry>),
-    Manifest(&'a Vec<Vec<u8>>),
-}
-
 #[derive(Copy, Clone, Debug, Eq, PartialEq)]
 pub enum EntryState {
     Normal,
--- a/rust/hg-core/src/dirstate/dirs_multiset.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-core/src/dirstate/dirs_multiset.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -9,8 +9,7 @@
 //!
 //! Used to counts the references to directories in a manifest or dirstate.
 use crate::{
-    dirstate::EntryState, utils::files, DirsIterable, DirstateEntry,
-    DirstateMapError,
+    dirstate::EntryState, utils::files, DirstateEntry, DirstateMapError,
 };
 use std::collections::hash_map::Entry;
 use std::collections::HashMap;
@@ -21,40 +20,44 @@
 }
 
 impl DirsMultiset {
-    /// Initializes the multiset from a dirstate or a manifest.
+    /// Initializes the multiset from a dirstate.
     ///
     /// If `skip_state` is provided, skips dirstate entries with equal state.
-    pub fn new(
-        iterable: DirsIterable,
+    pub fn from_dirstate(
+        vec: &HashMap<Vec<u8>, DirstateEntry>,
         skip_state: Option<EntryState>,
     ) -> Self {
         let mut multiset = DirsMultiset {
             inner: HashMap::new(),
         };
 
-        match iterable {
-            DirsIterable::Dirstate(vec) => {
-                for (filename, DirstateEntry { state, .. }) in vec {
-                    // This `if` is optimized out of the loop
-                    if let Some(skip) = skip_state {
-                        if skip != *state {
-                            multiset.add_path(filename);
-                        }
-                    } else {
-                        multiset.add_path(filename);
-                    }
-                }
-            }
-            DirsIterable::Manifest(vec) => {
-                for filename in vec {
+        for (filename, DirstateEntry { state, .. }) in vec {
+            // This `if` is optimized out of the loop
+            if let Some(skip) = skip_state {
+                if skip != *state {
                     multiset.add_path(filename);
                 }
+            } else {
+                multiset.add_path(filename);
             }
         }
 
         multiset
     }
 
+    /// Initializes the multiset from a manifest.
+    pub fn from_manifest(vec: &Vec<Vec<u8>>) -> Self {
+        let mut multiset = DirsMultiset {
+            inner: HashMap::new(),
+        };
+
+        for filename in vec {
+            multiset.add_path(filename);
+        }
+
+        multiset
+    }
+
     /// Increases the count of deepest directory contained in the path.
     ///
     /// If the directory is not yet in the map, adds its parents.
@@ -118,7 +121,7 @@
 
     #[test]
     fn test_delete_path_path_not_found() {
-        let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
+        let mut map = DirsMultiset::from_manifest(&vec![]);
         let path = b"doesnotexist/";
         assert_eq!(
             Err(DirstateMapError::PathNotFound(path.to_vec())),
@@ -128,8 +131,7 @@
 
     #[test]
     fn test_delete_path_empty_path() {
-        let mut map =
-            DirsMultiset::new(DirsIterable::Manifest(&vec![vec![]]), None);
+        let mut map = DirsMultiset::from_manifest(&vec![vec![]]);
         let path = b"";
         assert_eq!(Ok(()), map.delete_path(path));
         assert_eq!(
@@ -169,7 +171,7 @@
 
     #[test]
     fn test_add_path_empty_path() {
-        let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
+        let mut map = DirsMultiset::from_manifest(&vec![]);
         let path = b"";
         map.add_path(path);
 
@@ -178,7 +180,7 @@
 
     #[test]
     fn test_add_path_successful() {
-        let mut map = DirsMultiset::new(DirsIterable::Manifest(&vec![]), None);
+        let mut map = DirsMultiset::from_manifest(&vec![]);
 
         map.add_path(b"a/");
         assert_eq!(1, *map.inner.get(&b"a".to_vec()).unwrap());
@@ -223,15 +225,13 @@
 
     #[test]
     fn test_dirsmultiset_new_empty() {
-        use DirsIterable::{Dirstate, Manifest};
-
-        let new = DirsMultiset::new(Manifest(&vec![]), None);
+        let new = DirsMultiset::from_manifest(&vec![]);
         let expected = DirsMultiset {
             inner: HashMap::new(),
         };
         assert_eq!(expected, new);
 
-        let new = DirsMultiset::new(Dirstate(&HashMap::new()), None);
+        let new = DirsMultiset::from_dirstate(&HashMap::new(), None);
         let expected = DirsMultiset {
             inner: HashMap::new(),
         };
@@ -240,8 +240,6 @@
 
     #[test]
     fn test_dirsmultiset_new_no_skip() {
-        use DirsIterable::{Dirstate, Manifest};
-
         let input_vec = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|e| e.as_bytes().to_vec())
@@ -251,7 +249,7 @@
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
-        let new = DirsMultiset::new(Manifest(&input_vec), None);
+        let new = DirsMultiset::from_manifest(&input_vec);
         let expected = DirsMultiset {
             inner: expected_inner,
         };
@@ -276,7 +274,7 @@
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
-        let new = DirsMultiset::new(Dirstate(&input_map), None);
+        let new = DirsMultiset::from_dirstate(&input_map, None);
         let expected = DirsMultiset {
             inner: expected_inner,
         };
@@ -285,8 +283,6 @@
 
     #[test]
     fn test_dirsmultiset_new_skip() {
-        use DirsIterable::{Dirstate, Manifest};
-
         let input_vec = ["a/", "b/", "a/c", "a/d/"]
             .iter()
             .map(|e| e.as_bytes().to_vec())
@@ -296,8 +292,9 @@
             .map(|(k, v)| (k.as_bytes().to_vec(), *v))
             .collect();
 
-        let new =
-            DirsMultiset::new(Manifest(&input_vec), Some(EntryState::Normal));
+        // this was
+        // DirsMultiset::new(Manifest(&input_vec), Some(EntryState::Normal))
+        let new = DirsMultiset::from_manifest(&input_vec);
         let expected = DirsMultiset {
             inner: expected_inner,
         };
@@ -331,7 +328,7 @@
             .collect();
 
         let new =
-            DirsMultiset::new(Dirstate(&input_map), Some(EntryState::Normal));
+            DirsMultiset::from_dirstate(&input_map, Some(EntryState::Normal));
         let expected = DirsMultiset {
             inner: expected_inner,
         };
--- a/rust/hg-core/src/dirstate/dirstate_map.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-core/src/dirstate/dirstate_map.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -7,9 +7,9 @@
 
 use crate::{
     dirstate::{parsers::PARENT_SIZE, EntryState},
-    pack_dirstate, parse_dirstate, CopyMap, DirsIterable, DirsMultiset,
-    DirstateEntry, DirstateError, DirstateMapError, DirstateParents,
-    DirstateParseError, StateMap,
+    pack_dirstate, parse_dirstate, CopyMap, DirsMultiset, DirstateEntry,
+    DirstateError, DirstateMapError, DirstateParents, DirstateParseError,
+    StateMap,
 };
 use core::borrow::Borrow;
 use std::collections::{HashMap, HashSet};
@@ -224,17 +224,15 @@
     /// good idea.
     pub fn set_all_dirs(&mut self) {
         if self.all_dirs.is_none() {
-            self.all_dirs = Some(DirsMultiset::new(
-                DirsIterable::Dirstate(&self.state_map),
-                None,
-            ));
+            self.all_dirs =
+                Some(DirsMultiset::from_dirstate(&self.state_map, None));
         }
     }
 
     pub fn set_dirs(&mut self) {
         if self.dirs.is_none() {
-            self.dirs = Some(DirsMultiset::new(
-                DirsIterable::Dirstate(&self.state_map),
+            self.dirs = Some(DirsMultiset::from_dirstate(
+                &self.state_map,
                 Some(EntryState::Removed),
             ));
         }
--- a/rust/hg-core/src/lib.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-core/src/lib.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -12,8 +12,7 @@
     dirs_multiset::DirsMultiset,
     dirstate_map::DirstateMap,
     parsers::{pack_dirstate, parse_dirstate, PARENT_SIZE},
-    CopyMap, DirsIterable, DirstateEntry, DirstateParents, EntryState,
-    StateMap,
+    CopyMap, DirstateEntry, DirstateParents, EntryState, StateMap,
 };
 mod filepatterns;
 pub mod utils;
--- a/rust/hg-cpython/src/dirstate/dirs_multiset.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-cpython/src/dirstate/dirs_multiset.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -17,10 +17,7 @@
 };
 
 use crate::{dirstate::extract_dirstate, ref_sharing::PySharedState};
-use hg::{
-    DirsIterable, DirsMultiset, DirstateMapError, DirstateParseError,
-    EntryState,
-};
+use hg::{DirsMultiset, DirstateMapError, DirstateParseError, EntryState};
 
 py_class!(pub class Dirs |py| {
     data inner: RefCell<DirsMultiset>;
@@ -45,19 +42,13 @@
         }
         let inner = if let Ok(map) = map.cast_as::<PyDict>(py) {
             let dirstate = extract_dirstate(py, &map)?;
-            DirsMultiset::new(
-                DirsIterable::Dirstate(&dirstate),
-                skip_state,
-            )
+            DirsMultiset::from_dirstate(&dirstate, skip_state)
         } else {
             let map: Result<Vec<Vec<u8>>, PyErr> = map
                 .iter(py)?
                 .map(|o| Ok(o?.extract::<PyBytes>(py)?.data(py).to_owned()))
                 .collect();
-            DirsMultiset::new(
-                DirsIterable::Manifest(&map?),
-                skip_state,
-            )
+            DirsMultiset::from_manifest(&map?)
         };
 
         Self::create_instance(
--- a/rust/hg-cpython/src/dirstate/dirstate_map.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-cpython/src/dirstate/dirstate_map.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -24,7 +24,7 @@
     ref_sharing::PySharedState,
 };
 use hg::{
-    DirsIterable, DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
+    DirsMultiset, DirstateEntry, DirstateMap as RustDirstateMap,
     DirstateParents, DirstateParseError, EntryState, PARENT_SIZE,
 };
 
@@ -356,8 +356,8 @@
         self.inner(py).borrow_mut().set_dirs();
         Dirs::from_inner(
             py,
-            DirsMultiset::new(
-                DirsIterable::Dirstate(&self.inner(py).borrow()),
+            DirsMultiset::from_dirstate(
+                &self.inner(py).borrow(),
                 Some(EntryState::Removed),
             ),
         )
@@ -367,8 +367,8 @@
         self.inner(py).borrow_mut().set_all_dirs();
         Dirs::from_inner(
             py,
-            DirsMultiset::new(
-                DirsIterable::Dirstate(&self.inner(py).borrow()),
+            DirsMultiset::from_dirstate(
+                &self.inner(py).borrow(),
                 None,
             ),
         )
--- a/rust/hg-cpython/src/parsers.rs	Sat Aug 17 16:33:05 2019 +0900
+++ b/rust/hg-cpython/src/parsers.rs	Sat Aug 17 18:25:29 2019 +0900
@@ -15,8 +15,8 @@
     PythonObject, ToPyObject,
 };
 use hg::{
-    pack_dirstate, parse_dirstate, DirstateEntry,
-    DirstatePackError, DirstateParents, DirstateParseError, PARENT_SIZE,
+    pack_dirstate, parse_dirstate, DirstateEntry, DirstatePackError,
+    DirstateParents, DirstateParseError, PARENT_SIZE,
 };
 use std::collections::HashMap;
 use std::convert::TryInto;