Generally looks good. Can you fix a couple of nits? And if possible, fold the tests from D5577 into this and the next patch. We prefer including relevant test in each commit.
> -@predicate('merge()', safe=True) > +@predicate('merge(withbranch)', safe=True) `merge([withbranch])` as it is an optional parameter. > def merge(repo, subset, x): > - """Changeset is a merge changeset. > + """Changeset is a merge changeset > + > + All merge revisions are returned by default. If a "withbranch" > + pattern is provided only merges with (i.e. whose second parent > + belongs to) those branches that match the pattern will be returned. > + The simplest pattern is the name of a single branch. It is also > + possible to specify a regular expression by starting the pattern > + with "re:". This can be used to match more than one branch > + (e.g. "re:branch1|branch2"). > """ > # i18n: "merge" is a keyword > - getargs(x, 0, 0, _("merge takes no arguments")) > + args = getargsdict(x, 'merge', 'withbranch') > + withbranch = '' > + if 'withbranch' in args: > + withbranch = getstring(args['withbranch'], > + _('withbranch argument must be a string')) > + kind, branchname, branchmatcher = > stringutil.stringmatcher(withbranch) Can you merge this with the next `if withbranch:` block to reduce the number of conditionally defined variables referenced later? > cl = repo.changelog > - return subset.filter(lambda r: cl.parentrevs(r)[1] != -1, > - condrepr='<merge>') > + # create the function that will be used to filter the subset > + if withbranch: > + # matchfn is a function that returns true when a revision > + # is a merge and the second parent belongs to a branch that > + # matches the withbranch pattern (which can be a literal or a regex) Nit: these comments seem a bit verbose. It's documented in stringmatcher(). > + if kind == 'literal': > + matchfn = lambda r: (cl.parentrevs(r)[1] != -1 > + and repo[r].p2().branch() == withbranch) > + else: > + matchfn = lambda r: (cl.parentrevs(r)[1] != -1 > + and branchmatcher(repo[r].p2().branch())) If we don't have anything special about the `literal` kind, we can always use the `branchmatcher()`. `if kind == 'literal'` isn't needed. _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel