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

trailing comma #10

Open
Banyc opened this issue Apr 15, 2022 · 1 comment
Open

trailing comma #10

Banyc opened this issue Apr 15, 2022 · 1 comment

Comments

@Banyc
Copy link

Banyc commented Apr 15, 2022

It is also a common case

@tmccombs
Copy link
Owner

tmccombs commented Nov 3, 2023

This would be pretty difficult to do with the current implementation.

The current implementation acts as a filter for a Read implementation and modifies the buffer in place.

For comments this works fine, since it can just replace the comments with spaces.

However for a trailing comma, it can't actually know if a comma is trailing until it encounters a "]" or "}". And that might not happen until the next block is read.

I think it could be done, by allocating a separate Vec to store results in across multiple reads. Or possibly it could use a fixed-size buffer if I also changed it to remove whitespace and comments instead of replacing with whitespace.

Either way, that adds a fair amount of complexity, and could hurt performance even if removing trailing commas isn't desired.

It might be cleaner to have a separate filter to remove trailing commas, either as part of this crate or a separate crate.

Or I suppose do what https://github.com/parcel-bundler/parcel/pull/9032/files#diff-194e737b9e37f7950c7d86d1508d15d18f158db5d9ec44084f2ece3dc6eac7d4 does and have a separate API that only works on a single String (or &[u8]) that contains the entire object, so it doesn't have to worry about handling individual blocks of data separately.

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

No branches or pull requests

2 participants