| Submitter | phabricator |
|---|---|
| Date | July 4, 2019, 3 p.m. |
| Message ID | <b832118416ecb0ef87a5e6d48d1399ca@localhost.localdomain> |
| Download | mbox | patch |
| Permalink | /patch/40754/ |
| State | Not Applicable |
| Headers | show |
Comments
> impl DirsMultiset { > /// Initializes the multiset from a dirstate or a manifest. > /// > @@ -132,6 +123,22 @@ > > Ok(()) > } > + > + pub fn contains_key(&self, key: &[u8]) -> bool { > + self.inner.contains_key(key) > + } > + > + pub fn iter(&self) -> Iter<Vec<u8>, u32> { > + self.inner.iter() > + } > + > + pub fn len(&self) -> usize { > + self.inner.len() > + } > + > + pub fn get(&self, key: &[u8]) -> Option<&u32> { > + self.inner.get(key) > + } Maybe these can be: - pub contains() -> inner.contains_key() - pub iter() -> inner.keys() - pub len() -> inner.len() - (not pub) get() -> inner.get() The point is that DirsMultiset should be more like a set regarding public API. `get() -> inner.get()` can be removed if we rewrite the tests to peek inner.get(), but I don't care much about that.
yuja added a comment. > impl DirsMultiset { > > /// Initializes the multiset from a dirstate or a manifest. > /// > > @@ -132,6 +123,22 @@ > > Ok(()) > } > > + > + pub fn contains_key(&self, key: &[u8]) -> bool { > + self.inner.contains_key(key) > + } > + > + pub fn iter(&self) -> Iter<Vec<u8>, u32> { > + self.inner.iter() > + } > + > + pub fn len(&self) -> usize { > + self.inner.len() > + } > + > + pub fn get(&self, key: &[u8]) -> Option<&u32> { > + self.inner.get(key) > + } Maybe these can be: - pub contains() -> inner.contains_key() - pub iter() -> inner.keys() - pub len() -> inner.len() - (not pub) get() -> inner.get() The point is that DirsMultiset should be more like a set regarding public API. `get() -> inner.get()` can be removed if we rewrite the tests to peek inner.get(), but I don't care much about that. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6593/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6593 To: Alphare, #hg-reviewers Cc: yuja, durin42, kevincox, mercurial-devel
Patch
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 @@ -8,9 +8,8 @@ //! A multiset of directory names. //! //! Used to counts the references to directories in a manifest or dirstate. -use std::collections::hash_map::Entry; +use std::collections::hash_map::{Entry, Iter}; use std::collections::HashMap; -use std::ops::Deref; use {DirsIterable, DirstateEntry, DirstateMapError}; #[derive(PartialEq, Debug)] @@ -18,14 +17,6 @@ inner: HashMap<Vec<u8>, u32>, } -impl Deref for DirsMultiset { - type Target = HashMap<Vec<u8>, u32>; - - fn deref(&self) -> &Self::Target { - &self.inner - } -} - impl DirsMultiset { /// Initializes the multiset from a dirstate or a manifest. /// @@ -132,6 +123,22 @@ Ok(()) } + + pub fn contains_key(&self, key: &[u8]) -> bool { + self.inner.contains_key(key) + } + + pub fn iter(&self) -> Iter<Vec<u8>, u32> { + self.inner.iter() + } + + pub fn len(&self) -> usize { + self.inner.len() + } + + pub fn get(&self, key: &[u8]) -> Option<&u32> { + self.inner.get(key) + } } #[cfg(test)]