Navigation Menu

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

support multi-byte strings on Windows #343

Merged
merged 4 commits into from Aug 19, 2022

Conversation

mattn
Copy link
Contributor

@mattn mattn commented Jun 17, 2022

On Windows, it use double-byte character sets not UTF-8. So ReadFile return DBCS bytes. it should be converted to UTF-8.

@muesli
Copy link
Member

muesli commented Jun 17, 2022

Hey @mattn!

Nice seeing you over here! Your contribution looks great, but I'm a bit puzzled about the go fmt changes. I can't reproduce that output locally, neither with go fmt nor with goimports or goreturns. What tool did you use to format the sources?

If you don't mind, I could revert the formatting changes, so we can focus on reviewing the actual change.

Thanks again!

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

Ah, it seems that specification of go fmt changes recently. Sorry. I'll update this PR soon.

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

Hmm, at least, same result in go1.18.

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

I'll make this changes again.

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

golang/go#51082

gofmt is changed

@muesli
Copy link
Member

muesli commented Jun 18, 2022

golang/go#51082

gofmt is changed

Understood! This seems to be a pending change for 1.19, tho? Can't reproduce it with 1.18.3 for what it's worth. I think we should stick to the existing format until this gofmt change is part of a stable Go release.

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

Reverted format

@mattn
Copy link
Contributor Author

mattn commented Jun 18, 2022

I can squash this changes. Do you want?

@muesli
Copy link
Member

muesli commented Jun 18, 2022

I can squash this changes. Do you want?

Thanks, but no need to, can do that here just fine!

@meowgorithm
Copy link
Member

Small note that this will close #393.

@maaslalani maaslalani linked an issue Aug 19, 2022 that may be closed by this pull request
@meowgorithm meowgorithm merged commit 6b68505 into charmbracelet:master Aug 19, 2022
@meowgorithm
Copy link
Member

Thank you so much for the contribution, @mattn. Multi-byte support is really important to us. Cheers 🍻.

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.

cancelreader.NewReader not support chinese?
3 participants