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

[Matt Harbison]

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()).  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.


> 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