Unicode win process #316

Merged
merged 2 commits into from Apr 7, 2011

Projects

None yet

3 participants

@takluyver
Member

This apparently fixes a bug with non-ascii characters on Windows for Jörgen. I don't know if it's going to work in all cases (it uses the encoding of the kernel's stdin - will that always match the encoding of the stdout of a subprocess?), but it does solve the problem at least in his configuration, and I can't easily test it. The change shouldn't affect kernels running on anything other than Windows.

@minrk minrk commented on the diff Mar 28, 2011
IPython/utils/_process_win32.py
@@ -88,9 +88,12 @@ def _find_cmd(cmd):
def _system_body(p):
"""Callback for _system."""
+ enc = sys.stdin.encoding
@minrk
minrk Mar 28, 2011 Member

Is it possible for sys.stdin.encoding to be None? If so, maybe this should be:
enc = sys.stdin.encoding or sys.defaultencoding()

Also probably add 'replace' as the second argument to decode, so it won't fail even if we can't figure out the encoding.

@minrk
Member
minrk commented Mar 28, 2011

The OutStream object should probably handle unencodeable bytestrings without crashing. Other streams just print garbage when they don't understand the input, so we can probably just use the 'replace' unicode error handler.

Something like:
data = self._buffer.getvalue()
if data:
# handle unencoded streams:
if not isinstance(data, unicode):
data = data.decode(sys.stdin.encoding or sys.defaultencoding(), 'replace')
content = {u'name':self.name, u'data':data}

That way, even if it can't figure out the input, it will not raise a UnicodeError.

@takluyver
Member

Good points - thanks, Min.

@minrk
Member
minrk commented Mar 29, 2011

I think this looks fine now.

@fperez
Member
fperez commented Apr 3, 2011

@jstenar, what's your take on this? Since none of us seems to have the combination of OS/internationalization necessary to reproduce the problem, your input would be great to have on this one...

Thanks!

@fperez
Member
fperez commented Apr 7, 2011

Since we haven't heard from Jorgen, I'm inclined to merge it then: it doesn't make anything worse, and may help. We can always keep improving later if he reports lingering problems...

Go for it!

@fperez
Member
fperez commented Apr 7, 2011

ps - remember to do the merges with --no-ff if there's 3 or more commits, so we can see them together more cleanly.

@takluyver takluyver merged commit 3d869ab into ipython:master Apr 7, 2011
@icook icook added a commit to icook/ipython that referenced this pull request Aug 26, 2014
@icook icook A basic whack at nbviewer/#316. Close, but height isn't adjusting. be784cc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment