Skip to content

Conversation

@poya
Copy link

@poya poya commented Feb 12, 2020

Fix Swift's StringPrinterEscapingHelper incorrectly returning the
first byte of the UTF-8 representation instead of the Unicode code point
for non printable unicode characters.

https://bugs.swift.org/browse/SR-8111

Fix Swift's StringPrinterEscapingHelper incorrectly returning the
first byte of the UTF-8 representation instead of the Unicode code point
for non printable unicode characters.

https://bugs.swift.org/browse/SR-8111
@poya poya requested a review from dcci February 12, 2020 04:17
@dcci dcci requested a review from milseman February 12, 2020 04:21
@dcci
Copy link
Member

dcci commented Feb 12, 2020

@swift-ci test

Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

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

I'm not a big unicode expert but this seems reasonable to me. I cc:ed @milseman as he understands this area much better than I do and can probably provide some more constructive feedback on this patch.

@poya
Copy link
Author

poya commented Feb 12, 2020

This part of the Swift implementation seems to be a copy/paste adaptation of the default implementation in DataFormatters/StringPrinter.cpp which is displaying the code point for Unicode characters. Most likely a copy/paste error from the handling of non printable ASCII characters.

@dcci
Copy link
Member

dcci commented Feb 12, 2020

I'll wait a bit in case Michael wants to comment -- otherwise I'll merge it. Please don't hesitate to ping this PR in case I forget.

@dcci dcci merged commit 66300b7 into swiftlang:swift/master Feb 18, 2020
Copy link
Member

@dcci dcci left a comment

Choose a reason for hiding this comment

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

LGTM.

@dcci
Copy link
Member

dcci commented Feb 18, 2020

Thanks, I forgot about this one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants