[PATCH 7 of 7] largefiles: use the optional badfn argument when building a matcher

Martin von Zweigbergk martinvonz at google.com
Mon Jun 8 23:02:22 CDT 2015


On Mon, Jun 8, 2015 at 5:35 PM Matt Harbison <mharbison72 at gmail.com> wrote:

> On Mon, 08 Jun 2015 12:52:07 -0400, Martin von Zweigbergk
> <martinvonz at google.com> wrote:
>
> > I've pushed this to the clowncopter, thanks.
> >
> > There are still 3 cases of ".bad = " in the codebase. Did you
> > intentionally
> > not update those? Will they be dealt with in another series?
>
> Thanks for raising this.  I couldn't explain it in patches unrelated to
> these uses.
>
> It was intentional for different reasons:
>
> - largefiles/overrides.py: explained below
>
> - context.py: super._matchstatus() creates the matcher, and then the
> override patches it.  So we would have to add a badfn to this signature,
> default it to None, and no caller other than the override should ever use
> it.  Then, does the overriding method need to change for consistency with
> what it is overriding, even though it shouldn't be used by a caller?  Not
> sure if it is worth it, especially since the unpatched matcher isn't
> available anywhere.
>
> - localrepo.py: commit optionally takes in a matcher, and creates one
> locally if None is provided.  So one path could use badmatch(), the other
> could pass a bad() callback (except I didn't think to add it to
> matchmod.always()).


Or they could both use badmatch() perhaps, after the None-path has created
an always-matcher. But I don't really care that much. I think it's fine as
it is.


>   But since this method doesn't use subrepo style
> narrowing and recursion, there aren't any issues that I can see with the
> patching here.  I wasn't sure if it was worth messing with.  (This method
> also patches matcher.explicitdir() in a similar way).
>
> If the goal of this was to check-code ban this idiom, I'm not sure we can
> because matchmod.badmatch() needs to use it.
>

That wasn't my goal at least. I'm happy to just hear that you had not
simply missed them. Thanks for the usual detailed explanation!


>
>
> > On Fri, Jun 5, 2015 at 9:00 PM Matt Harbison <mharbison72 at gmail.com>
> > wrote:
> >
> >> # HG changeset patch
> >> # User Matt Harbison <matt_harbison at yahoo.com>
> >> # Date 1433559195 14400
> >> #      Fri Jun 05 22:53:15 2015 -0400
> >> # Node ID 2e18852e1324a67455c39a1a39b5a2c266c1d387
> >> # Parent  63faca7efc382bd10477106373596e81b7129576
> >> largefiles: use the optional badfn argument when building a matcher
> >>
> >> The monkey patching in cat() can't be fixed, because it still delegates
> >> to
> >> the
> >> original bad().  Overriding commands.cat() should go away in favor
> >> overriding
> >> cmdutil.cat() anyway, and that matcher can be wrapped with
> >> matchmod.badmatch().
> >>
> >> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
> >> --- a/hgext/largefiles/lfutil.py
> >> +++ b/hgext/largefiles/lfutil.py
> >> @@ -241,16 +241,18 @@
> >>  def getstandinmatcher(repo, rmatcher=None):
> >>      '''Return a match object that applies rmatcher to the standin
> >> directory'''
> >>      standindir = repo.wjoin(shortname)
> >> +
> >> +    # no warnings about missing files or directories
> >> +    badfn = lambda f, msg: None
> >> +
> >>      if rmatcher and not rmatcher.always():
> >>          pats = [os.path.join(standindir, pat) for pat in
> >> rmatcher.files()]
> >> -        match = scmutil.match(repo[None], pats)
> >> +        match = scmutil.match(repo[None], pats, badfn=badfn)
> >>          # if pats is empty, it would incorrectly always match, so clear
> >> _always
> >>          match._always = False
> >>      else:
> >>          # no patterns: relative to repo root
> >> -        match = scmutil.match(repo[None], [standindir])
> >> -    # no warnings about missing files or directories
> >> -    match.bad = lambda f, msg: None
> >> +        match = scmutil.match(repo[None], [standindir], badfn=badfn)
> >>      return match
> >>
> >>  def composestandinmatcher(repo, rmatcher):
> >> _______________________________________________
> >> Mercurial-devel mailing list
> >> Mercurial-devel at selenic.com
> >> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150609/b6ec8e59/attachment.html>


More information about the Mercurial-devel mailing list