Page MenuHomePhabricator

rhg: make output of `files` relative to the current directory and the root
ClosedPublic

Authored by acezar on Aug 3 2020, 10:26 AM.

Details

Summary

This matches the behavior of hg files.
The util is added in hg-core instead of rhg because this operation could
be useful for other external tools. (this was definitely not prompted by rust
issue #50784, I swear)

Diff Detail

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

Event Timeline

acezar created this revision.Aug 3 2020, 10:26 AM
Alphare accepted this revision.Aug 3 2020, 11:25 AM
Alphare added a subscriber: Alphare.

To reviewers: this was my changeset, I'm not sure how authorship is attributed but I... accept the revision of course.

acezar updated this revision to Diff 22255.Aug 4 2020, 4:59 AM
indygreg added inline comments.
rust/hg-core/src/utils/files.rs
274

It feels weird to me that this returns [u8] instead of an HgPath[Buf]. What's your justification for this?

Alphare added inline comments.Aug 12 2020, 6:01 AM
rust/hg-core/src/utils/files.rs
274

An HgPath[Buf] is meant to be the canonical internal representation of a Mercurial path. There are a few places already where this "rule" is violated, but I am planning on refactoring them, and I can start by not adding a new instance of that issue.

acezar updated this revision to Diff 22403.Aug 14 2020, 5:26 AM

@indygreg Does my justification seem reasonable? Could we get both patches accepted?

marmoute accepted this revision.Fri, Sep 11, 4:42 AM
marmoute added a subscriber: marmoute.

This looks fine.

rust/hg-core/src/operations/list_tracked_files.rs
71

Doesn't all operation that need a repository needs to find the repository root just to be able to open it anyway ? or am I missing something?

marmoute added inline comments.Fri, Sep 11, 4:47 AM
rust/hg-core/src/utils/files.rs
274

By canonical, you mean "relative to the repository root" right ? And that is why a "relative to current working copy" path cannot fit in that. That seems right.

Using Vec<u8> seems fine since this is a intended for display only.

pulkit accepted this revision.Fri, Sep 11, 4:58 AM
This revision is now accepted and ready to land.Fri, Sep 11, 4:58 AM