This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3461

shortest: don't keep checking for longer prefix if node doesn't exist (API)
ClosedPublic

Authored by martinvonz on May 7 2018, 12:55 PM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHGda083d9fafab: shortest: don't keep checking for longer prefix if node doesn't exist (API)
Summary

If revlog.shortest() is called with an invalid nodeid, we keep
checking if longer and longer prefixes are valid. We call
revlog._partialmatch() for each prefix. That function will give us
None if the node doesn't exist (and a RevlogError if it's ambiguous),
so there's no need to keep checking.

This patch instead makes revlog.shortest() raise a LookupError is the
node does not exist, and updates the caller to handle it. Before this
patch, revlog.shortest() would return the full hexnode for nonexistent
nodeids. By the same reasoning as in 7b2955624777 (scmutil: make
shortesthexnodeidprefix() take a full binary nodeid, 2018-04-14), it's
not revlog.shortest() that should decide how to present nonexistent
nodeids, so that's now moved to the template function.

This should speed up cases where {shortest()} is applied to an invalid
nodeid, but I couldn't think of a reasonable case where that would
happen.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Revision Contents
Changeset List

PathPackages
Mmercurial/revlog.py (5 lines)
Mmercurial/scmutil.py (5 lines)
Mmercurial/templatefuncs.py (5 lines)

Diff 8486

mercurial/revlog.py

if prefix[0] == '0' or i > len(self): if prefix[0] == '0' or i > len(self):
return False return False
return True return True
except ValueError: except ValueError:
return False return False
def isvalid(prefix): def isvalid(prefix):
try: try:
if self._partialmatch(prefix) is None: node = self._partialmatch(prefix)
return False
except error.RevlogError: except error.RevlogError:
return False return False
except error.WdirUnsupported: except error.WdirUnsupported:
# single 'ff...' match # single 'ff...' match
return True return True
if node is None:
raise LookupError(node, self.indexfile, _('no node'))
return not isrev(prefix) return not isrev(prefix)
hexnode = hex(node) hexnode = hex(node)
shortest = hexnode shortest = hexnode
startlength = max(6, minlength) startlength = max(6, minlength)
length = startlength length = startlength
while True: while True:
prefix = hexnode[:length] prefix = hexnode[:length]

mercurial/scmutil.py

repo.changelog.rev(node) # make sure node isn't filtered repo.changelog.rev(node) # make sure node isn't filtered
return node return node
def shortesthexnodeidprefix(repo, node, minlength=1): def shortesthexnodeidprefix(repo, node, minlength=1):
"""Find the shortest unambiguous prefix that matches hexnode.""" """Find the shortest unambiguous prefix that matches hexnode."""
# _partialmatch() of filtered changelog could take O(len(repo)) time, # _partialmatch() of filtered changelog could take O(len(repo)) time,
# which would be unacceptably slow. so we look for hash collision in # which would be unacceptably slow. so we look for hash collision in
# unfiltered space, which means some hashes may be slightly longer. # unfiltered space, which means some hashes may be slightly longer.
try:
return repo.unfiltered().changelog.shortest(node, minlength) return repo.unfiltered().changelog.shortest(node, minlength)
except error.LookupError:
raise error.RepoLookupError()
def isrevsymbol(repo, symbol): def isrevsymbol(repo, symbol):
"""Checks if a symbol exists in the repo. """Checks if a symbol exists in the repo.
See revsymbol() for details. Raises error.LookupError if the symbol is an See revsymbol() for details. Raises error.LookupError if the symbol is an
ambiguous nodeid prefix. ambiguous nodeid prefix.
""" """
try: try:

mercurial/templatefuncs.py

try: try:
node = scmutil.resolvehexnodeidprefix(repo, hexnode) node = scmutil.resolvehexnodeidprefix(repo, hexnode)
except error.WdirUnsupported: except error.WdirUnsupported:
node = wdirid node = wdirid
except error.LookupError: except error.LookupError:
return hexnode return hexnode
if not node: if not node:
return hexnode return hexnode
try:
return scmutil.shortesthexnodeidprefix(repo, node, minlength) return scmutil.shortesthexnodeidprefix(repo, node, minlength)
except error.RepoLookupError:
return hexnode
@templatefunc('strip(text[, chars])') @templatefunc('strip(text[, chars])')
def strip(context, mapping, args): def strip(context, mapping, args):
"""Strip characters from a string. By default, """Strip characters from a string. By default,
strips all leading and trailing whitespace.""" strips all leading and trailing whitespace."""
if not (1 <= len(args) <= 2): if not (1 <= len(args) <= 2):
# i18n: "strip" is a keyword # i18n: "strip" is a keyword
raise error.ParseError(_("strip expects one or two arguments")) raise error.ParseError(_("strip expects one or two arguments"))