This is an archive of the discontinued Mercurial Phabricator instance.

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
Branch
default
Lint
No Linters Available
Unit
No Unit Test Coverage

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
129–132

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

136

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
136

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.