D5495: revset: add "branch" positional arguments to the merge revset

[yuja (Yuya Nishihara)]

yuja added a comment.


  Generally looks good.
  
  Can you fix a couple of nits? And if possible, fold the tests from https://phab.mercurial-scm.org/D5577
  into this and the next patch. We prefer including relevant test in each
  commit.
  
  > - at predicate('merge()', safe=True)
  >  + at 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"). """
  >   1. 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.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D5495

To: angel.ezquerra, #hg-reviewers
Cc: pulkit, yuja, mercurial-devel