This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3929

revlog: replace descendant(b, a) by isdescendantrev(a, b) (API)
ClosedPublic

Authored by martinvonz on Jul 11 2018, 8:09 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG160da69ba1bf: revlog: replace descendant(b, a) by isdescendantrev(a, b) (API)
Summary

The "is" is to match "isancestor" and to make it clear that it doesn't
return a descendant. The "rev" is to make it clear that it's not about
nodeids (unlike e.g. isancestor()). The argument order change is just
seems more natural (and makes isancestor() less confusing).

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Jul 11 2018, 8:09 PM
lothiraldan added a subscriber: lothiraldan.Jul 12 2018, 5:28 AM

This doesn't feels simpler, would it be possible to simply rename descendant into isancestorrev without changing the order of the arguments?

Also it would be useful for extensions to have a deprecation warning at least for 1 cycle?

Closed by commit rHG160da69ba1bf: revlog: replace descendant(b, a) by isdescendantrev(a, b) (API) (authored by martinvonz). · Explain WhyJul 12 2018, 9:11 AM
This revision was automatically updated to reflect the committed changes.
yuja added a subscriber: yuja.Jul 12 2018, 9:22 AM
This doesn't feels simpler, would it be possible to simply rename `descendant` into `isancestorrev` without changing the order of the arguments?

While I've queued this without reading any comments (I hate junk mails
from Phabricator), I second the removal/deprecation of isdescendant*()
in favor of isancestor*().

martinvonz added a comment.Jul 12 2018, 11:11 AM
In D3929#61325, @yuja wrote:
This doesn't feels simpler, would it be possible to simply rename `descendant` into `isancestorrev` without changing the order of the arguments?

While I've queued this without reading any comments (I hate junk mails
from Phabricator), I second the removal/deprecation of isdescendant*()
in favor of isancestor*().

That's fine with me. Kind of ironic since someone very recently went in the opposite direction and wrote isancestor() in terms of descendant() :) I'll send a patch.

Revision Contents
Changeset List

PathPackages
Mmercurial/context.py (2 lines)
Mmercurial/revlog.py (20 lines)

Diff 9565

mercurial/context.py

(short(anc), short(self._node), short(n2))) + (short(anc), short(self._node), short(n2))) +
''.join(_(" alternatively, use --config " ''.join(_(" alternatively, use --config "
"merge.preferancestor=%s\n") % "merge.preferancestor=%s\n") %
short(n) for n in sorted(cahs) if n != anc)) short(n) for n in sorted(cahs) if n != anc))
return changectx(self._repo, anc) return changectx(self._repo, anc)
def descendant(self, other): def descendant(self, other):
"""True if other is descendant of this changeset""" """True if other is descendant of this changeset"""
return self._repo.changelog.descendant(self._rev, other._rev) return self._repo.changelog.isdescendantrev(other._rev, self._rev)
def walk(self, match): def walk(self, match):
'''Generates matching file names.''' '''Generates matching file names.'''
# Wrap match.bad method to have message with nodeid # Wrap match.bad method to have message with nodeid
def bad(fn, msg): def bad(fn, msg):
# The manifest doesn't know about subrepos, so don't complain about # The manifest doesn't know about subrepos, so don't complain about
# paths into valid subrepos. # paths into valid subrepos.

mercurial/revlog.py

if prevs: if prevs:
for pr in prevs: for pr in prevs:
if pr == p: if pr == p:
c.append(self.node(r)) c.append(self.node(r))
elif p == nullrev: elif p == nullrev:
c.append(self.node(r)) c.append(self.node(r))
return c return c
def descendant(self, start, end): def isdescendantrev(self, a, b):
"""True if revision 'end' is an descendant of revision 'start' """True if revision a is a descendant of revision b
A revision is considered as a descendant of itself. A revision is considered a descendant of itself.
The implementation of this is trivial but the use of The implementation of this is trivial but the use of
commonancestorsheads is not.""" commonancestorsheads is not."""
if start == nullrev: if b == nullrev:
return True return True
elif start == end: elif a == b:
return True return True
return start in self._commonancestorsheads(start, end) return b in self._commonancestorsheads(a, b)
def commonancestorsheads(self, a, b): def commonancestorsheads(self, a, b):
"""calculate all the heads of the common ancestors of nodes a and b""" """calculate all the heads of the common ancestors of nodes a and b"""
a, b = self.rev(a), self.rev(b) a, b = self.rev(a), self.rev(b)
ancs = self._commonancestorsheads(a, b) ancs = self._commonancestorsheads(a, b)
return pycompat.maplist(self.node, ancs) return pycompat.maplist(self.node, ancs)
def _commonancestorsheads(self, *revs): def _commonancestorsheads(self, *revs):
"""calculate all the heads of the common ancestors of revs""" """calculate all the heads of the common ancestors of revs"""
try: try:
ancs = self.index.commonancestorsheads(*revs) ancs = self.index.commonancestorsheads(*revs)
except (AttributeError, OverflowError): # C implementation failed except (AttributeError, OverflowError): # C implementation failed
ancs = ancestor.commonancestorsheads(self.parentrevs, *revs) ancs = ancestor.commonancestorsheads(self.parentrevs, *revs)
return ancs return ancs
def isancestor(self, a, b): def isancestor(self, a, b):
"""return True if node a is an ancestor of node b""" """return True if node a is an ancestor of node b
A revision is considered an ancestor of itself."""
a, b = self.rev(a), self.rev(b) a, b = self.rev(a), self.rev(b)
return self.descendant(a, b) return self.isdescendantrev(b, a)
def ancestor(self, a, b): def ancestor(self, a, b):
"""calculate the "best" common ancestor of nodes a and b""" """calculate the "best" common ancestor of nodes a and b"""
a, b = self.rev(a), self.rev(b) a, b = self.rev(a), self.rev(b)
try: try:
ancs = self.index.ancestors(a, b) ancs = self.index.ancestors(a, b)
except (AttributeError, OverflowError): except (AttributeError, OverflowError):