This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7255

rust-matchers: remove default implementations for `Matcher` trait
ClosedPublic

Authored by Alphare on Nov 6 2019, 10:24 AM.

Details

Reviewers
kevincox
Group Reviewers
hg-reviewers
Commits
rHG27c25c0dc967: rust-matchers: remove default implementations for `Matcher` trait
Summary

We don't expect a whole lot of matchers to be defined, and this makes it more
obvious what a matcher does by reading its impl Matcher for FooMatcher.

This patch has the added benefit of fixing the AlwaysMatcher, its matches
function differs from the former default.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Alphare created this revision.Nov 6 2019, 10:24 AM
Alphare added a comment.EditedNov 6 2019, 10:27 AM

phabsend failed with the following:

** Unknown exception encountered with possibly-broken third-party extension perf
** which supports versions unknown of Mercurial.
** Please disable perf and try your action again.
** If that fixes the bug please report it to the extension author.
** Python 2.7.15 (default, Feb 11 2019, 16:23:45) [GCC 8.2.0]
** Mercurial Distributed SCM (version 5.2rc0+2-dcf396551305)
** Extensions loaded: evolve, topic, patchbomb, rebase, absorb, phabricator, histedit, formatsource, perf, fix
Traceback (most recent call last):
  File "/home/alphare/hg/hg", line 36, in <module>
    dispatch.run()
  File "/home/alphare/hg/mercurial/dispatch.py", line 111, in run
    status = dispatch(req)
  File "/home/alphare/hg/mercurial/dispatch.py", line 250, in dispatch
    ret = _runcatch(req) or 0
  File "/home/alphare/hg/mercurial/dispatch.py", line 424, in _runcatch
    return _callcatch(ui, _runcatchfunc)
  File "/home/alphare/hg/mercurial/dispatch.py", line 433, in _callcatch
    return scmutil.callcatch(ui, func)
  File "/home/alphare/hg/mercurial/scmutil.py", line 177, in callcatch
    return func()
  File "/home/alphare/hg/mercurial/dispatch.py", line 414, in _runcatchfunc
    return _dispatch(req)
  File "/home/alphare/hg/mercurial/dispatch.py", line 1174, in _dispatch
    lui, repo, cmd, fullargs, ui, options, d, cmdpats, cmdoptions
  File "/home/alphare/hg/mercurial/dispatch.py", line 862, in runcommand
    ret = _runcommand(ui, options, cmd, d)
  File "/home/alphare/hg/mercurial/dispatch.py", line 1185, in _runcommand
    return cmdfunc()
  File "/home/alphare/hg/mercurial/dispatch.py", line 1171, in <lambda>
    d = lambda: util.checksignature(func)(ui, *args, **strcmdopt)
  File "/home/alphare/hg/mercurial/util.py", line 1843, in check
    return func(*args, **kwargs)
  File "/home/alphare/hg/hgext/phabricator.py", line 204, in inner
    return fn(*args, **kwargs)
  File "/home/alphare/hg/hgext/phabricator.py", line 1158, in phabsend
    extra=old.extra(),
  File "/home/alphare/hg/mercurial/context.py", line 2885, in __init__
    r"can't reuse the manifest: its p1 "
RuntimeError: can't reuse the manifest: its p1 doesn't match the new ctx p1

I'll have to figure out which extension broke.

EDIT: somehow I managed to empty my changeset (whether that's a bug in my brain or in mercurial remains to be seen ;) ), and phabsend does not handle empty changesets gracefully.

Alphare updated this revision to Diff 17626.Nov 6 2019, 12:36 PM
kevincox accepted this revision.Nov 6 2019, 12:43 PM
Alphare added a commit: rHG27c25c0dc967: rust-matchers: remove default implementations for `Matcher` trait.Nov 11 2019, 4:17 PM
This revision was not accepted when it landed; it landed in state Needs Review.
Closed by commit rHG27c25c0dc967: rust-matchers: remove default implementations for `Matcher` trait (authored by Alphare). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mrust/hg-core/src/matchers.rs (35 lines)

Diff 18027

rust/hg-core/src/matchers.rs

/// Visit this directory and all subdirectories /// Visit this directory and all subdirectories
Recursive, Recursive,
} }
pub trait Matcher { pub trait Matcher {
/// Explicitly listed files /// Explicitly listed files
fn file_set(&self) -> HashSet<&HgPath>; fn file_set(&self) -> HashSet<&HgPath>;
/// Returns whether `filename` is in `file_set` /// Returns whether `filename` is in `file_set`
fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool { fn exact_match(&self, filename: impl AsRef<HgPath>) -> bool;
false
}
/// Returns whether `filename` is matched by this matcher /// Returns whether `filename` is matched by this matcher
fn matches(&self, _filename: impl AsRef<HgPath>) -> bool { fn matches(&self, filename: impl AsRef<HgPath>) -> bool;
false
}
/// Decides whether a directory should be visited based on whether it /// Decides whether a directory should be visited based on whether it
/// has potential matches in it or one of its subdirectories, and /// has potential matches in it or one of its subdirectories, and
/// potentially lists which subdirectories of that directory should be /// potentially lists which subdirectories of that directory should be
/// visited. This is based on the match's primary, included, and excluded /// visited. This is based on the match's primary, included, and excluded
/// patterns. /// patterns.
/// ///
/// # Example /// # Example
/// ///
/// a file (like the yet to be implemented in Rust `ExactMatcher` does), /// a file (like the yet to be implemented in Rust `ExactMatcher` does),
/// it may return `VisitChildrenSet::This`. /// it may return `VisitChildrenSet::This`.
/// Do not rely on the return being a `HashSet` indicating that there are /// 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 /// no files in this dir to investigate (or equivalently that if there are
/// files to investigate in 'dir' that it will always return /// files to investigate in 'dir' that it will always return
/// `VisitChildrenSet::This`). /// `VisitChildrenSet::This`).
fn visit_children_set( fn visit_children_set(
&self, &self,
_directory: impl AsRef<HgPath>, directory: impl AsRef<HgPath>,
) -> VisitChildrenSet { ) -> VisitChildrenSet;
VisitChildrenSet::This
}
/// Matcher will match everything and `files_set()` will be empty: /// Matcher will match everything and `files_set()` will be empty:
/// optimization might be possible. /// optimization might be possible.
fn matches_everything(&self) -> bool { fn matches_everything(&self) -> bool;
false
}
/// Matcher will match exactly the files in `files_set()`: optimization /// Matcher will match exactly the files in `files_set()`: optimization
/// might be possible. /// might be possible.
fn is_exact(&self) -> bool { fn is_exact(&self) -> bool;
false
}
} }
/// Matches everything. /// Matches everything.
#[derive(Debug)] #[derive(Debug)]
pub struct AlwaysMatcher; pub struct AlwaysMatcher;
impl Matcher for AlwaysMatcher { impl Matcher for AlwaysMatcher {
fn file_set(&self) -> HashSet<&HgPath> { fn file_set(&self) -> HashSet<&HgPath> {
HashSet::new() HashSet::new()
} }
fn exact_match(&self, _filename: impl AsRef<HgPath>) -> bool {
false
}
fn matches(&self, _filename: impl AsRef<HgPath>) -> bool {
true
}
fn visit_children_set( fn visit_children_set(
&self, &self,
_directory: impl AsRef<HgPath>, _directory: impl AsRef<HgPath>,
) -> VisitChildrenSet { ) -> VisitChildrenSet {
VisitChildrenSet::Recursive VisitChildrenSet::Recursive
} }
fn matches_everything(&self) -> bool {
true
}
fn is_exact(&self) -> bool {
false
}
} }