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

Enable to pass an object to createBlob #464

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dudaerich
Copy link

createBlob automatically encodes string parameter
by Utf8 library. In some cases the string value
can be already encoded in Utf8 so the encoding
should be skipped.

This commit enables to pass object containing
the content and encoding items. This object is
directly sent to Github. Users have better control
about what is actually sending.

closes #463

@j-rewerts
Copy link
Member

This definitely seems like an important issue. @dudaerich Would you be able to write a test that demonstrates the issue you've run into?

@dudaerich
Copy link
Author

@j-rewerts sure. You can find it in this commit.

@j-rewerts
Copy link
Member

Could you put that on your issue-463 branch?

createBlob automatically encodes string parameter
by Utf8 library. In some cases the string value
can be already encoded in Utf8 so the encoding
should be skipped.

This commit enables to pass object containing
the content and encoding items. This object is
directly sent to Github. Users have better control
about what is actually sending.

closes github-tools#463
@dudaerich
Copy link
Author

Done. I updated the test to use the object type. Otherwise it would still fail.

To be honest, I am not very convinced that my fix is right. Its advantage is that is backward compatible. On the other hand it is quite unexpected that this test fails. IMO the library shouldn't encode strings to UTF-8. On most platforms UTF-8 is default encoding so it doesn't make sense to encode already encoded strings. If someone uses more exotic encoding, they can encode it to UTF-8 before they pass it to the library. Wdyt?

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.

createBlob automatically encodes string content to UTF8
2 participants