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

Fix getting pandoc version number #638

Merged
merged 1 commit into from Aug 7, 2017

Conversation

Projects
None yet
2 participants
@takluyver
Copy link
Member

takluyver commented Aug 3, 2017

Allowing for non-ascii home directory on Windows. We only need to decode the first line, not the whole output.

Closes gh-621

Fix getting pandoc version number
Allowing for non-ascii home directory on Windows. We only need to decode
the first line, not the whole output.

Closes gh-621

@takluyver takluyver added the bug label Aug 3, 2017

@takluyver takluyver added this to the 5.3 milestone Aug 3, 2017

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Aug 7, 2017

Why did you get rid of universal_newlines=True? Just curious.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Aug 7, 2017

Is the issue that we just need to get it to be in the defaultlocale so that we can use that information to implicitly decode the string to ascii?

@takluyver

This comment has been minimized.

Copy link
Member Author

takluyver commented Aug 7, 2017

universal_newlines in subprocess actually means 'decode the output from this process using the locale default encoding'. It's presumably like that for historical reasons, because it makes absolutely no sense now.

The issue in #621 was that pandoc produces output that's not in the locale encoding. It may be using UTF-8 in all cases, but I don't think that really matters. The version number which we're trying to get is probably always going to be made up of ascii characters.

So the change here tells subprocess not to decode the output, so we get bytes back from it. Then we take the first line, and decode that ourselves to get the version number. The bytes.splitlines() method already copes with the different newline conventions, so we don't need subprocess to handle that.

@mpacer

This comment has been minimized.

Copy link
Member

mpacer commented Aug 7, 2017

Got it!

@mpacer mpacer merged commit 2124c81 into jupyter:master Aug 7, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@takluyver takluyver deleted the takluyver:pandoc-version-unicode branch Aug 20, 2017

@mpacer mpacer added unlogged and removed unlogged labels Aug 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment