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
Lint Skipped
Unit
Unit Tests Skipped

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 (5 lines)

Diff 7878

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.
return 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!
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):