D6593: rust-minor-fixes: remove Deref in favor of explicit methods

kevincox added inline comments. INLINE COMMENTS > dirs_multiset.rs:131 > + > +pub fn iter() -> Iter, u32> { > +self.inner.iter() Returning a `std::collections::hash_map::Iter` binds you to this implementation. I would recommend instead returning `impl Iterator`. REPOSITORY rHG

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Alphare added a comment. In D6593#96417 , @yuja wrote: >> Using `keys()` will change the behavior, as I actually need to expose an `Iter, u32>`. Is your issue with the naming? > > No. My point is that the refcount value is an

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

yuja added a comment. > Using `keys()` will change the behavior, as I actually need to expose an `Iter, u32>`. Is your issue with the naming? No. My point is that the refcount value is an implementation detail. Do you have some plan to use the refcount value? diff --git

Re: D6593: rust-minor-fixes: remove Deref in favor of explicit methods

> Using `keys()` will change the behavior, as I actually need to expose an > `Iter, u32>`. Is your issue with the naming? No. My point is that the refcount value is an implementation detail. Do you have some plan to use the refcount value? ``` diff --git

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Alphare added a comment. > Again, > > - **contains**() -> inner.contains_key() > - iter() -> inner.**keys**() > > Somewhat similar to HashSet, which is basically a proxy type to > HashMap. `contains()` is my bad, I forgot. Using `keys()` will change the behavior, as

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Closed by commit rHGa80464e85ddd: rust: remove Deref in favor of explicit methods (authored by Alphare). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG Mercurial

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

yuja added a comment. > +pub fn contains_key(, key: &[u8]) -> bool { > +self.inner.contains_key(key) > +} > + > +pub fn iter() -> Iter, u32> { > +self.inner.iter() > +} Again, - **contains**() -> inner.contains_key() - iter() ->

Re: D6593: rust-minor-fixes: remove Deref in favor of explicit methods

> +pub fn contains_key(, key: &[u8]) -> bool { > +self.inner.contains_key(key) > +} > + > +pub fn iter() -> Iter, u32> { > +self.inner.iter() > +} Again, - **contains**() -> inner.contains_key() - iter() -> inner.**keys**() Somewhat similar to HashSet, which is

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Alphare updated this revision to Diff 15771. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6593?vs=15740=15771 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6593/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6593 AFFECTED FILES

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

yuja added a comment. > impl DirsMultiset { > > /// Initializes the multiset from a dirstate or a manifest. > /// > > @@ -132,6 +123,22 @@ > > Ok(()) > } > > + > +pub fn contains_key(, key: &[u8]) -> bool { > +self.inner.contains_key(key) > +

Re: D6593: rust-minor-fixes: remove Deref in favor of explicit methods

> impl DirsMultiset { > /// Initializes the multiset from a dirstate or a manifest. > /// > @@ -132,6 +123,22 @@ > > Ok(()) > } > + > +pub fn contains_key(, key: &[u8]) -> bool { > +self.inner.contains_key(key) > +} > + > +pub fn iter() -> Iter, u32>

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Alphare updated this revision to Diff 15740. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D6593?vs=15729=15740 CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D6593/new/ REVISION DETAIL https://phab.mercurial-scm.org/D6593 AFFECTED FILES

D6593: rust-minor-fixes: remove Deref in favor of explicit methods

Alphare created this revision. Herald added subscribers: mercurial-devel, kevincox, durin42. Herald added a reviewer: hg-reviewers. Alphare updated this revision to Diff 15740. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D6593 AFFECTED FILES