D7525: rust-matchers: improve `Matcher` trait ergonomics
Alphare created this revision.
Herald added subscribers: mercurial-devel, kevincox, durin42.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
`VisitChildrenSet` has no need to own the set, this will save allocations.
The `file_set` return type change is motivated by both ergonomics and... being
able to compile code.
The `AlwaysMatcher` does not store a `file_set`, which requires it to return
an
owned `HashSet`, which in turn would change our return type to `Cow<&HgPath>`
(lifetimes omitted). This is both un-ergonomic and troublesome for more
complex lifetime issues (especially with the upcoming `FileMatcher` in the
following patch).
REPOSITORY
rHG Mercurial
BRANCH
default
REVISION DETAIL
https://phab.mercurial-scm.org/D7525
AFFECTED FILES
rust/hg-core/src/matchers.rs
CHANGE DETAILS
diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
--- a/rust/hg-core/src/matchers.rs
+++ b/rust/hg-core/src/matchers.rs
@@ -10,21 +10,21 @@
use crate::utils::hg_path::{HgPath, HgPathBuf};
use std::collections::HashSet;
-pub enum VisitChildrenSet {
+pub enum VisitChildrenSet<'a> {
/// Don't visit anything
Empty,
/// Only visit this directory
This,
/// Visit this directory and these subdirectories
/// TODO Should we implement a `NonEmptyHashSet`?
-Set(HashSet),
+Set(HashSet<&'a HgPath>),
/// Visit this directory and all subdirectories
Recursive,
}
pub trait Matcher {
/// Explicitly listed files
-fn file_set(&self) -> HashSet<&HgPath>;
+fn file_set(&self) -> Option<&HashSet<&HgPath>>;
/// Returns whether `filename` is in `file_set`
fn exact_match(&self, filename: impl AsRef) -> bool;
/// Returns whether `filename` is matched by this matcher
@@ -82,8 +82,8 @@
pub struct AlwaysMatcher;
impl Matcher for AlwaysMatcher {
-fn file_set(&self) -> HashSet<&HgPath> {
-HashSet::new()
+fn file_set(&self) -> Option<&HashSet<&HgPath>> {
+None
}
fn exact_match(&self, _filename: impl AsRef) -> bool {
false
To: Alphare, #hg-reviewers
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7525: rust-matchers: improve `Matcher` trait ergonomics
kevincox added inline comments.
kevincox accepted this revision.
INLINE COMMENTS
> matchers.rs:87
> +None
> }
> fn exact_match(&self, _filename: impl AsRef) -> bool {
Another option here would just to have a global empty hash set.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7525/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7525
To: Alphare, #hg-reviewers, kevincox
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7525: rust-matchers: improve `Matcher` trait ergonomics
Alphare added inline comments. INLINE COMMENTS > kevincox wrote in matchers.rs:87 > Another option here would just to have a global empty hash set. I feel like an `Option` is more explicit and less contrived, but yes. REPOSITORY rHG Mercurial CHANGES SINCE LAST ACTION https://phab.mercurial-scm.org/D7525/new/ REVISION DETAIL https://phab.mercurial-scm.org/D7525 To: Alphare, #hg-reviewers, kevincox Cc: durin42, kevincox, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D7525: rust-matchers: improve `Matcher` trait ergonomics
Closed by commit rHG1bb4e9b02984: rust-matchers: improve `Matcher` trait
ergonomics (authored by Alphare).
This revision was automatically updated to reflect the committed changes.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D7525?vs=18401&id=18583
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7525/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7525
AFFECTED FILES
rust/hg-core/src/matchers.rs
CHANGE DETAILS
diff --git a/rust/hg-core/src/matchers.rs b/rust/hg-core/src/matchers.rs
--- a/rust/hg-core/src/matchers.rs
+++ b/rust/hg-core/src/matchers.rs
@@ -10,21 +10,21 @@
use crate::utils::hg_path::{HgPath, HgPathBuf};
use std::collections::HashSet;
-pub enum VisitChildrenSet {
+pub enum VisitChildrenSet<'a> {
/// Don't visit anything
Empty,
/// Only visit this directory
This,
/// Visit this directory and these subdirectories
/// TODO Should we implement a `NonEmptyHashSet`?
-Set(HashSet),
+Set(HashSet<&'a HgPath>),
/// Visit this directory and all subdirectories
Recursive,
}
pub trait Matcher {
/// Explicitly listed files
-fn file_set(&self) -> HashSet<&HgPath>;
+fn file_set(&self) -> Option<&HashSet<&HgPath>>;
/// Returns whether `filename` is in `file_set`
fn exact_match(&self, filename: impl AsRef) -> bool;
/// Returns whether `filename` is matched by this matcher
@@ -82,8 +82,8 @@
pub struct AlwaysMatcher;
impl Matcher for AlwaysMatcher {
-fn file_set(&self) -> HashSet<&HgPath> {
-HashSet::new()
+fn file_set(&self) -> Option<&HashSet<&HgPath>> {
+None
}
fn exact_match(&self, _filename: impl AsRef) -> bool {
false
To: Alphare, #hg-reviewers, kevincox, pulkit
Cc: durin42, kevincox, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
