Add explicit_struct_names Extension

[sylv256]

• I've included my change in CHANGELOG.md

Fixes #519

[sylv256]

added some questions and comments

[sylv256]

FYI Before you merge, I'm adding some documentation Finished ✨

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e27d1d5) 100.00% compared to head (4bf6ef7) 100.00%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #522   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           82        83    +1     
  Lines        14410     14443   +33     
=========================================
+ Hits         14410     14443   +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[juntyr]

Overall, the changes all look good - thank you for also adding documentation!

One question that we should still resolve is how this extension relates to the PrettyConfig::struct_names option. I think it would make sense that enabling the extension during serialisation forces all struct names to be emitted (even when we're not in pretty serialising mode). Do we then still need the PrettyConfig::struct_names option? I think if we can still emit struct names with and without (as two distinct options) emitting the explicit #[enable(explicit_struct_names)] marker, we can probably also axe the config option.

[sylv256]

I think it would be more intuitive to keep the serialization configuration options separate from the deserialization extension. If PrettyConfig is only used for serialization, then it would make sense to put all of the serialization options in one place. If we go that route, we should add a "see also" link to the documentation for PrettyConfig::struct_names and vice versa.

[sylv256]

If PrettyConfig is only used for serialization, then it would make sense to put all of the serialization options in one place. If we go that route, we should add a "see also" link to the documentation for PrettyConfig::struct_names and vice versa.

It turns out PrettyConfig does use Extensions, so I think we should just drop PrettyConfig::struct_names.

[juntyr]

I think it would be more intuitive to keep the serialization configuration options separate from the deserialization extension. If PrettyConfig is only used for serialization, then it would make sense to put all of the serialization options in one place. If we go that route, we should add a "see also" link to the documentation for PrettyConfig::struct_names and vice versa.

So far, all extensions have served a dual purpose of specifying a variation of the RON format and enabling it during both serialisation and deserialisation, so that you can produce a RON document with the same options (that’s how the Options struct originated) that you will apply during parsing. Therefore, I think it would be good for consistency to keep that dual functionality.

So far, explicit struct names have just been a prettifying option, but this PR obviously changes that. If we keep the PrettyConfig option as well, the extension would be used in non-pretty mode, but we’d need to decide how to merge the extension and the pretty config - or-ing them makes most sense.

Either way, whether we keep the PrettyConfig option or not, I agree with you that we should add a short explanation to each item.

[juntyr]

Could you also bring back the coverage to 100%? There is a test in the error file where you can just add another case. Then, in your own test, you could either try to use asserts or manually disable coverage for the unreachable panics (search for GRCOV_EXCL_ in other tests for examples).

All in all, thank you so much for working on this addition! Once these last minor nits are addressed, it’ll be ready to be merged.

[sylv256]

I believe I've fixed the formatting and coverage now.

[juntyr]

Ok, just a couple more minor nits - thank you again for implementing the extension!

[juntyr]

I believe I've fixed the formatting and coverage now.

Yes, thank you! It seems one unrelated #[derive(Debug)] line has regressed in https://app.codecov.io/gh/ron-rs/ron/pull/522/blob/tests/non_string_tag.rs though

[juntyr]

LGTM!

[juntyr]

@SylvKT Thank you so much again for implementing this extension!