bivas / protobuf-java-format

Provide serialization and de-serialization of different formats based on Google’s protobuf Message. Enables overriding the default (byte array) output to text based formats such as XML, JSON and HTML.
BSD 3-Clause "New" or "Revised" License
153 stars 97 forks source link

Issue #35 - printToString doesn't encode to bytes #36

Open tonicsoft opened 8 years ago

tonicsoft commented 8 years ago

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.)

scr commented 7 years ago

Please update https://github.com/bivas/protobuf-java-format/blob/master/RELEASE-NOTES.md for this fix in 1.5.

scr commented 7 years ago

Would you mind fixing the issue you pointed out in Issue #35 ?

https://github.com/bivas/protobuf-java-format/blob/master/src/main/java/com/googlecode/protobuf/format/ProtobufFormatter.java#L91

Basically,

Convert this to

return new String(out.getBytes(), defaultCharset);
tonicsoft commented 7 years ago

Sure - shall I create a new issue/PR etc or just tag onto this one?

tonicsoft commented 7 years ago

Done - just added change to this issue, found another bug too.

scr commented 7 years ago

So sorry, but recent RELEASE-NOTES change for #37 collide with yours… please resolve the conflicts and upload.

tonicsoft commented 7 years ago

Conflicts resolved. Should be ready for merging now.

onesuper commented 6 years ago

Is this PR still in progress now? Without this PR, We have to invoke print() method and convert a OutputStream to String manually

luigi-riefolo commented 5 years ago

any update on this?

tonicsoft commented 5 years ago

I am no longer working on this, apologies.

Please feel free to take it up.