[PATCH 1 of 2 STABLE] context: don't complain about a matcher's subrepo paths in changectx.walk()

[Matt Harbison]

On Thu, 04 Jun 2015 00:37:44 -0400, Martin von Zweigbergk  
<martinvonz at google.com> wrote:

> On Wed, Jun 3, 2015 at 5:52 PM Matt Harbison <mharbison72 at gmail.com>  
> wrote:
>
>> Gentle ping on this.  I didn't want to just swipe the idea and run with  
>> it.
>>
>> ------- Forwarded message -------
>> From: "Matt Harbison" <mharbison72 at gmail.com>
>> To: "Matt Mackall" <mpm at selenic.com>
>> Cc: mercurial-devel at selenic.com, matt_harbison at yahoo.com
>> Subject: Re: [PATCH 1 of 2 STABLE] context: don't complain about a
>> matcher's subrepo paths in changectx.walk()
>> Date: Tue, 19 May 2015 19:24:51 -0400
>>
>> On Tue, 19 May 2015 09:14:13 -0400, Matt Mackall <mpm at selenic.com>  
>> wrote:
>>
>> > On Mon, 2015-05-18 at 22:14 -0400, Matt Harbison wrote:
>> >> # HG changeset patch
>> >> # User Matt Harbison <matt_harbison at yahoo.com>
>> >> # Date 1431839170 14400
>> >> #      Sun May 17 01:06:10 2015 -0400
>> >> # Branch stable
>> >> # Node ID 73abecce8140e8c03b215c1b68c2669d44c62d91
>> >> # Parent  91c2278c68a387903c00a7170509c9dd343a7e55
>> >> context: don't complain about a matcher's subrepo paths in
>> >> changectx.walk()
>> >
>> > Here's an alternate solution to the problem of the "bad" callback, let
>> > me know what you think:
>>
>> I like it.  I was worried about recursive methods like cmdutil.add(),  
>> but
>> it doesn't look like any of the 20 or so matches for '\.bad =' would be
>> problematic as long as the bad matcher isn't passed to the recursive
>> call.  And it would fix some current mis-usage.
>>
>> Any reason not to make it a function on matchmod that internally calls
>> copy() and then overwrites bad?  I'm thinking avoiding long undetected  
>> and
>> subtle bugs if someone adds a field to the base class, but forgets to  
>> copy
>> it here.
>
>
> I think a copy() method makes sense.
>
>
>> I have no idea what a copy() costs though.
>>
>
> I don't think matchers are created frequently enough that it matters.  
> It's
> usually a few times per repo, right? Ever per revision? Definitely not  
> per
> file, I would think.

To my knowledge, it's generally once per command.  The exception being  
narrowing for subrepos, and some largefile -> standin translations  
(usually also once per command per repo).

I just haven't see it used much in the code I'm familiar with aside from  
largefiles, and given the manual copying in the strawman, I thought maybe  
there's a reason to avoid it.  I'll submit a series with a method and an  
internal copy().