D6393: rust-dirstate: add "dirs" Rust implementation
kevincox (Kevin Cox)
phabricator at mercurial-scm.org
Mon Jun 24 09:46:51 EDT 2019
kevincox added a comment.
kevincox accepted this revision.
Sorry. I was busy. In general don't worry about blocking on me, I can't promise any sort of reasonable response time. Worst case I can review the changes after submission and changes can be made afterwards.
INLINE COMMENTS
> dirs_multiset.rs:43
> + if let Some(skip) = skip_state {
> + if skip != state {
> + multiset.add_path(filename);
You can replate the nested if with:
if skip_state == Some(state) {
> dirs_multiset.rs:63
> + /// without trailing slash, from right to left.
> + fn find_dir(path: &[u8], mut pos: usize) -> Option<&[u8]> {
> + loop {
I would remove the `pos` argument. IIUC following two are currently identical.
find_dir(path, n);
find_dir(path[..n], n);
If you remove the second argument then you can just always remove the last component of the path. This will also allow you to have a more clear doc comment. Right now I find it a little confusing.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D6393/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D6393
To: Alphare, #hg-reviewers, kevincox
Cc: martinvonz, durin42, kevincox, mercurial-devel
More information about the Mercurial-devel
mailing list