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 05:30:37 EDT 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