<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Jun 3, 2015 at 5:52 PM Matt Harbison <<a href="mailto:mharbison72@gmail.com">mharbison72@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Gentle ping on this. I didn't want to just swipe the idea and run with it.<br>
<br>
------- Forwarded message -------<br>
From: "Matt Harbison" <<a href="mailto:mharbison72@gmail.com" target="_blank">mharbison72@gmail.com</a>><br>
To: "Matt Mackall" <<a href="mailto:mpm@selenic.com" target="_blank">mpm@selenic.com</a>><br>
Cc: <a href="mailto:mercurial-devel@selenic.com" target="_blank">mercurial-devel@selenic.com</a>, <a href="mailto:matt_harbison@yahoo.com" target="_blank">matt_harbison@yahoo.com</a><br>
Subject: Re: [PATCH 1 of 2 STABLE] context: don't complain about a<br>
matcher's subrepo paths in changectx.walk()<br>
Date: Tue, 19 May 2015 19:24:51 -0400<br>
<br>
On Tue, 19 May 2015 09:14:13 -0400, Matt Mackall <<a href="mailto:mpm@selenic.com" target="_blank">mpm@selenic.com</a>> wrote:<br>
<br>
> On Mon, 2015-05-18 at 22:14 -0400, Matt Harbison wrote:<br>
>> # HG changeset patch<br>
>> # User Matt Harbison <<a href="mailto:matt_harbison@yahoo.com" target="_blank">matt_harbison@yahoo.com</a>><br>
>> # Date 1431839170 14400<br>
>> # Sun May 17 01:06:10 2015 -0400<br>
>> # Branch stable<br>
>> # Node ID 73abecce8140e8c03b215c1b68c2669d44c62d91<br>
>> # Parent 91c2278c68a387903c00a7170509c9dd343a7e55<br>
>> context: don't complain about a matcher's subrepo paths in<br>
>> changectx.walk()<br>
><br>
> Here's an alternate solution to the problem of the "bad" callback, let<br>
> me know what you think:<br>
<br>
I like it. I was worried about recursive methods like cmdutil.add(), but<br>
it doesn't look like any of the 20 or so matches for '\.bad =' would be<br>
problematic as long as the bad matcher isn't passed to the recursive<br>
call. And it would fix some current mis-usage.<br>
<br>
Any reason not to make it a function on matchmod that internally calls<br>
copy() and then overwrites bad? I'm thinking avoiding long undetected and<br>
subtle bugs if someone adds a field to the base class, but forgets to copy<br>
it here. </blockquote><div><br></div><div><div>I think a copy() method makes sense.</div></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> I have no idea what a copy() costs though.<br></blockquote><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
I'll make a pass over these 20 once it shows up in the main repo in<br>
whatever form. Should we add a checkcode rule, even though some uses<br>
(like scmutil.matchandpats or lfutil.getstandinmatcher) don't need to be<br>
(and shouldn't be) replaced?<br>
<br>
<br>
> diff -r 472a685a4961 mercurial/context.py<br>
> --- a/mercurial/context.py Tue May 19 07:17:57 2015 -0500<br>
> +++ b/mercurial/context.py Tue May 19 08:11:21 2015 -0500<br>
> @@ -9,7 +9,7 @@<br>
> from i18n import _<br>
> import mdiff, error, util, scmutil, subrepo, patch, encoding, phases<br>
> import match as matchmod<br>
> -import copy, os, errno, stat<br>
> +import os, errno, stat<br>
> import obsolete as obsmod<br>
> import repoview<br>
> import fileset<br>
> @@ -591,19 +591,16 @@<br>
> def walk(self, match):<br>
> '''Generates matching file names.'''<br>
> - # Override match.bad method to have message with nodeid<br>
> - match = copy.copy(match)<br>
> - oldbad = match.bad<br>
> def bad(fn, msg):<br>
> # The manifest doesn't know about subrepos, so don't<br>
> complain about<br>
> # paths into valid subrepos.<br>
> if any(fn == s or fn.startswith(s + '/')<br>
> for s in self.substate):<br>
> return<br>
> - oldbad(fn, _('no such file in rev %s') % self)<br>
> - match.bad = bad<br>
> + match.bad(fn, _('no such file in rev %s') % self)<br>
> + m = matchmod.badmatch(match, bad)<br>
> - return self._manifest.walk(match)<br>
> + return self._manifest.walk(m)<br>
> def matches(self, match):<br>
> return self.walk(match)<br>
> diff -r 472a685a4961 mercurial/match.py<br>
> --- a/mercurial/match.py Tue May 19 07:17:57 2015 -0500<br>
> +++ b/mercurial/match.py Tue May 19 08:11:21 2015 -0500<br>
> @@ -234,6 +234,23 @@<br>
> def always(root, cwd):<br>
> return match(root, cwd, [])<br>
> +class badmatch(match):<br>
> + '''wrap a matcher with a bad() callback'''<br>
> + def __init__(self, matcher, badfn):<br>
> + # copy base object<br>
> + self._root = matcher._root<br>
> + self._cwd = matcher._cwd<br>
> + self._matcher = matcher<br>
> + self._always = matcher._always<br>
> + self._pathrestricted = matcher._pathrestricted<br>
> + self._files = matcher._files<br>
> + self._always = matcher._always<br>
> + self._anypats = matcher._anypats<br>
> + self._fileroots = matcher._fileroots<br>
> + self.matchfn = matcher.matchfn<br>
> + # add callback<br>
> + self.bad = badfn<br>
> +<br>
> class narrowmatcher(match):<br>
> """Adapt a matcher to work on a subdirectory only.<br>
_______________________________________________<br>
Mercurial-devel mailing list<br>
<a href="mailto:Mercurial-devel@selenic.com" target="_blank">Mercurial-devel@selenic.com</a><br>
<a href="https://selenic.com/mailman/listinfo/mercurial-devel" target="_blank">https://selenic.com/mailman/listinfo/mercurial-devel</a><br>
</blockquote></div></div>