D7528: rust-matchers: add `FileMatcher` implementation

martinvonz (Martin von Zweigbergk) phabricator at mercurial-scm.org
Tue Dec 10 16:34:22 EST 2019


martinvonz added inline comments.

INLINE COMMENTS

> matchers.rs:129-132
> +/// assert_eq!(true, matcher.matches(HgPath::new(b"a.txt")));
> +/// assert_eq!(false, matcher.matches(HgPath::new(b"b.txt")));
> +/// assert_eq!(false, matcher.matches(HgPath::new(b"main.c")));
> +/// assert_eq!(true, matcher.matches(HgPath::new(br"re:.*\.c$")));

Same comment as on another (already submitted) patch: the argument order feels backwards. Do you mind changing it?

> matchers.rs:136
> +pub struct FileMatcher<'a> {
> +    files: HashSet<&'a HgPath>,
> +    dirs: DirsMultiset,

It would be slightly simpler if the paths were owned. It doesn't seem too expensive to take ownership of the paths. Do we have any call sites in this series that we can look at?

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: martinvonz, durin42, kevincox, mercurial-devel


More information about the Mercurial-devel mailing list