New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed TypeError using readline with spawnu #67

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@auntieNeo

auntieNeo commented Jun 3, 2014

There seems to be a bug in the spawn.readline() function that makes it impossible to use with spawnu objects (i.e. when using strings rather than bytes). I wrote a small patch that fixes this. I tested with Python 3.2 on cygwin. I also included a small test that can reproduce this bug.

strings.
"""
child = pexpect.spawnu(ECHO, [ "foobar"])
foobar = child.readline()

This comment has been minimized.

@takluyver

takluyver Jun 3, 2014

Member

Can you make this a proper test case - i.e. it shouldn't run on import. I'd probably put it in test_unicode as well, rather than making a new file for it.

@@ -970,9 +970,10 @@ def readline(self, size=-1):
if size == 0:
return self.string_type()
# delimiter default is EOF
index = self.expect([b'\r\n', self.delimiter])
line_ending = b'\r\n' if self.string_type == bytes else '\r\n'

This comment has been minimized.

@takluyver

takluyver Jun 3, 2014

Member

Unfortunately, this doesn't work on Python 2, because both alternatives are bytes. And we can't use u'' literals until we drop Python 3.2 support (soon, but not yet).

Actually, we already have a self.linesep variable which contains \n as the appropriate bytes/unicode value. I think we should just make self.crlf as a similar class-level variable.

This comment has been minimized.

@auntieNeo

auntieNeo Jun 3, 2014

I'll make these changes later this week and submit another pull request.

2014-06-03 10:50 GMT-06:00 Thomas Kluyver notifications@github.com:

In pexpect/init.py:

@@ -970,9 +970,10 @@ def readline(self, size=-1):
if size == 0:
return self.string_type()
# delimiter default is EOF

  •    index = self.expect([b'\r\n', self.delimiter])
    
  •    line_ending = b'\r\n' if self.string_type == bytes else '\r\n'
    

Unfortunately, this doesn't work on Python 2, because both alternatives
are bytes. And we can't use u'' literals until we drop Python 3.2 support
(soon, but not yet).

Actually, we already have a self.linesep variable which contains \n as
the appropriate bytes/unicode value. I think we should just make self.crlf
as a similar class-level variable.


Reply to this email directly or view it on GitHub
https://github.com/pexpect/pexpect/pull/67/files#r13345313.

This comment has been minimized.

@takluyver

takluyver Jun 3, 2014

Member

If you push to the same branch, this pull request should get updated, so you don't need to open a new one. :-)

@auntieNeo

This comment has been minimized.

auntieNeo commented Jun 8, 2014

I implemented the changes you suggested. I also added a test for spawn() in addition to spawnu(). I tested it on both python2.7 and python3.4, but let me know if there are any errors in the string types. I'll admit they continue to confuse me in python.

@jquast

This comment has been minimized.

Member

jquast commented Jun 9, 2014

Thank you! Added to doc/history.rst and split spawn() tests beside its neighbors in test_misc.py.

merge --squashed as commit 8aea7dd

@jquast jquast closed this Jun 9, 2014

jquast referenced this pull request Jun 9, 2014

PR #66: TypeError thrown by spawnu.readline()
Submitted by @auntieNeo, fixes exception, "TypeError:
got <type 'str'> ('\r\n') as pattern" in spawnu.readline().

Bytes b'\r\n' was concatenated to u'unicode', causing an
exception to be thrown when using readline().
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment