This is an archive of the discontinued Mercurial Phabricator instance.

Differential D482

bundle2: fixed usage of an attribute that was removed in py3k
ClosedPublic

Authored by alex.gaynor on Aug 22 2017, 9:10 PM.

Details

Reviewers
durin42
indygreg
Group Reviewers
hg-reviewers
Commits
rHG8d75b9effa07: bundle2: fixed usage of an attribute that was removed in py3k

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

alex.gaynor created this revision.Aug 22 2017, 9:10 PM
indygreg accepted this revision.Aug 22 2017, 9:39 PM
indygreg added a subscriber: indygreg.

The commit message should likely say something about how the existing use of string.letters was logically wrong, since it is locale dependent. string.ascii_letters is definitely the variable we should have been using all along.

This revision is now accepted and ready to land.Aug 22 2017, 9:39 PM
lothiraldan added a subscriber: lothiraldan.Aug 23 2017, 4:15 AM

Would it be possible to define pycompat.bytestr(string.ascii_letters) at the module level once and for all?

durin42 edited edge metadata.Aug 23 2017, 11:03 AM
In D482#7640, @lothiraldan wrote:

Would it be possible to define pycompat.bytestr(string.ascii_letters) at the module level once and for all?

I think I'd bias against this, just because it's only used in six or so places and pycompat is already getting kind of shaggy.

lothiraldan added a comment.Aug 23 2017, 11:24 AM

Good for me.

I have the impression that we tend to defined static data (like pycompat.bytestr(string.ascii_letters) or some translations) inside functions while they could be defined once at module level. Is it intentional, is it for reducing startup time?

Closed by commit rHG8d75b9effa07: bundle2: fixed usage of an attribute that was removed in py3k (authored by alex.gaynor). · Explain WhyAug 23 2017, 11:40 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/bundle2.py (4 lines)

Diff 1201

mercurial/bundle2.py

"""total number of parts added to the bundler""" """total number of parts added to the bundler"""
return len(self._parts) return len(self._parts)
# methods used to defines the bundle2 content # methods used to defines the bundle2 content
def addparam(self, name, value=None): def addparam(self, name, value=None):
"""add a stream level parameter""" """add a stream level parameter"""
if not name: if not name:
raise ValueError('empty parameter name') raise ValueError('empty parameter name')
if name[0] not in string.letters: if name[0] not in pycompat.bytestr(string.ascii_letters):
raise ValueError('non letter first character: %r' % name) raise ValueError('non letter first character: %r' % name)
self._params.append((name, value)) self._params.append((name, value))
def addpart(self, part): def addpart(self, part):
"""add a new part to the bundle2 container """add a new part to the bundle2 container
Parts contains the actual applicative payload.""" Parts contains the actual applicative payload."""
assert part.id is None assert part.id is None
ignored when unknown. Those starting with an upper case letter are ignored when unknown. Those starting with an upper case letter are
mandatory and will this function will raise a KeyError when unknown. mandatory and will this function will raise a KeyError when unknown.
Note: no option are currently supported. Any input will be either Note: no option are currently supported. Any input will be either
ignored or failing. ignored or failing.
""" """
if not name: if not name:
raise ValueError('empty parameter name') raise ValueError('empty parameter name')
if name[0] not in string.letters: if name[0] not in pycompat.bytestr(string.ascii_letters):
raise ValueError('non letter first character: %r' % name) raise ValueError('non letter first character: %r' % name)
try: try:
handler = b2streamparamsmap[name.lower()] handler = b2streamparamsmap[name.lower()]
except KeyError: except KeyError:
if name[0].islower(): if name[0].islower():
indebug(self.ui, "ignoring unknown parameter %r" % name) indebug(self.ui, "ignoring unknown parameter %r" % name)
else: else:
raise error.BundleUnknownFeatureError(params=(name,)) raise error.BundleUnknownFeatureError(params=(name,))