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

Proposal: add an option to prevent encoder from adding a newline on Encode #118

Closed
Jerska opened this issue Feb 15, 2022 · 3 comments · Fixed by #119
Closed

Proposal: add an option to prevent encoder from adding a newline on Encode #118

Jerska opened this issue Feb 15, 2022 · 3 comments · Fixed by #119

Comments

@Jerska
Copy link
Contributor

Jerska commented Feb 15, 2022

Hi there!

Thanks for this amazing lib that allowed me to get close to 10x perf improvements on a project for both Marshaling & Unmarshaling use-cases.

A small suggestion I'd have following my usage of library would be to add an option to prevent adding a newline at the end of Encode.
For manual crafting of a JSON without newlines, this behavior requires to add a Truncate call on the byte buffer that is both easy to forget & confusing for readers.

val := "foo"

var buf bytes.Buffer
buf.WriteByte('[')
encoder := json.NewEncoder(&buf)
encoder.Encode(val)
buf.Truncate(buf.Len() - 1) // Remove useless newline
buf.WriteByte(']')

// Outputs:
// ["foo"]

// Without the trucate, outputs:
// ["foo"
// ]

https://go.dev/play/p/4Jkoe_A6sk0

Your version of Encoder already has a few non-standard options (e.g. SetTrustRawMessage), so I believe this could be another one, e.g. SetNoExtraNewline.
Is this an addition you'd be open to? I'd be happy to open a PR.

@achille-roussel
Copy link
Contributor

Hello @Jerska, thanks for reaching out!

The suggestion you make sound fine to add, we add the newline to be compatible with the behavior of encoding/json, but I can imagine it could be useful to disable in some cases.

Would you have the time to submit a pull request to make the change?

@Jerska
Copy link
Contributor Author

Jerska commented Feb 16, 2022

Thank you for the answer. I'll open a PR today. :)

@Jerska
Copy link
Contributor Author

Jerska commented Feb 16, 2022

I've just opened #119 . :)

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 a pull request may close this issue.

2 participants