Skip to content

ttconv: Also replace carriage return with spaces. #6526

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

Merged
merged 1 commit into from
Jun 13, 2016

Conversation

QuLogic
Copy link
Member

@QuLogic QuLogic commented Jun 1, 2016

Multi-line strings will cause issues in comments since the secondary lines will not be comments. This replacement is already done for newline characters (\n), but not carriage return (\r) which is prevalent on Windows.

I went with the simpler route of just replacing with a space instead of trying to compress \r\n because that would require shifting the entire string and would be a more involved change.

I assume that changing ttconv is alright even though it's external since I can't find an upstream and there are already comments to the effect that it has already been modified.

Fixes #5862.

Multi-line strings will cause issues in comments since the secondary
lines will not be comments. This replacement is already done for newline
characters (\n), but not carriage return (\r) which is prevalent on
Windows.
@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Jun 3, 2016
@tacaswell
Copy link
Member

👍 I was hoping this had a simple fix like this.

@mdboom should probably review this before it is merged.

@jenshnielsen jenshnielsen merged commit 494c4f6 into matplotlib:master Jun 13, 2016
jenshnielsen added a commit that referenced this pull request Jun 13, 2016
ttconv: Also replace carriage return with spaces.
@jenshnielsen
Copy link
Member

Backported to 2.x as 77caea0

@QuLogic QuLogic deleted the ttconv-carriage-return branch June 13, 2016 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants