This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4040

shortest: make isrev() a top-level function
ClosedPublic

Authored by martinvonz on Aug 1 2018, 5:57 PM.

Details

Reviewers
lothiraldan
pulkit
Group Reviewers
hg-reviewers
Commits
rHG531b86cc8fb3: shortest: make isrev() a top-level function
Summary

I'm going to add another caller in the next patch.

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.Aug 1 2018, 5:57 PM
lothiraldan accepted this revision.Aug 3 2018, 5:14 AM
pulkit added a subscriber: pulkit.Aug 3 2018, 5:48 PM
pulkit added inline comments.
mercurial/scmutil.py
496

This looks unrelated here.

martinvonz marked an inline comment as done.Aug 3 2018, 5:50 PM
martinvonz added inline comments.
mercurial/scmutil.py
496

Not completely unrelated. I moved it down here because it's no longer needed until here. It was used on line 478 before (and given Python scoping rules, it could have been moved down even before this patch, but that makes it harder to read IMO).

pulkit accepted this revision.Aug 3 2018, 5:52 PM
yuja added a subscriber: yuja.Aug 3 2018, 11:10 PM

+def mayberevnum(repo, prefix):
+ """Checks if the given prefix may be mistaken for a revision number"""
+ try:
+ i = int(prefix)
+ # if we are a pure int, then starting with zero will not be
+ # confused as a rev; or, obviously, if the int is larger
+ # than the value of the tip rev
+ if prefix[0:1] == b'0' or i > len(repo.changelog):

len(repo) should be better since repo.changelog over repoview isn't
instant. Another option is to pass in an unfiltered repo as before.

martinvonz marked an inline comment as done.Aug 4 2018, 12:27 AM
In D4040#63462, @yuja wrote:

+def mayberevnum(repo, prefix):
+ """Checks if the given prefix may be mistaken for a revision number"""
+ try:
+ i = int(prefix)
+ # if we are a pure int, then starting with zero will not be
+ # confused as a rev; or, obviously, if the int is larger
+ # than the value of the tip rev
+ if prefix[0:1] == b'0' or i > len(repo.changelog):

len(repo) should be better since repo.changelog over repoview isn't
instant. Another option is to pass in an unfiltered repo as before.

Good catch, I didn't mean to change that. Will fix.

martinvonz updated this revision to Diff 9888.Aug 4 2018, 1:26 AM
Closed by commit rHG531b86cc8fb3: shortest: make isrev() a top-level function (authored by martinvonz). · Explain WhyAug 4 2018, 2:44 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/scmutil.py (29 lines)

Diff 9895

mercurial/scmutil.py

if len(matches) == 1: if len(matches) == 1:
return matches[0] return matches[0]
raise raise
if node is None: if node is None:
return return
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, cache=None): def mayberevnum(repo, prefix):
"""Find the shortest unambiguous prefix that matches hexnode. """Checks if the given prefix may be mistaken for a revision number"""
If "cache" is not None, it must be a dictionary that can be used for
caching between calls to this method.
"""
# _partialmatch() of filtered changelog could take O(len(repo)) time,
# which would be unacceptably slow. so we look for hash collision in
# unfiltered space, which means some hashes may be slightly longer.
cl = repo.unfiltered().changelog
def isrev(prefix):
try: try:
i = int(prefix) i = int(prefix)
# if we are a pure int, then starting with zero will not be # if we are a pure int, then starting with zero will not be
# confused as a rev; or, obviously, if the int is larger # confused as a rev; or, obviously, if the int is larger
# than the value of the tip rev # than the value of the tip rev
if prefix[0:1] == b'0' or i > len(cl): if prefix[0:1] == b'0' or i > len(repo):
return False return False
return True return True
except ValueError: except ValueError:
return False return False
def shortesthexnodeidprefix(repo, node, minlength=1, cache=None):
"""Find the shortest unambiguous prefix that matches hexnode.
If "cache" is not None, it must be a dictionary that can be used for
caching between calls to this method.
"""
# _partialmatch() of filtered changelog could take O(len(repo)) time,
# which would be unacceptably slow. so we look for hash collision in
# unfiltered space, which means some hashes may be slightly longer.
def disambiguate(prefix): def disambiguate(prefix):
"""Disambiguate against revnums.""" """Disambiguate against revnums."""
hexnode = hex(node) hexnode = hex(node)
for length in range(len(prefix), len(hexnode) + 1): for length in range(len(prefix), len(hexnode) + 1):
prefix = hexnode[:length] prefix = hexnode[:length]
if not isrev(prefix): if not mayberevnum(repo, prefix):
return prefix return prefix
cl = repo.unfiltered().changelog
pulkitUnsubmitted
Done

This looks unrelated here.

pulkit: This looks unrelated here.
martinvonzAuthorUnsubmitted
Not Done

Not completely unrelated. I moved it down here because it's no longer needed until here. It was used on line 478 before (and given Python scoping rules, it could have been moved down even before this patch, but that makes it harder to read IMO).

martinvonz: Not completely unrelated. I moved it down here because it's no longer needed until here. It was…
revset = repo.ui.config('experimental', 'revisions.disambiguatewithin') revset = repo.ui.config('experimental', 'revisions.disambiguatewithin')
if revset: if revset:
revs = None revs = None
if cache is not None: if cache is not None:
revs = cache.get('disambiguationrevset') revs = cache.get('disambiguationrevset')
if revs is None: if revs is None:
revs = repo.anyrevs([revset], user=True) revs = repo.anyrevs([revset], user=True)
if cache is not None: if cache is not None: