D7178: [RFC] rust-matchers: add `Matcher` trait and implement `AlwaysMatcher`
kevincox (Kevin Cox)
phabricator at mercurial-scm.org
Wed Oct 30 11:24:54 EDT 2019
kevincox added a comment.
I see a lot of the functions are here to give optimization hints. In order to make someone non-familiar with the code able to understand it each method should state the contracts that it is making. I am having a really hard time reconciling how the different functions interact and which methods have precedence over each other.
INLINE COMMENTS
> matchers.rs:28
> + fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool;
> + fn file_set(&self) -> Option<HashSet<&HgPath>> {
> + None
Please document.
> matchers.rs:31
> + }
> + fn roots(&self) -> Option<HashSet<&HgPath>> {
> + None
Please document.
> martinvonz wrote in matchers.rs:104
> Should this be `is_always()` per Rust naming conventions?
always isn't an adjective so I don't think that really makes sense. How about `matches_everything`.
> matchers.rs:104
> + /// optimization might be possible.
> + fn always(&self) -> bool {
> + false
It is probably worth noting that false-negatives are fine but false-positives will lead to incorrect behaviour.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7178/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7178
To: Alphare, #hg-reviewers
Cc: martinvonz, spectral, durin42, kevincox, mercurial-devel
More information about the Mercurial-devel
mailing list