Patchwork [V2] py3: change explicit conversion from str to pycompat.bytestr

login
register
mail settings
Submitter Rishabh Madan
Date March 17, 2017, 1:44 p.m.
Message ID <3d6d9294e22473165419.1489758266@bunty>
Download mbox | patch
Permalink /patch/19419/
State Accepted
Headers show

Comments

Rishabh Madan - March 17, 2017, 1:44 p.m.
# HG changeset patch
# User Rishabh Madan <rishabhmadan96@gmail.com>
# Date 1489758142 -19800
#      Fri Mar 17 19:12:22 2017 +0530
# Node ID 3d6d9294e2247316541942c4bec5186cb5772cd6
# Parent  a5bad127128d8f60060be53d161acfa7a32a17d5
py3: change explicit conversion from str to pycompat.bytestr
Yuya Nishihara - March 17, 2017, 2:10 p.m.
On Fri, 17 Mar 2017 19:14:26 +0530, Rishabh Madan wrote:
> # HG changeset patch
> # User Rishabh Madan <rishabhmadan96@gmail.com>
> # Date 1489758142 -19800
> #      Fri Mar 17 19:12:22 2017 +0530
> # Node ID 3d6d9294e2247316541942c4bec5186cb5772cd6
> # Parent  a5bad127128d8f60060be53d161acfa7a32a17d5
> py3: change explicit conversion from str to pycompat.bytestr

Queued this, thanks.
Yuya Nishihara - March 18, 2017, 2:01 a.m.
On Fri, 17 Mar 2017 11:42:03 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> On Fri, Mar 17, 2017 at 11:22 AM, Rishabh Madan <rishabhmadan96@gmail.com>
> wrote:
> 
> > The replace() function for py3 wasn't working when `value` was being
> > typecasted as str. When I used pycompat.bytestr the error was resolved.
> > So I guess it is preserved by such methods.
> >
> 
> Do you know how that works? Since it's not pycompat.bytestr, I don't
> understand what makes it work. So if you add something like this at the end
> of the "if fm.isplain():" block:
> 
>             if pycompat.ispy3:
>                print 'value is bytestr', isinstance(value, pycompat.bytestr)
> 
> it prints "value is bytestr: True"? Or is that actually false and it
> somehow doesn't rely on the __iter__ or __getitem__ methods to work
> correctly when fm.isplain()?

The bytestr-ness isn't important here. We just need to stringify arbitrary
objects (e.g. booleans, integers) and we can't use bytes() as it would create
an n-length zeroed bytes.
via Mercurial-devel - March 18, 2017, 5:16 a.m.
On Fri, Mar 17, 2017 at 7:01 PM, Yuya Nishihara <yuya@tcha.org> wrote:
> On Fri, 17 Mar 2017 11:42:03 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> On Fri, Mar 17, 2017 at 11:22 AM, Rishabh Madan <rishabhmadan96@gmail.com>
>> wrote:
>>
>> > The replace() function for py3 wasn't working when `value` was being
>> > typecasted as str. When I used pycompat.bytestr the error was resolved.
>> > So I guess it is preserved by such methods.
>> >
>>
>> Do you know how that works? Since it's not pycompat.bytestr, I don't
>> understand what makes it work. So if you add something like this at the end
>> of the "if fm.isplain():" block:
>>
>>             if pycompat.ispy3:
>>                print 'value is bytestr', isinstance(value, pycompat.bytestr)
>>
>> it prints "value is bytestr: True"? Or is that actually false and it
>> somehow doesn't rely on the __iter__ or __getitem__ methods to work
>> correctly when fm.isplain()?
>
> The bytestr-ness isn't important here. We just need to stringify arbitrary
> objects (e.g. booleans, integers) and we can't use bytes() as it would create
> an n-length zeroed bytes.

Aha! I had thought the whole point of pycompat.bytestr was the
overridden __getitem__ and __iter__, but it's also useful for
stringifying. So we could also use it instead of the "b'%d' % x"
pattern we've used in some places. That would more clearly express the
intent, but it is longer, so I guess that's why we haven't done it.
Thanks for explaining.

Patch

diff -r a5bad127128d -r 3d6d9294e224 mercurial/commands.py
--- a/mercurial/commands.py	Wed Mar 15 15:48:57 2017 -0700
+++ b/mercurial/commands.py	Fri Mar 17 19:12:22 2017 +0530
@@ -1813,7 +1813,7 @@ 
     matched = False
     for section, name, value in ui.walkconfig(untrusted=untrusted):
         source = ui.configsource(section, name, untrusted)
-        value = str(value)
+        value = pycompat.bytestr(value)
         if fm.isplain():
             source = source or 'none'
             value = value.replace('\n', '\\n')