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

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



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

Diff Detail

rHG Mercurial
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

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.

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


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?


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


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


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

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.

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 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

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.