Patchwork D7178: [RFC] rust-matchers: add `Matcher` trait and implement `AlwaysMatcher`

login
register
mail settings
Submitter phabricator
Date Nov. 6, 2019, 1:23 a.m.
Message ID <5fca821973346ea498c2adeb8b052f57@localhost.localdomain>
Download mbox | patch
Permalink /patch/42777/
State Not Applicable
Headers show

Comments

phabricator - Nov. 6, 2019, 1:23 a.m.
Closed by commit rHGa77d4fe347a4: rust-matchers: add `Matcher` trait and implement `AlwaysMatcher` (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
This revision was not accepted when it landed; it landed in state "Needs Review".

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D7178?vs=17417&id=17581

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

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

AFFECTED FILES
  rust/hg-core/src/lib.rs
  rust/hg-core/src/matchers.rs

CHANGE DETAILS




To: Alphare, #hg-reviewers
Cc: martinvonz, spectral, durin42, kevincox, mercurial-devel
Yuya Nishihara - Nov. 6, 2019, 12:14 p.m.
> +pub trait Matcher {
> +    /// Explicitly listed files
> +    fn file_set(&self) -> HashSet<&HgPath>;

[...]
> +    /// Matcher will match everything and `files_set()` will be empty:
> +    /// optimization might be possible.
> +    fn matches_everything(&self) -> bool {
> +        false
> +    }

Maybe better to not provide the default implementations since not a few
matchers will have to reimplement them.

> +impl Matcher for AlwaysMatcher {
> +    fn file_set(&self) -> HashSet<&HgPath> {
> +        HashSet::new()
> +    }
> +
> +    fn visit_children_set(
> +        &self,
> +        _directory: impl AsRef<HgPath>,
> +    ) -> VisitChildrenSet {
> +        VisitChildrenSet::Recursive
> +    }

Need to implement `matches()` and `matches_everything()`?
phabricator - Nov. 6, 2019, 12:16 p.m.
yuja added a comment.


  > +pub trait Matcher {
  > +    /// Explicitly listed files
  > +    fn file_set(&self) -> HashSet<&HgPath>;
  
  [...]
  
  > +    /// Matcher will match everything and `files_set()` will be empty:
  > +    /// optimization might be possible.
  > +    fn matches_everything(&self) -> bool {
  > +        false
  > +    }
  
  Maybe better to not provide the default implementations since not a few
  matchers will have to reimplement them.
  
  > +impl Matcher for AlwaysMatcher {
  > +    fn file_set(&self) -> HashSet<&HgPath> {
  > +        HashSet::new()
  > +    }
  > +
  > +    fn visit_children_set(
  > +        &self,
  > +        _directory: impl AsRef<HgPath>,
  > +    ) -> VisitChildrenSet {
  > +        VisitChildrenSet::Recursive
  > +    }
  
  Need to implement `matches()` and `matches_everything()`?

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: yuja, martinvonz, spectral, durin42, kevincox, mercurial-devel

Patch

diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
new file mode 100644
--- /dev/null
+++ b/rust/hg-core/src/matchers.rs
@@ -0,0 +1,105 @@ 
+// matchers.rs
+//
+// Copyright 2019 Raphaël Gomès <rgomes@octobus.net>
+//
+// This software may be used and distributed according to the terms of the
+// GNU General Public License version 2 or any later version.
+
+//! Structs and types for matching files and directories.
+
+use crate::utils::hg_path::{HgPath, HgPathBuf};
+use std::collections::HashSet;
+
+pub enum VisitChildrenSet {
+    /// Don't visit anything
+    Empty,
+    /// Only visit this directory
+    This,
+    /// Visit this directory and these subdirectories
+    /// TODO Should we implement a `NonEmptyHashSet`?
+    Set(HashSet<HgPathBuf>),
+    /// Visit this directory and all subdirectories
+    Recursive,
+}
+
+pub trait Matcher {
+    /// Explicitly listed files
+    fn file_set(&self) -> HashSet<&HgPath>;
+    /// Returns whether `filename` is in `file_set`
+    fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool {
+        false
+    }
+    /// Returns whether `filename` is matched by this matcher
+    fn matches(&self, _filename: impl AsRef<HgPath>) -> bool {
+        false
+    }
+    /// Decides whether a directory should be visited based on whether it
+    /// has potential matches in it or one of its subdirectories, and
+    /// potentially lists which subdirectories of that directory should be
+    /// visited. This is based on the match's primary, included, and excluded
+    /// patterns.
+    ///
+    /// # Example
+    ///
+    /// Assume matchers `['path:foo/bar', 'rootfilesin:qux']`, we would
+    /// return the following values (assuming the implementation of
+    /// visit_children_set is capable of recognizing this; some implementations
+    /// are not).
+    ///
+    /// ```ignore
+    /// '' -> {'foo', 'qux'}
+    /// 'baz' -> set()
+    /// 'foo' -> {'bar'}
+    /// // Ideally this would be `Recursive`, but since the prefix nature of
+    /// // matchers is applied to the entire matcher, we have to downgrade this
+    /// // to `This` due to the (yet to be implemented in Rust) non-prefix
+    /// // `RootFilesIn'-kind matcher being mixed in.
+    /// 'foo/bar' -> 'this'
+    /// 'qux' -> 'this'
+    /// ```
+    /// # Important
+    ///
+    /// Most matchers do not know if they're representing files or
+    /// directories. They see `['path:dir/f']` and don't know whether `f` is a
+    /// file or a directory, so `visit_children_set('dir')` for most matchers
+    /// will return `HashSet{ HgPath { "f" } }`, but if the matcher knows it's
+    /// a file (like the yet to be implemented in Rust `ExactMatcher` does),
+    /// it may return `VisitChildrenSet::This`.
+    /// Do not rely on the return being a `HashSet` indicating that there are
+    /// no files in this dir to investigate (or equivalently that if there are
+    /// files to investigate in 'dir' that it will always return
+    /// `VisitChildrenSet::This`).
+    fn visit_children_set(
+        &self,
+        _directory: impl AsRef<HgPath>,
+    ) -> VisitChildrenSet {
+        VisitChildrenSet::This
+    }
+    /// Matcher will match everything and `files_set()` will be empty:
+    /// optimization might be possible.
+    fn matches_everything(&self) -> bool {
+        false
+    }
+    /// Matcher will match exactly the files in `files_set()`: optimization
+    /// might be possible.
+    fn is_exact(&self) -> bool {
+        false
+    }
+}
+
+/// Matches everything.
+#[derive(Debug)]
+pub struct AlwaysMatcher;
+
+impl Matcher for AlwaysMatcher {
+    fn file_set(&self) -> HashSet<&HgPath> {
+        HashSet::new()
+    }
+
+    fn visit_children_set(
+        &self,
+        _directory: impl AsRef<HgPath>,
+    ) -> VisitChildrenSet {
+        VisitChildrenSet::Recursive
+    }
+}
diff --git a/rust/hg-core/src/lib.rs b/rust/hg-core/src/lib.rs
--- a/rust/hg-core/src/lib.rs
+++ b/rust/hg-core/src/lib.rs
@@ -17,6 +17,7 @@ 
     StateMap, StateMapIter,
 };
 mod filepatterns;
+pub mod matchers;
 pub mod utils;
 
 use crate::utils::hg_path::HgPathBuf;