-
Notifications
You must be signed in to change notification settings - Fork 127
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
Add compact arrays (#299) and pretty inline separators #349
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it useful in practice to turn compact arrays completely on or off? It seems to me I'd want arrays with few and short elements compact, but as soon as the elements are non-primitive I'd probably want them all in their own line.
However, handling all the cases seems pretty hard as well...
I think the same question came up in the original PR. I think the best we could do is check the optionally sequence length against some threshold for finer control. While doing things based on type info would be awesome, I have no idea how this could be done in serde. |
I don't think we can do that ahead of time. Also, I don't think we should try to cover all the possible formatting of RON. A full-blown formatter inside of the serialization code just makes is it unreadable and bloated - and I don't think other serialization libraries offer as much configuration as we have right now. |
So my proposal:
|
That makes sense ... I assume this formatter would perform some light parsing of RON and take in a subset of the options |
Do you mean a superset? And yes, it will have to do its own parsing because
|
Maybe a superset of a subset? I think the options
What if this new |
Ah, yes, that makes sense!
Yes, certainly, I think it should be. I've even started such an attempt here: https://github.com/ron-rs/ron-reboot
|
That looks really cool! |
So what's the plan now? Should this PR get merged and at some point the options move to your formatter? |
Yes, I think we should merge this but not accept any further configuration in the future, because I fear with every option the code becomes harder to maintain. |
Ok, that's sounds like a good plan! |
Add compact arrays (ron-rs#299) and pretty inline separators
This PR includes the implementation for compact arrays from #299 (stale and quite tricky to rebase), and a fix for #290 to make the pretty RON prettier. Specifically, this means that:
,
when multiline is disabled:
and,
CHANGELOG.md