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