D7524: rust-dirs-multiset: use `AsRef` instead of concrete types when possible
Alphare created this revision. Herald added subscribers: mercurial-devel, kevincox, durin42. Herald added a reviewer: hg-reviewers. REVISION SUMMARY I also renamed `vec` to `dirstate`, because it was not a great name. REPOSITORY rHG Mercurial BRANCH default REVISION DETAIL https://phab.mercurial-scm.org/D7524 AFFECTED FILES rust/hg-core/src/dirstate/dirs_multiset.rs CHANGE DETAILS diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs --- a/rust/hg-core/src/dirstate/dirs_multiset.rs +++ b/rust/hg-core/src/dirstate/dirs_multiset.rs @@ -28,14 +28,14 @@ /// /// If `skip_state` is provided, skips dirstate entries with equal state. pub fn from_dirstate( -vec: &FastHashMap, +dirstate: &FastHashMap, skip_state: Option, ) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for (filename, DirstateEntry { state, .. }) in vec { +for (filename, DirstateEntry { state, .. }) in dirstate { // This `if` is optimized out of the loop if let Some(skip) = skip_state { if skip != *state { @@ -50,13 +50,13 @@ } /// Initializes the multiset from a manifest. -pub fn from_manifest(vec: &Vec) -> Self { +pub fn from_manifest(manifest: &[impl AsRef]) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for filename in vec { -multiset.add_path(filename); +for filename in manifest { +multiset.add_path(filename.as_ref()); } multiset @@ -65,8 +65,8 @@ /// Increases the count of deepest directory contained in the path. /// /// If the directory is not yet in the map, adds its parents. -pub fn add_path(&mut self, path: &HgPath) { -for subpath in files::find_dirs(path) { +pub fn add_path(&mut self, path: impl AsRef) { +for subpath in files::find_dirs(path.as_ref()) { if let Some(val) = self.inner.get_mut(subpath) { *val += 1; break; @@ -82,9 +82,9 @@ /// If the directory is not in the map, something horrible has happened. pub fn delete_path( &mut self, -path: &HgPath, +path: impl AsRef, ) -> Result<(), DirstateMapError> { -for subpath in files::find_dirs(path) { +for subpath in files::find_dirs(path.as_ref()) { match self.inner.entry(subpath.to_owned()) { Entry::Occupied(mut entry) => { let val = entry.get().clone(); @@ -96,7 +96,7 @@ } Entry::Vacant(_) => { return Err(DirstateMapError::PathNotFound( -path.to_owned(), +path.as_ref().to_owned(), )) } }; @@ -105,8 +105,8 @@ Ok(()) } -pub fn contains(&self, key: &HgPath) -> bool { -self.inner.contains_key(key) +pub fn contains(&self, key: impl AsRef) -> bool { +self.inner.contains_key(key.as_ref()) } pub fn iter(&self) -> DirsMultisetIter { @@ -124,7 +124,8 @@ #[test] fn test_delete_path_path_not_found() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPathBuf::from_bytes(b"doesnotexist/"); assert_eq!( Err(DirstateMapError::PathNotFound(path.to_owned())), @@ -180,7 +181,8 @@ #[test] fn test_add_path_empty_path() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPath::new(b""); map.add_path(path); @@ -189,7 +191,8 @@ #[test] fn test_add_path_successful() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); map.add_path(HgPath::new(b"a/")); assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap()); @@ -234,7 +237,8 @@ #[test] fn test_dirsmultiset_new_empty() { -let new = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let new = DirsMultiset::from_manifest(&manifest); let expected = DirsMultiset { inner: FastHashMap::default(), }; @@ -249,7 +253,7 @@ #[test] fn test_dirsmultiset_new_no_skip() { -let input_vec = ["a/", "b/", "a/c", "a/d/"] +let input_vec: Vec = ["a/", "b/", "a/c", "a/d/"] .iter() .map(|e| HgPathBuf::from_bytes(e.as_bytes())) .collect(); To: Alphare, #hg-reviewers Cc: durin42, kevincox, mercurial-devel __
D7524: rust-dirs-multiset: use `AsRef` instead of concrete types when possible
pulkit added a comment. This one fails to apply on default. Skipping pushing this and it's accepted children. REPOSITORY rHG Mercurial BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7524/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7524 To: Alphare, #hg-reviewers, kevincox, pulkit Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7524: rust-dirs-multiset: use `AsRef` instead of concrete types when possible
Alphare updated this revision to Diff 18566. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7524?vs=18400&id=18566 BRANCH default CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7524/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7524 AFFECTED FILES rust/hg-core/src/dirstate/dirs_multiset.rs CHANGE DETAILS diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs --- a/rust/hg-core/src/dirstate/dirs_multiset.rs +++ b/rust/hg-core/src/dirstate/dirs_multiset.rs @@ -28,14 +28,14 @@ /// /// If `skip_state` is provided, skips dirstate entries with equal state. pub fn from_dirstate( -vec: &FastHashMap, +dirstate: &FastHashMap, skip_state: Option, ) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for (filename, DirstateEntry { state, .. }) in vec { +for (filename, DirstateEntry { state, .. }) in dirstate { // This `if` is optimized out of the loop if let Some(skip) = skip_state { if skip != *state { @@ -50,13 +50,13 @@ } /// Initializes the multiset from a manifest. -pub fn from_manifest(vec: &Vec) -> Self { +pub fn from_manifest(manifest: &[impl AsRef]) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for filename in vec { -multiset.add_path(filename); +for filename in manifest { +multiset.add_path(filename.as_ref()); } multiset @@ -65,8 +65,11 @@ /// Increases the count of deepest directory contained in the path. /// /// If the directory is not yet in the map, adds its parents. -pub fn add_path(&mut self, path: &HgPath) -> Result<(), DirstateMapError> { -for subpath in files::find_dirs(path) { +pub fn add_path( +&mut self, +path: impl AsRef, +) -> Result<(), DirstateMapError> { +for subpath in files::find_dirs(path.as_ref()) { if subpath.as_bytes().last() == Some(&b'/') { // TODO Remove this once PathAuditor is certified // as the only entrypoint for path data @@ -88,9 +91,9 @@ /// If the directory is not in the map, something horrible has happened. pub fn delete_path( &mut self, -path: &HgPath, +path: impl AsRef, ) -> Result<(), DirstateMapError> { -for subpath in files::find_dirs(path) { +for subpath in files::find_dirs(path.as_ref()) { match self.inner.entry(subpath.to_owned()) { Entry::Occupied(mut entry) => { let val = entry.get().clone(); @@ -102,7 +105,7 @@ } Entry::Vacant(_) => { return Err(DirstateMapError::PathNotFound( -path.to_owned(), +path.as_ref().to_owned(), )) } }; @@ -111,8 +114,8 @@ Ok(()) } -pub fn contains(&self, key: &HgPath) -> bool { -self.inner.contains_key(key) +pub fn contains(&self, key: impl AsRef) -> bool { +self.inner.contains_key(key.as_ref()) } pub fn iter(&self) -> DirsMultisetIter { @@ -130,7 +133,8 @@ #[test] fn test_delete_path_path_not_found() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPathBuf::from_bytes(b"doesnotexist/"); assert_eq!( Err(DirstateMapError::PathNotFound(path.to_owned())), @@ -186,7 +190,8 @@ #[test] fn test_add_path_empty_path() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPath::new(b""); map.add_path(path); @@ -195,7 +200,8 @@ #[test] fn test_add_path_successful() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); map.add_path(HgPath::new(b"a/")); assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap()); @@ -240,7 +246,8 @@ #[test] fn test_dirsmultiset_new_empty() { -let new = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let new = DirsMultiset::from_manifest(&manifest); let expected = DirsMultiset { inner: FastHashMap::default(), }; @@ -255,7 +262,7 @@ #[test] fn test_dirsmultiset_new_no_skip() { -let input_vec = ["a/", "b/", "a/c", "a/d/"] +let input_vec: Vec = ["a/", "b/", "a/c", "a/d/"] .iter() .map(|e| HgPa
D7524: rust-dirs-multiset: use `AsRef` instead of concrete types when possible
Closed by commit rHG088ba9d94079: rust-dirs-multiset: use `AsRef` instead of concrete types when possible (authored by Alphare). This revision was automatically updated to reflect the committed changes. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D7524?vs=18566&id=18582 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7524/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7524 AFFECTED FILES rust/hg-core/src/dirstate/dirs_multiset.rs CHANGE DETAILS diff --git a/rust/hg-core/src/dirstate/dirs_multiset.rs b/rust/hg-core/src/dirstate/dirs_multiset.rs --- a/rust/hg-core/src/dirstate/dirs_multiset.rs +++ b/rust/hg-core/src/dirstate/dirs_multiset.rs @@ -28,14 +28,14 @@ /// /// If `skip_state` is provided, skips dirstate entries with equal state. pub fn from_dirstate( -vec: &FastHashMap, +dirstate: &FastHashMap, skip_state: Option, ) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for (filename, DirstateEntry { state, .. }) in vec { +for (filename, DirstateEntry { state, .. }) in dirstate { // This `if` is optimized out of the loop if let Some(skip) = skip_state { if skip != *state { @@ -50,13 +50,13 @@ } /// Initializes the multiset from a manifest. -pub fn from_manifest(vec: &Vec) -> Self { +pub fn from_manifest(manifest: &[impl AsRef]) -> Self { let mut multiset = DirsMultiset { inner: FastHashMap::default(), }; -for filename in vec { -multiset.add_path(filename); +for filename in manifest { +multiset.add_path(filename.as_ref()); } multiset @@ -65,8 +65,11 @@ /// Increases the count of deepest directory contained in the path. /// /// If the directory is not yet in the map, adds its parents. -pub fn add_path(&mut self, path: &HgPath) -> Result<(), DirstateMapError> { -for subpath in files::find_dirs(path) { +pub fn add_path( +&mut self, +path: impl AsRef, +) -> Result<(), DirstateMapError> { +for subpath in files::find_dirs(path.as_ref()) { if subpath.as_bytes().last() == Some(&b'/') { // TODO Remove this once PathAuditor is certified // as the only entrypoint for path data @@ -88,9 +91,9 @@ /// If the directory is not in the map, something horrible has happened. pub fn delete_path( &mut self, -path: &HgPath, +path: impl AsRef, ) -> Result<(), DirstateMapError> { -for subpath in files::find_dirs(path) { +for subpath in files::find_dirs(path.as_ref()) { match self.inner.entry(subpath.to_owned()) { Entry::Occupied(mut entry) => { let val = entry.get().clone(); @@ -102,7 +105,7 @@ } Entry::Vacant(_) => { return Err(DirstateMapError::PathNotFound( -path.to_owned(), +path.as_ref().to_owned(), )) } }; @@ -111,8 +114,8 @@ Ok(()) } -pub fn contains(&self, key: &HgPath) -> bool { -self.inner.contains_key(key) +pub fn contains(&self, key: impl AsRef) -> bool { +self.inner.contains_key(key.as_ref()) } pub fn iter(&self) -> DirsMultisetIter { @@ -130,7 +133,8 @@ #[test] fn test_delete_path_path_not_found() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPathBuf::from_bytes(b"doesnotexist/"); assert_eq!( Err(DirstateMapError::PathNotFound(path.to_owned())), @@ -186,7 +190,8 @@ #[test] fn test_add_path_empty_path() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); let path = HgPath::new(b""); map.add_path(path); @@ -195,7 +200,8 @@ #[test] fn test_add_path_successful() { -let mut map = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let mut map = DirsMultiset::from_manifest(&manifest); map.add_path(HgPath::new(b"a/")); assert_eq!(1, *map.inner.get(HgPath::new(b"a")).unwrap()); @@ -240,7 +246,8 @@ #[test] fn test_dirsmultiset_new_empty() { -let new = DirsMultiset::from_manifest(&vec![]); +let manifest: Vec = vec![]; +let new = DirsMultiset::from_manifest(&manifest); let expected = DirsMultiset { inner: FastHashMap::default(), }; @@ -255,7 +262,7 @@ #[test] fn test_dirsmultiset_new_no_skip() { -let inpu