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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel