martinvonz added inline comments.

INLINE COMMENTS

> Alphare wrote in files.rs:220-222
> I could reword this sentence if you feel that it does not convey the same 
> meaning. It fits the Python implementation better, that's for sure.

To me, the current sentence very much makes it sound like it iterates until 
`name != name.parent()`, which doesn't seem correct. So, yes, please reword it. 
(Unless I'm just misunderstanding something.)

> Alphare wrote in files.rs:227-230
> For `b'/'` it would return `Some(std::Path::Components::RootDir)`. It only 
> returns `None` if there are no components.

Okay, but it seems that that doesn't address my concern. If I understand 
correctly (now that you've corrected me), we get here if `name == ""`. However, 
name is an absolute path (right?), so with `root = "/some/path/"` and `name = 
""`, why should we return `Ok()`? If `name` is indeed an absolute path, I would 
interpret "" as the root (file system root, not repo root), which is not inside 
"/some/path". What am I missing?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7871/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7871

To: Alphare, #hg-reviewers, kevincox, martinvonz
Cc: martinvonz, durin42, kevincox, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to