( )⚙ D299 py3: introduce a wrapper for __builtins__.{raw_,}input()

This is an archive of the discontinued Mercurial Phabricator instance.

py3: introduce a wrapper for __builtins__.{raw_,}input()
ClosedPublic

Authored by durin42 on Aug 9 2017, 10:24 AM.

Details

Summary

In order to make this work, we have to wrap the io streams in a
TextIOWrapper so that __builtins__.input() can do unicode IO on Python

  1. We can't just restore the original (unicode) sys.std* because we

might be running a cmdserver, and if we blindly restore sys.* to the
original values then we end up breaking the cmdserver. Sadly,
TextIOWrapper tries to close the underlying stream during its del,
so we have to make a sublcass to prevent that.

If you see errors like:

TypeError: a bytes-like object is required, not 'str'

On an input() or print() call on Python 3, the substitution of
sys.std* is probably the root cause.

A previous version of this change tried to put the bytesinput() method
in pycompat - it turns out we need to do some encoding handling, so we
have to be in a higher layer that's allowed to use
mercurial.encoding.encoding. As a result, this is in util for now,
with the TextIOWrapper subclass hiding in encoding.py. I'm not sure of
a better place for the time being.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Aug 9 2017, 10:24 AM
yuja added a subscriber: yuja.Aug 9 2017, 11:41 AM
yuja added inline comments.
mercurial/pycompat.py
84

Needs to specify encoding because user input may contain
non-ascii characters.

Perhaps it should be noclosetextio(s, encoding=encoding.encoding) and
encoding.strtolocal(input(...)).
Alternatively, forcing latin-1 might work, but I'm not sure.

mercurial/ui.py
1222

I think it's better to specify input and output explicitly.

bytesinput(..., self.fin, self.fout)

Setting BytesIO to sys.stdin/stdout is just plain wrong on Python 3.

durin42 marked an inline comment as done.Aug 11 2017, 3:52 PM
durin42 added inline comments.
mercurial/pycompat.py
84

Ugh, gross. I can't use encoding from pycompat, can I?

mercurial/ui.py
1222

Yeah, it's pretty clowny.

durin42 marked an inline comment as done.Aug 11 2017, 3:52 PM
durin42 updated this revision to Diff 801.
yuja added inline comments.Aug 12 2017, 6:31 AM
hgext/hgk.py
100

Perhaps hgk should use ui.fin and ui.fout.

mercurial/pycompat.py
84

Nah. So this could be util.bytesinput(fin, fout) and encoding.strio().

durin42 edited the summary of this revision. (Show Details)Aug 15 2017, 4:49 PM
durin42 retitled this revision from pycompat: introduce a wrapper for __builtins__.{raw_,}input() to py3: introduce a wrapper for __builtins__.{raw_,}input().
durin42 updated this revision to Diff 948.
durin42 marked 5 inline comments as done.Aug 15 2017, 4:50 PM
durin42 added inline comments.
mercurial/pycompat.py
84

Sigh. I'm not sure I got the encoding.py bit quite right, take a look?

yuja accepted this revision.Aug 15 2017, 11:34 PM
yuja added inline comments.
mercurial/util.py
180 ↗(On Diff #948)

s/pycompat.bytestr/encoding.strtolocal/ and queued, thanks.

This revision is now accepted and ready to land.Aug 15 2017, 11:34 PM
durin42 marked an inline comment as done.Aug 15 2017, 11:47 PM
This revision was automatically updated to reflect the committed changes.
yuja added inline comments.Aug 16 2017, 12:41 AM
mercurial/encoding.py
585 ↗(On Diff #971)

Fixed as s/encoding/_sysstr(encoding)/.