This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3189

context: extract partial nodeid lookup method to scmutil
ClosedPublic

Authored by martinvonz on Apr 8 2018, 12:03 PM.

Details

Reviewers
yuja
Group Reviewers
hg-reviewers
Commits
rHG901e749ca0e1: context: extract partial nodeid lookup method to scmutil
Summary

We will add another caller soon, and there's a non-obvious reason to
use the unfiltered repo that we don't want to copy across the code
base.

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.Apr 8 2018, 12:03 PM
yuja requested changes to this revision.Apr 9 2018, 9:57 AM
yuja added a subscriber: yuja.
yuja added inline comments.
mercurial/scmutil.py
439

We'll need to verify that the returned node is NOT hidden by e.g.
calling changelog.rev(node).

It's unlikely that callers expect this function may return a hidden
node.

This revision now requires changes to proceed.Apr 9 2018, 9:57 AM
martinvonz added inline comments.Apr 10 2018, 10:00 AM
mercurial/scmutil.py
439

Makes sense. Okay if I do that in a follow-up so the rest of the stack is not blocked by this (I will be away until Friday)?

yuja added inline comments.Apr 10 2018, 11:07 AM
mercurial/scmutil.py
439

I could add it if you're happy with the following change.

node = repo.unfiltered().changelog._partialmatch(prefix)
repo.changelog.rev(node)  # make sure node isn't filtered
return node
martinvonz added inline comments.Apr 10 2018, 11:35 AM
mercurial/scmutil.py
439

LGTM. Thank you!

Closed by commit rHG901e749ca0e1: context: extract partial nodeid lookup method to scmutil (authored by martinvonz). · Explain WhyApr 10 2018, 12:03 PM
This revision was automatically updated to reflect the committed changes.
yuja added a comment.Apr 10 2018, 12:03 PM

My bad. Added if node is None: return and pushed, thanks.

Revision Contents
Changeset List

PathPackages
Mmercurial/context.py (2 lines)
Mmercurial/scmutil.py (9 lines)

Diff 7934

mercurial/context.py

# lookup bookmarks through the name interface # lookup bookmarks through the name interface
try: try:
self._node = repo.names.singlenode(repo, changeid) self._node = repo.names.singlenode(repo, changeid)
self._rev = repo.changelog.rev(self._node) self._rev = repo.changelog.rev(self._node)
return return
except KeyError: except KeyError:
pass pass
self._node = repo.unfiltered().changelog._partialmatch(changeid) self._node = scmutil.resolvepartialhexnodeid(repo, changeid)
if self._node is not None: if self._node is not None:
self._rev = repo.changelog.rev(self._node) self._rev = repo.changelog.rev(self._node)
return return
# lookup failed # lookup failed
# check if it might have come from damaged dirstate # check if it might have come from damaged dirstate
# #
# XXX we could avoid the unfiltered if we had a recognizable # XXX we could avoid the unfiltered if we had a recognizable

mercurial/scmutil.py

def formatrevnode(ui, rev, node): def formatrevnode(ui, rev, node):
"""Format given revision and node depending on the current verbosity""" """Format given revision and node depending on the current verbosity"""
if ui.debugflag: if ui.debugflag:
hexfunc = hex hexfunc = hex
else: else:
hexfunc = short hexfunc = short
return '%d:%s' % (rev, hexfunc(node)) return '%d:%s' % (rev, hexfunc(node))
def resolvepartialhexnodeid(repo, prefix):
# Uses unfiltered repo because it's faster when then prefix is ambiguous/
# This matches the "shortest" template function.
node = repo.unfiltered().changelog._partialmatch(prefix)
yujaUnsubmitted
Not Done

We'll need to verify that the returned node is NOT hidden by e.g.
calling changelog.rev(node).

It's unlikely that callers expect this function may return a hidden
node.

yuja: We'll need to verify that the returned node is NOT hidden by e.g. calling `changelog.rev(node)`.
martinvonzAuthorUnsubmitted
Not Done

Makes sense. Okay if I do that in a follow-up so the rest of the stack is not blocked by this (I will be away until Friday)?

martinvonz: Makes sense. Okay if I do that in a follow-up so the rest of the stack is not blocked by this…
yujaUnsubmitted
Not Done

I could add it if you're happy with the following change.

node = repo.unfiltered().changelog._partialmatch(prefix)
repo.changelog.rev(node)  # make sure node isn't filtered
return node
yuja: I could add it if you're happy with the following change. ``` node = repo.unfiltered().
martinvonzAuthorUnsubmitted
Not Done

LGTM. Thank you!

martinvonz: LGTM. Thank you!
if node is None:
return
repo.changelog.rev(node) # make sure node isn't filtered
return node
def isrevsymbol(repo, symbol): def isrevsymbol(repo, symbol):
try: try:
revsymbol(repo, symbol) revsymbol(repo, symbol)
return True return True
except error.RepoLookupError: except error.RepoLookupError:
return False return False
def revsymbol(repo, symbol): def revsymbol(repo, symbol):