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

ProcessWord support UTF-8 #17055

Merged
merged 1 commit into from Oct 21, 2015
Merged

ProcessWord support UTF-8 #17055

merged 1 commit into from Oct 21, 2015

Conversation

daehyeok
Copy link
Contributor

modified PorcessWord to working normally for UTF-8 strings.
fixed #16793

Signed-off-by: Daehyeok Mun daehyeok@gmail.com

@duglin
Copy link
Contributor

duglin commented Oct 15, 2015

nice. can we get some testcases?

@calavera
Copy link
Contributor

as @duglin says, having tests would be awesome.

@daehyeok
Copy link
Contributor Author

Sorry, I forgot to testing..... I added test case for UTF-8 strings using Korean words "안녕하세요" (it's mean hello). I write new test case based on previous test case.

@vdemeester
Copy link
Member

Oh Cool ! Unicode all the things 🙌 — with tests 😻

ありがとう
고맙습니다

ch = sw.next()

ch = sw.scanner.Next()

if ch == '\000' {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to check for \000 ?

@duglin
Copy link
Contributor

duglin commented Oct 19, 2015

thanks for the tests - very nice.
Just a few comments/questions on the code.

@daehyeok
Copy link
Contributor Author

@duglin you right. I confused \000 need for multiline .... I remove/rewrite code related with \000

@duglin
Copy link
Contributor

duglin commented Oct 21, 2015

@daehyeok please squash your commits
Other than that, LGTM

modified PorcessWord to working normally for UTF-8 strings and added test cases

Signed-off-by: Daehyeok Mun <daehyeok@gmail.com>
@daehyeok
Copy link
Contributor Author

@duglin squashed, Thanks

@vdemeester
Copy link
Member

LGTM 🐰

@vdemeester
Copy link
Member

Merging 😍
ping @thaJeztah putting impact/changelog maybe ?

vdemeester added a commit that referenced this pull request Oct 21, 2015
@vdemeester vdemeester merged commit ab22fe0 into moby:master Oct 21, 2015
@thaJeztah
Copy link
Member

Yah, we can do that

@thaJeztah thaJeztah added this to the 1.10.0 milestone Jan 15, 2016
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.

docker client renders utf-8 encoded characters incorrectly
6 participants