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

Strip BOM from the event data, add an encoding configuration option on the writer for writing the BOM #458

Closed
wants to merge 3 commits into from

Conversation

dralley
Copy link
Collaborator

@dralley dralley commented Aug 16, 2022

We need to allow users to write a BOM, but encoding it in the event stream creates other issues. Instead lets delegate it to an option on the Writer.

@dralley
Copy link
Collaborator Author

dralley commented Aug 16, 2022

Somewhat depends on your feelings about #441 (comment)


/// Creates a Writer from a generic Write implementor with configured whitespace indents and a
/// specified encoding scheme.
pub fn new_with_indent_and_encoding(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it's right up at the line of perhaps justifying a builder.

Only Utf8 and Utf8WithBom are supported, as encoding_rs doesn't and
likely won't support other encodings.
StartText would be out of place once all events are expected to contain UTF-8.
Additionally the decoder implementation strips BOM bytes out of the
bytestream so there's no good way to access them.
@dralley dralley changed the title Configure an encoding configuration option on the writer for writing the BOM and remove BytesStart Strip BOM from the event data, configure an encoding configuration option on the writer for writing the BOM Aug 16, 2022
@dralley dralley changed the title Strip BOM from the event data, configure an encoding configuration option on the writer for writing the BOM Strip BOM from the event data, add an encoding configuration option on the writer for writing the BOM Aug 16, 2022
@dralley
Copy link
Collaborator Author

dralley commented Aug 16, 2022

Rethinking the approach slightly.

@dralley dralley closed this Aug 16, 2022
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.

1 participant