Issue #35 - printToString doesn't encode to bytes #36
Conversation
|
Please update https://github.com/bivas/protobuf-java-format/blob/master/RELEASE-NOTES.md for this fix in 1.5. |
|
Would you mind fixing the issue you pointed out in Issue #35 ? Basically, Convert this to |
|
Sure - shall I create a new issue/PR etc or just tag onto this one? |
|
Done - just added change to this issue, found another bug too. |
|
So sorry, but recent RELEASE-NOTES change for #37 collide with yours… please resolve the conflicts and upload. |
| protected JsonGenerator createGenerator(OutputStream output) throws IOException { | ||
| JsonGenerator generator = jsonFactory.createJsonGenerator(output, JsonEncoding.UTF8); | ||
| return createGenerator(output, Charset.forName(JsonEncoding.UTF8.getJavaName())); |
There was a problem hiding this comment.
Hmm… If you know you want UTF8, can you just use StandardCharsets.html.UTF_8?
https://docs.oracle.com/javase/8/docs/api/java/nio/charset/StandardCharsets.html#UTF_8
For any AbstractCharBasedFormatter, printToString can print directly to a String and not have to convert to bytes and back again. This avoids call to ByteArrayOutputStream.toString() at ProtobufFormatter:91 which uses JVM default encoding instead of ProtobufFormatter.defaultCharset. (I discovered this issue when trying to create JSON with a broad range of unicode characters on a machine where the default Charset did not cover the full Unicode range.
- use the same char set when converting from characters to bytes and back again. - Remove usage of deprecated jsonFactory API in order to respect charset when printing to an OutputStream using the JsonJacksonFormat. - Added test class hierarchy to match production code hierarchy so tests do not need to be duplicated for each ProtobufFormatter.
|
Conflicts resolved. Should be ready for merging now. |
|
Is this PR still in progress now? Without this PR, We have to invoke print() method and convert a OutputStream to String manually |
|
any update on this? |
|
I am no longer working on this, apologies. Please feel free to take it up. |
For any AbstractCharBasedFormatter, printToString can print directly to
a String and not have to convert to bytes and back again. This avoids
call to ByteArrayOutputStream.toString() at ProtobufFormatter:91 which
uses JVM default encoding instead of ProtobufFormatter.defaultCharset.
(I discovered this issue when trying to create JSON with a broad range
of unicode characters on a machine where the default Charset did not
cover the full Unicode range.)