Page MenuHomePhabricator

rust-matchers: add `FileMatcher` implementation
ClosedPublic

Authored by Alphare on Nov 29 2019, 12:57 PM.

Details

Summary

Mercurial defines an exactmatcher, I find FileMatcher to be clearer, but
am not opposed to using the old name.
This change also switched the order of assert_eq arguments as it is clearer
that way for most people.

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

Alphare created this revision.Nov 29 2019, 12:57 PM
Alphare updated this revision to Diff 18418.Dec 2 2019, 6:49 AM
martinvonz added inline comments.
rust/hg-core/src/matchers.rs
128–131

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

135

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?

Alphare added inline comments.Dec 11 2019, 4:29 AM
rust/hg-core/src/matchers.rs
135

There is a single call site for now in D7529, it's really straightforward.

Automated tools like CI can produce matchers with a lot of files and it seems to me not worth changing existing no-allocation code to get rid of a few lifetime annotations. Can we see if it gets too unwieldy?

Alphare edited the summary of this revision. (Show Details)Dec 12 2019, 10:40 AM
Alphare updated this revision to Diff 18634.
This revision was not accepted when it landed; it landed in state Needs Review.
This revision was automatically updated to reflect the committed changes.