Skip to content
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 from
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions pexpect/__init__.py
Expand Up @@ -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'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

index = self.expect([line_ending, self.delimiter])
if index == 0:
return self.before + b'\r\n'
return self.before + line_ending
else:
return self.before

Expand Down
14 changes: 14 additions & 0 deletions tests/test_readline_spawnu.py
@@ -0,0 +1,14 @@
#!/usr/bin/env python
# -*- coding: utf-8 -*-

import pexpect

ECHO = pexpect.which('echo')

"""
Test using readline() with spawnu objects. This fails with a TypeError on older
versions of pexpect because bytes are used for line endings rather than
strings.
"""
child = pexpect.spawnu(ECHO, [ "foobar"])
foobar = child.readline()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.