D6773: rust-hgpath: add HgPath and HgPathBuf structs to encapsulate handling of paths
Alphare (Raphaël Gomès)
phabricator at mercurial-scm.org
Sun Sep 1 09:30:37 UTC 2019
Alphare added a comment.
> @kevincox said "it would be good to [...] add a debug_assert! on
> creation/modification to check that everything is as expected."
> IIUC, his idea is that the `debug_assert!()` condition must always be met,
> which is the guarantee that the HgPath type provides. In order to ensure
> that, we'll have to validate all paths read from dirstate, for example,
> prior to casting to HgPath type. Otherwise `debug_assert!()` would fail,
> meaning the implementation had a bug.
> So, my question is, do we really need such strong guarantee by default?
I indeed do not think we do, for the reasons I gave in my previous comment.
> Instead, we could lower the bar to "HgPath is merely a path represented in
> platform-specific encoding though it's supposed to be a canonical path",
> and add e.g. `is_canonicalized()` to check if the path meets the strong
> requirement.
Is it really represented in a platform-specific encoding or it just treated as ASCII until file system operations are needed?
I would use the strong check when converting to platform-specific types like `OsStr` or `Path`.
>> >> +impl Index<usize> for HgPath {
>> >> + type Output = u8;
>> >> +
>> >> + fn index(&self, i: usize) -> &Self::Output {
>> >> + &self.inner[i]
>> >> + }
>> >> +}
>> >
>> > [...]
>> > Better to not implement `path[...]` since it can be considered returning
>> > a path component at the specified index. We can always do
>> > `path.as_bytes()[...]` explicitly.
>> I'd leave `Index<RangeFull>`since it it not ambiguous and is a neat shorthand.
>
> What shorthand would it be used for?
> I feel it's weird only `path[..]` is allowed.
This is useful in `Deref` for example: `&self[..]`. But I can remove it if needed.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D6773/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D6773
To: Alphare, #hg-reviewers, kevincox
Cc: yuja, durin42, kevincox, mercurial-devel
More information about the Mercurial-devel
mailing list