Patchwork D8089: py3: __repr__ needs to return str, not bytes

login
register
mail settings
Submitter phabricator
Date Feb. 6, 2020, 6:25 p.m.
Message ID <differential-rev-PHID-DREV-pnllblo6raytnx2jhsmy-req@mercurial-scm.org>
Download mbox | patch
Permalink /patch/45001/
State Superseded
Headers show

Comments

phabricator - Feb. 6, 2020, 6:25 p.m.
spectral created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D8089

AFFECTED FILES
  mercurial/bundle2.py
  mercurial/linelog.py
  mercurial/manifest.py
  mercurial/patch.py

CHANGE DETAILS




To: spectral, #hg-reviewers
Cc: mercurial-devel
phabricator - Feb. 6, 2020, 7:46 p.m.
marmoute added a comment.
marmoute accepted this revision.


  looks good.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8089/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8089

To: spectral, #hg-reviewers, marmoute
Cc: marmoute, mercurial-devel
phabricator - Feb. 6, 2020, 8:34 p.m.
mharbison72 added inline comments.

INLINE COMMENTS

> bundle2.py:1018
>      def __repr__(self):
>          cls = b"%s.%s" % (self.__class__.__module__, self.__class__.__name__)
>          return b'<%s object at %x; id: %s; type: %s; mandatory: %s>' % (

Aren't `self.__class__.__module__` and `self.__class__.__name__` str?

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8089/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8089

To: spectral, #hg-reviewers, marmoute
Cc: mharbison72, marmoute, mercurial-devel
phabricator - Feb. 6, 2020, 11:56 p.m.
spectral added inline comments.

INLINE COMMENTS

> mharbison72 wrote in bundle2.py:1018
> Aren't `self.__class__.__module__` and `self.__class__.__name__` str?

Yep, so this instance fails for other reasons as well. Sigh.  I've sent a followup (D8091 <https://phab.mercurial-scm.org/D8091>). This may cause output like <foo object at 0xabcd1234; id: b'id'; type: b'type'; mandatory: True>.

I didn't actually test most of these, obviously :P  I just did a grep for __repr__ returning bytes...

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D8089/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D8089

To: spectral, #hg-reviewers, marmoute
Cc: mharbison72, marmoute, mercurial-devel

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1090,6 +1090,7 @@ 
     def filename(self):
         return self.header.filename()
 
+    @encoding.strmethod
     def __repr__(self):
         return b'<hunk %r@%d>' % (self.filename(), self.fromline)
 
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -21,6 +21,7 @@ 
 )
 from .pycompat import getattr
 from . import (
+    encoding,
     error,
     mdiff,
     pathutil,
@@ -867,9 +868,10 @@ 
         self._loadalllazy()
         return not self._dirs or all(m._isempty() for m in self._dirs.values())
 
+    @encoding.strmethod
     def __repr__(self):
         return (
-            b'<treemanifest dir=%s, node=%s, loaded=%s, dirty=%s at 0x%x>'
+            b'<treemanifest dir=%s, node=%s, loaded=%r, dirty=%r at 0x%x>'
             % (
                 self._dir,
                 hex(self._node),
diff --git a/mercurial/linelog.py b/mercurial/linelog.py
--- a/mercurial/linelog.py
+++ b/mercurial/linelog.py
@@ -255,7 +255,7 @@ 
         )
 
     def __repr__(self):
-        return b'<linelog at %s: maxrev=%d size=%d>' % (
+        return '<linelog at %s: maxrev=%d size=%d>' % (
             hex(id(self)),
             self._maxrev,
             len(self._program),
diff --git a/mercurial/bundle2.py b/mercurial/bundle2.py
--- a/mercurial/bundle2.py
+++ b/mercurial/bundle2.py
@@ -1013,6 +1013,7 @@ 
         self._generated = None
         self.mandatory = mandatory
 
+    @encoding.strmethod
     def __repr__(self):
         cls = b"%s.%s" % (self.__class__.__module__, self.__class__.__name__)
         return b'<%s object at %x; id: %s; type: %s; mandatory: %s>' % (