D5496: revset: add "samebranch" keyword argument to the merge revset

yuja (Yuya Nishihara) phabricator at mercurial-scm.org
Wed Jan 16 08:10:03 EST 2019


yuja added a comment.


  >   > `[, samebranch]` or [, samebranch=False]`.
  >   
  >   I guess that means:
  >   
  >   @predicate('merge([withbranch [, samebranch=None]])', safe=True)
  >   
  >   Right? (I realized that it is incorrect to say that samebranch's default value is False).
  
  Okay, I didn't notice that. And it's tricky to map `samebranch=False` to
  "different branch" constraint. I would read it as "I don't care whether
  the branches are the same or not."
  
  We can instead express it as `merge() - merge(samebranch=True)`.
  
  >   >   if len(matchfns) == 1:
  >   >       # fast path for common case
  >   >       return subset.filter(matchfn[0], ...)
  >   >   else:
  >   >       return subset.filter(lambda r: all(p(r) for p in matchfn), ...)
  >   
  >   Do you think this makes the code simpler?
  
  Yes. The original version was hard to find all possible call paths.
  Separate function per constraint is easier to follow.
  
  > In any case, if you think this approach is best I can do it, but perhaps it would be a little better to keep a single subset.filter call as follows:
  > 
  >      
  >   if len(matchfns) == 1:
  >       finalmatchfn = matchfns[0]
  >   else:
  >       finalmatchfn = lambda r: all(p(r) for p in matchfns)
  >   return subset.filter(finalmatchfn, condrepr='<merge>')
  
  I don't care about these differences.

REPOSITORY
  rHG Mercurial

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

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


More information about the Mercurial-devel mailing list