D458: revset: correct ordering issues caused by `return subset.filter(...)`
quark (Jun Wu)
phabricator at mercurial-scm.org
Sun Aug 20 22:00:55 UTC 2017
quark created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION SUMMARY
A lot of revset functions use `return subset.filter(...)` which enforces
`followorder`. This patch changes them to use `intersect` so order is
handled correctly.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D458
AFFECTED FILES
mercurial/revset.py
tests/test-revset-ordering.t
CHANGE DETAILS
diff --git a/tests/test-revset-ordering.t b/tests/test-revset-ordering.t
--- a/tests/test-revset-ordering.t
+++ b/tests/test-revset-ordering.t
@@ -49,54 +49,39 @@
ordering: defineorder not respected
$ check 'adds("*")'
- ordering: defineorder not respected
$ check 'all()'
$ check 'ancestors(E)'
$ check 'author(test)'
- ordering: defineorder not respected
$ check 'bisect(untested)'
$ check 'bookmark()'
$ check 'branch(default)'
- ordering: defineorder not respected
$ check 'branchpoint()'
- ordering: defineorder not respected
$ check 'children(A)'
$ check 'closed()'
- ordering: defineorder not respected
$ check 'contains(A)'
- ordering: defineorder not respected
$ check 'date("0 0")'
- ordering: defineorder not respected
$ check 'desc("")'
- ordering: defineorder not respected
$ check 'descendants(A)'
$ check 'destination()'
- ordering: defineorder not respected
$ check 'draft()'
$ check 'file("*")'
- ordering: defineorder not respected
$ check 'filelog("A")'
$ check 'follow()'
$ check 'grep("A")'
- ordering: defineorder not respected
$ check 'head()'
$ check 'heads(E+C)'
ordering: followorder not respected
$ check 'merge()'
- ordering: defineorder not respected
$ check 'modifies("A")'
- ordering: defineorder not respected
$ check 'named("tags")'
$ check 'origin()'
$ check 'outgoing()'
$ check 'p1(B+E+D)'
$ check 'p2(B+E+D)'
$ check 'parents(E)'
$ check 'public()'
- ordering: defineorder not respected
$ check 'reverse(B+E+D)'
$ check 'removes("A")'
- ordering: defineorder not respected
$ check 'roots(B+E+D)'
$ check 'secret()'
$ check 'sort(all())'
diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -57,7 +57,7 @@
raise error.ParseError(_("missing argument"))
return methods[x[0]](repo, subset, *x[1:], order=order)
-def intersect(subset, newset, order):
+def intersect(subset, newset, order, **kwargs):
"""intersect two revsets with order preserved as requested
If order is "followorder", preserve subset's order. If order is
@@ -69,7 +69,17 @@
def myrevset(repo, subset, x, order):
newset = ... # a smartset that implements "myrevset"
return revset.intersect(subset, newset, order)
+
+ If newset is a lambda, it will be applied to subset. kwargs will be passed
+ to "subset.filter". If order is "defineorder", the result will be sorted.
"""
+ # similar to subset.filter(....), but takes order into consideration
+ if util.safehasattr(newset, '__call__'):
+ s = subset.filter(newset, **kwargs)
+ if order == defineorder:
+ s.sort()
+ return s
+
# some old code does not use smartset, normalize to smartset
if not util.safehasattr(newset, 'isascending'):
newset = subset & newset
@@ -270,16 +280,16 @@
return baseset([destutil.destmerge(repo, sourceset=sourceset)])
@predicate('adds(pattern)', safe=True)
-def adds(repo, subset, x):
+def adds(repo, subset, x, order):
"""Changesets that add a file matching pattern.
The pattern without explicit kind like ``glob:`` is expected to be
relative to the current directory and match against a file or a
directory.
"""
# i18n: "adds" is a keyword
pat = getstring(x, _("adds requires a pattern"))
- return checkstatus(repo, subset, pat, 1)
+ return checkstatus(repo, subset, pat, 1, order)
@predicate('ancestor(*changeset)', safe=True)
def ancestor(repo, subset, x):
@@ -387,14 +397,14 @@
return subset & ps
@predicate('author(string)', safe=True)
-def author(repo, subset, x):
+def author(repo, subset, x, order):
"""Alias for ``user(string)``.
"""
# i18n: "author" is a keyword
n = getstring(x, _("author requires a string"))
kind, pattern, matcher = _substringmatcher(n, casesensitive=False)
- return subset.filter(lambda x: matcher(repo[x].user()),
- condrepr=('<user %r>', n))
+ return intersect(subset, lambda x: matcher(repo[x].user()), order,
+ condrepr=('<user %r>', n))
@predicate('bisect(string)', safe=True)
def bisect(repo, x):
@@ -455,7 +465,7 @@
return bms
@predicate('branch(string or set)', safe=True)
-def branch(repo, subset, x):
+def branch(repo, subset, x, order):
"""
All changesets belonging to the given branch or the branches of the given
changesets.
@@ -481,22 +491,22 @@
# note: falls through to the revspec case if no branch with
# this name exists and pattern kind is not specified explicitly
if pattern in repo.branchmap():
- return subset.filter(lambda r: matcher(getbranch(r)),
- condrepr=('<branch %r>', b))
+ return intersect(subset, lambda r: matcher(getbranch(r)),
+ order, condrepr=('<branch %r>', b))
if b.startswith('literal:'):
raise error.RepoLookupError(_("branch '%s' does not exist")
% pattern)
else:
- return subset.filter(lambda r: matcher(getbranch(r)),
- condrepr=('<branch %r>', b))
+ return intersect(subset, lambda r: matcher(getbranch(r)), order,
+ condrepr=('<branch %r>', b))
s = getset(repo, fullreposet(repo), x)
b = set()
for r in s:
b.add(getbranch(r))
c = s.__contains__
- return subset.filter(lambda r: c(r) or getbranch(r) in b,
- condrepr=lambda: '<branch %r>' % sorted(b))
+ return intersect(subset, lambda r: c(r) or getbranch(r) in b, order,
+ condrepr=lambda: '<branch %r>' % sorted(b))
@predicate('bumped()', safe=True)
def bumped(repo, subset, x):
@@ -530,7 +540,7 @@
raise error.Abort(_("no bundle provided - specify with -R"))
return bundlerevs
-def checkstatus(repo, subset, pat, field):
+def checkstatus(repo, subset, pat, field, order):
hasset = matchmod.patkind(pat) == 'set'
mcache = [None]
@@ -560,7 +570,8 @@
if m(f):
return True
- return subset.filter(matches, condrepr=('<status[%r] %r>', field, pat))
+ return intersect(subset, matches, order,
+ condrepr=('<status[%r] %r>', field, pat))
def _children(repo, subset, parentset):
if not parentset:
@@ -588,16 +599,16 @@
return intersect(subset, cs, order)
@predicate('closed()', safe=True)
-def closed(repo, subset, x):
+def closed(repo, subset, x, order):
"""Changeset is closed.
"""
# i18n: "closed" is a keyword
getargs(x, 0, 0, _("closed takes no arguments"))
- return subset.filter(lambda r: repo[r].closesbranch(),
- condrepr='<branch closed>')
+ return intersect(subset, lambda r: repo[r].closesbranch(), order,
+ condrepr='<branch closed>')
@predicate('contains(pattern)')
-def contains(repo, subset, x):
+def contains(repo, subset, x, order):
"""The revision's manifest contains a file matching pattern (but might not
modify it). See :hg:`help patterns` for information about file patterns.
@@ -621,10 +632,10 @@
return True
return False
- return subset.filter(matches, condrepr=('<contains %r>', pat))
+ return intersect(subset, matches, order, condrepr=('<contains %r>', pat))
@predicate('converted([id])', safe=True)
-def converted(repo, subset, x):
+def converted(repo, subset, x, order):
"""Changesets converted from the given identifier in the old repository if
present, or all converted changesets if no identifier is specified.
"""
@@ -643,21 +654,21 @@
source = repo[r].extra().get('convert_revision', None)
return source is not None and (rev is None or source.startswith(rev))
- return subset.filter(lambda r: _matchvalue(r),
- condrepr=('<converted %r>', rev))
+ return intersect(subset, lambda r: _matchvalue(r), order,
+ condrepr=('<converted %r>', rev))
@predicate('date(interval)', safe=True)
-def date(repo, subset, x):
+def date(repo, subset, x, order):
"""Changesets within the interval, see :hg:`help dates`.
"""
# i18n: "date" is a keyword
ds = getstring(x, _("date requires a string"))
dm = util.matchdate(ds)
- return subset.filter(lambda x: dm(repo[x].date()[0]),
- condrepr=('<date %r>', ds))
+ return intersect(subset, lambda x: dm(repo[x].date()[0]), order,
+ condrepr=('<date %r>', ds))
@predicate('desc(string)', safe=True)
-def desc(repo, subset, x):
+def desc(repo, subset, x, order):
"""Search commit message for string. The match is case-insensitive.
Pattern matching is supported for `string`. See
@@ -668,8 +679,8 @@
kind, pattern, matcher = _substringmatcher(ds, casesensitive=False)
- return subset.filter(lambda r: matcher(repo[r].description()),
- condrepr=('<desc %r>', ds))
+ return intersect(subset, lambda r: matcher(repo[r].description()), order,
+ condrepr=('<desc %r>', ds))
def _descendants(repo, x, followfirst=False, startdepth=None, stopdepth=None):
roots = getset(repo, fullreposet(repo), x)
@@ -714,7 +725,7 @@
return _descendants(repo, x, followfirst=True)
@predicate('destination([set])', safe=True)
-def destination(repo, subset, x):
+def destination(repo, subset, x, order):
"""Changesets that were created by a graft, transplant or rebase operation,
with the given revisions specified as the source. Omitting the optional set
is the same as passing all().
@@ -755,8 +766,8 @@
r = src
src = _getrevsource(repo, r)
- return subset.filter(dests.__contains__,
- condrepr=lambda: '<destination %r>' % sorted(dests))
+ return intersect(subset, dests.__contains__, order,
+ condrepr=lambda: '<destination %r>' % sorted(dests))
@predicate('divergent()', safe=True)
def divergent(repo, subset, x):
@@ -787,7 +798,7 @@
return extincts
@predicate('extra(label, [value])', safe=True)
-def extra(repo, subset, x):
+def extra(repo, subset, x, order):
"""Changesets with the given label in the extra metadata, with the given
optional value.
@@ -813,8 +824,8 @@
extra = repo[r].extra()
return label in extra and (value is None or matcher(extra[label]))
- return subset.filter(lambda r: _matchvalue(r),
- condrepr=('<extra[%r] %r>', label, value))
+ return intersect(subset, lambda r: _matchvalue(r), order,
+ condrepr=('<extra[%r] %r>', label, value))
@predicate('filelog(pattern)', safe=True)
def filelog(repo, x):
@@ -1017,7 +1028,7 @@
return spanset(repo) # drop "null" if any
@predicate('grep(regex)')
-def grep(repo, subset, x):
+def grep(repo, subset, x, order):
"""Like ``keyword(string)`` but accepts a regex. Use ``grep(r'...')``
to ensure special escape characters are handled correctly. Unlike
``keyword(string)``, the match is case-sensitive.
@@ -1035,10 +1046,10 @@
return True
return False
- return subset.filter(matches, condrepr=('<grep %r>', gr.pattern))
+ return intersect(subset, matches, order, condrepr=('<grep %r>', gr.pattern))
@predicate('_matchfiles', safe=True)
-def _matchfiles(repo, subset, x):
+def _matchfiles(repo, subset, x, order):
# _matchfiles takes a revset list of prefixed arguments:
#
# [p:foo, i:bar, x:baz]
@@ -1096,13 +1107,13 @@
return True
return False
- return subset.filter(matches,
- condrepr=('<matchfiles patterns=%r, include=%r '
- 'exclude=%r, default=%r, rev=%r>',
- pats, inc, exc, default, rev))
+ return intersect(subset, matches, order,
+ condrepr=('<matchfiles patterns=%r, include=%r '
+ 'exclude=%r, default=%r, rev=%r>',
+ pats, inc, exc, default, rev))
@predicate('file(pattern)', safe=True)
-def hasfile(repo, subset, x):
+def hasfile(repo, subset, x, order):
"""Changesets affecting files matched by pattern.
For a faster but less accurate result, consider using ``filelog()``
@@ -1112,7 +1123,7 @@
"""
# i18n: "file" is a keyword
pat = getstring(x, _("file requires a pattern"))
- return _matchfiles(repo, subset, ('string', 'p:' + pat))
+ return _matchfiles(repo, subset, ('string', 'p:' + pat), order)
@predicate('head()', safe=True)
def head(repo, x):
@@ -1144,7 +1155,7 @@
return hiddenrevs
@predicate('keyword(string)', safe=True)
-def keyword(repo, subset, x):
+def keyword(repo, subset, x, order):
"""Search commit message, user name, and names of changed files for
string. The match is case-insensitive.
@@ -1159,7 +1170,7 @@
return any(kw in encoding.lower(t)
for t in c.files() + [c.user(), c.description()])
- return subset.filter(matches, condrepr=('<keyword %r>', kw))
+ return intersect(subset, matches, order, condrepr=('<keyword %r>', kw))
@predicate('limit(set[, n[, offset]])', safe=True, takeorder=True)
def limit(repo, subset, x, order):
@@ -1215,17 +1226,17 @@
return baseset(datarepr=('<max %r, %r>', subset, os))
@predicate('merge()', safe=True)
-def merge(repo, subset, x):
+def merge(repo, subset, x, order):
"""Changeset is a merge changeset.
"""
# i18n: "merge" is a keyword
getargs(x, 0, 0, _("merge takes no arguments"))
cl = repo.changelog
- return subset.filter(lambda r: cl.parentrevs(r)[1] != -1,
- condrepr='<merge>')
+ return intersect(subset, lambda r: cl.parentrevs(r)[1] != -1, order,
+ condrepr='<merge>')
@predicate('branchpoint()', safe=True)
-def branchpoint(repo, subset, x):
+def branchpoint(repo, subset, x, order):
"""Changesets with more than one child.
"""
# i18n: "branchpoint" is a keyword
@@ -1241,8 +1252,8 @@
for p in cl.parentrevs(r):
if p >= baserev:
parentscount[p - baserev] += 1
- return subset.filter(lambda r: parentscount[r - baserev] > 1,
- condrepr='<branchpoint>')
+ return intersect(subset, lambda r: parentscount[r - baserev] > 1, order,
+ condrepr='<branchpoint>')
@predicate('min(set)', safe=True)
def minrev(repo, subset, x):
@@ -1260,16 +1271,16 @@
return baseset(datarepr=('<min %r, %r>', subset, os))
@predicate('modifies(pattern)', safe=True)
-def modifies(repo, subset, x):
+def modifies(repo, subset, x, order):
"""Changesets modifying files matched by pattern.
The pattern without explicit kind like ``glob:`` is expected to be
relative to the current directory and match against a file or a
directory.
"""
# i18n: "modifies" is a keyword
pat = getstring(x, _("modifies requires a pattern"))
- return checkstatus(repo, subset, pat, 0)
+ return checkstatus(repo, subset, pat, 0, order)
@predicate('named(namespace)')
def named(repo, x):
@@ -1570,15 +1581,15 @@
return _phase(repo, phases.draft, phases.secret)
@predicate('public()', safe=True)
-def public(repo, subset, x):
+def public(repo, subset, x, order):
"""Changeset in public phase."""
# i18n: "public" is a keyword
getargs(x, 0, 0, _("public takes no arguments"))
phase = repo._phasecache.phase
target = phases.public
condition = lambda r: phase(repo, r) == target
- return subset.filter(condition, condrepr=('<phase %r>', target),
- cache=False)
+ return intersect(subset, condition, order,
+ condrepr=('<phase %r>', target), cache=False)
@predicate('remote([id [,path]])', safe=False)
def remote(repo, subset, x):
@@ -1616,16 +1627,16 @@
return baseset()
@predicate('removes(pattern)', safe=True)
-def removes(repo, subset, x):
+def removes(repo, subset, x, order):
"""Changesets which remove files matching pattern.
The pattern without explicit kind like ``glob:`` is expected to be
relative to the current directory and match against a file or a
directory.
"""
# i18n: "removes" is a keyword
pat = getstring(x, _("removes requires a pattern"))
- return checkstatus(repo, subset, pat, 2)
+ return checkstatus(repo, subset, pat, 2, order)
@predicate('rev(number)', safe=True)
def rev(repo, x):
@@ -1644,7 +1655,7 @@
return baseset([l])
@predicate('matching(revision [, field])', safe=True)
-def matching(repo, subset, x):
+def matching(repo, subset, x, order):
"""Changesets in which a given set of fields match the set of fields in the
selected revision or set.
@@ -1753,7 +1764,8 @@
return True
return False
- return subset.filter(matches, condrepr=('<matching%r %r>', fields, revs))
+ return intersect(subset, matches, order,
+ condrepr=('<matching%r %r>', fields, revs))
@predicate('reverse(set)', safe=True, takeorder=True)
def reverse(repo, subset, x, order):
@@ -1878,7 +1890,7 @@
return baseset([c.rev() for c in ctxs])
@predicate('subrepo([pattern])')
-def subrepo(repo, subset, x):
+def subrepo(repo, subset, x, order):
"""Changesets that add, modify or remove the given subrepo. If no subrepo
pattern is named, any subrepo changes are returned.
"""
@@ -1919,7 +1931,7 @@
return False
- return subset.filter(matches, condrepr=('<subrepo %r>', pat))
+ return intersect(subset, matches, order, condrepr=('<subrepo %r>', pat))
def _mapbynodefunc(repo, s, f):
"""(repo, smartset, [node] -> [node]) -> smartset
@@ -2005,13 +2017,13 @@
@predicate('user(string)', safe=True)
-def user(repo, subset, x):
+def user(repo, subset, x, order):
"""User name contains string. The match is case-insensitive.
Pattern matching is supported for `string`. See
:hg:`help revisions.patterns`.
"""
- return author(repo, subset, x)
+ return author(repo, subset, x, order)
@predicate('wdir()', safe=True)
def wdir(repo, subset, x):
To: quark, #hg-reviewers
Cc: mercurial-devel
More information about the Mercurial-devel
mailing list