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

Matt Harbison mharbison72 at gmail.com
Thu Jun 4 00:52:14 UTC 2015


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 have no idea what a copy() costs though.

I'll make a pass over these 20 once it shows up in the main repo in
whatever form.  Should we add a checkcode rule, even though some uses
(like scmutil.matchandpats or lfutil.getstandinmatcher) don't need to be
(and shouldn't be) replaced?


> diff -r 472a685a4961 mercurial/context.py
> --- a/mercurial/context.py	Tue May 19 07:17:57 2015 -0500
> +++ b/mercurial/context.py	Tue May 19 08:11:21 2015 -0500
> @@ -9,7 +9,7 @@
>  from i18n import _
>  import mdiff, error, util, scmutil, subrepo, patch, encoding, phases
>  import match as matchmod
> -import copy, os, errno, stat
> +import os, errno, stat
>  import obsolete as obsmod
>  import repoview
>  import fileset
> @@ -591,19 +591,16 @@
>      def walk(self, match):
>          '''Generates matching file names.'''
> -        # Override match.bad method to have message with nodeid
> -        match = copy.copy(match)
> -        oldbad = match.bad
>          def bad(fn, msg):
>              # The manifest doesn't know about subrepos, so don't  
> complain about
>              # paths into valid subrepos.
>              if any(fn == s or fn.startswith(s + '/')
>                     for s in self.substate):
>                  return
> -            oldbad(fn, _('no such file in rev %s') % self)
> -        match.bad = bad
> +            match.bad(fn, _('no such file in rev %s') % self)
> +        m = matchmod.badmatch(match, bad)
> -        return self._manifest.walk(match)
> +        return self._manifest.walk(m)
>     def matches(self, match):
>          return self.walk(match)
> diff -r 472a685a4961 mercurial/match.py
> --- a/mercurial/match.py	Tue May 19 07:17:57 2015 -0500
> +++ b/mercurial/match.py	Tue May 19 08:11:21 2015 -0500
> @@ -234,6 +234,23 @@
>  def always(root, cwd):
>      return match(root, cwd, [])
> +class badmatch(match):
> +    '''wrap a matcher with a bad() callback'''
> +    def __init__(self, matcher, badfn):
> +        # copy base object
> +        self._root = matcher._root
> +        self._cwd = matcher._cwd
> +        self._matcher = matcher
> +        self._always = matcher._always
> +        self._pathrestricted = matcher._pathrestricted
> +        self._files = matcher._files
> +        self._always = matcher._always
> +        self._anypats = matcher._anypats
> +        self._fileroots = matcher._fileroots
> +        self.matchfn = matcher.matchfn
> +        # add callback
> +        self.bad = badfn
> +
>  class narrowmatcher(match):
>      """Adapt a matcher to work on a subdirectory only.


More information about the Mercurial-devel mailing list