This is an archive of the discontinued Mercurial Phabricator instance.

rust-hg-path: add method to get part of a path relative to a prefix
ClosedPublic

Authored by Alphare on Nov 29 2019, 12:57 PM.

Details

Summary

This will be used in the next patch in this series.

Diff Detail

Repository
rHG Mercurial
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

Event Timeline

Alphare created this revision.Nov 29 2019, 12:57 PM
kevincox requested changes to this revision.Dec 2 2019, 8:17 AM
kevincox added inline comments.
rust/hg-core/src/utils/hg_path.rs
131

base.is_empty()? If we don't have that helper we should probably add it.

131

The comment says that base must end with a / however you appear to handle cases where it doesn't Can you either update the documentation or the code?

134

Instead of using .last() can we add and use base.ends_with(b'/')?

134

base.len() > 0 is redundant both with the early return above and because last() will return None if base is empty.

215

I would do self.as_bytes() for consistency.

Or even better I believe you can #[derive(Eq,PartialEq)].

This revision now requires changes to proceed.Dec 2 2019, 8:17 AM
Alphare marked 4 inline comments as done.Dec 2 2019, 9:20 AM
Alphare updated this revision to Diff 18422.
Alphare added inline comments.Dec 2 2019, 9:20 AM
rust/hg-core/src/utils/hg_path.rs
131

I've clarified the documentation.

kevincox accepted this revision.Dec 2 2019, 10:15 AM
pulkit accepted this revision.Dec 10 2019, 10:12 AM
This revision is now accepted and ready to land.Dec 10 2019, 10:12 AM
martinvonz added inline comments.
rust/hg-core/src/utils/hg_path.rs
448

The argument order here feels backwards. I know some frameworks (like Java's JUnit) put the expected value first, but the definition of assert_eq! doesn't seem to have any opinion and https://doc.rust-lang.org/stable/rust-by-example/testing/unit_testing.html puts the actual value first. What do you think about changing the order? That would make it more readable to me.

Alphare added inline comments.Dec 11 2019, 3:42 AM
rust/hg-core/src/utils/hg_path.rs
448

It looks that I took the habit because the test runner in PyCharm that I used when starting with Rust prints expected and actual in that order. I can reverse it if you really think it's more readable this way.