This is an archive of the discontinued Mercurial Phabricator instance.

revset: make subrepo optional in revset predicate functions
AbandonedPublic

Authored by quark on Aug 20 2017, 3:27 AM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

Before this patch, a revset predicate must take a subset argument. It's
not too hard for an inexperienced developer to make mistakes:

@revsetpredicate('myrevset')
def myrevset(repo, subset, x):
    myset = calculate(repo, x)
    return myset  # INCORRECT: does not take "subset" into consideration

With order concept introduced, it's not obvious that the fix could still
cause issues:

return subset & myset  # SUBOPTIMAL: uses "subset" order blindly

The "most correct" version is:

@revsetpredicate('myrevset', takeorder=True)
def myrevset(repo, subset, x, order):
    myset = calculate(repo, x)
    return revset.intersect(subset, myset, order)

To make the API easier to use correctly, this patch makes subrepo
optional. So people can just write:

@revsetpredicate('myrevset')
def myrevset(repo, x):
    return calculate(repo, x)

which is simple, intuitive, and more importantly, handles ordering
correctly.

.. api:: revset predicate no longer has to take a `subset` argument

The ``subset`` argument is now optional for a revset predicate::

   @revsetpredicate('foo')
   def foo(repo, x):
       revs = calculate(repo, x)
       return smartset.baseset(revs)

If you do need ``subset``, it's recommended to take ``order`` too::

   @revsetpredicate('foo', takeorder=True)
   def foo(repo, subset, x, order):
       revs = calculate(repo, subset, x)
       return revset.intersect(subset, revs, order)

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

quark created this revision.Aug 20 2017, 3:27 AM
quark updated this revision to Diff 1105.Aug 20 2017, 4:43 AM
quark updated this revision to Diff 1113.Aug 20 2017, 4:13 PM
quark edited the summary of this revision. (Show Details)Aug 20 2017, 4:24 PM
quark edited the summary of this revision. (Show Details)
quark edited the summary of this revision. (Show Details)Aug 20 2017, 6:00 PM
quark updated this revision to Diff 1116.
quark abandoned this revision.Aug 25 2017, 1:10 PM