Add fs_preserveFieldCasing attribute #306
Conversation
This enables overriding the behavior of --normalize-field-names at the field, struct, or table level.
|
@jamescourtney I don't seem to have permission to add reviewer but FYI. cc @vvuk |
|
Thanks for this! I always appreciate contributions. I'll try to figure out how to get @vvuk added as a contributor. In the meantime, I will take a look at this, but I'll need you to be a little patient as real-life obligations prevent me from spending too much time on FlatSharp these days. Thanks again for the contribution -- I'll get back to you with my feedback in a few days. |
Codecov Report
@@ Coverage Diff @@
## main #306 +/- ##
==========================================
- Coverage 95.75% 95.73% -0.03%
==========================================
Files 114 114
Lines 7806 7816 +10
Branches 740 746 +6
==========================================
+ Hits 7475 7483 +8
Misses 229 229
- Partials 102 104 +2
Continue to review full report at Codecov.
|
| bool? preserveFieldCasing = field.Attributes != null ? preserveFieldCasing = new FlatSharpAttributes(field.Attributes).PreserveFieldCasing : default; | ||
|
|
||
| var preserve = (preserveFieldCasing ?? preserveFieldCasingParent) switch | ||
| { | ||
| false or null => false, | ||
| true => true, | ||
| }; |
There was a problem hiding this comment.
This seems odd, why bool? and default instead of just bool and false? Would also get rid of the switch, especially if PreserveFieldCasing on the attributes did a ?? false to explicitly specify the default there.
There was a problem hiding this comment.
I did it this way to match similar logic for NonVirtual here:
I initially had something simpler, but it didn't properly handle field overriding struct/table setting case. There may indeed be a simpler check than I have though.
|
@jamescourtney thanks -- @joncham and I work together, so you may see PRs for Unity integration coming from both of us over the next little while. |
| }; | ||
|
|
||
| if (!preserve) | ||
| field.Name = NormalizeFieldName(field.Name); |
There was a problem hiding this comment.
Nitpick: Please use curly braces.
|
@jamescourtney do you want a followup PR with different naming? |
|
@joncham -- I think it's fine. I may change it to |
This enables overriding the behavior of --normalize-field-names at the field, struct, or table level.
This PR attempts to implements a solution for issue #305 . I did make the attribute a bit more verbose as
fs_preserveFieldCasingbut am happy to use the original suggestion offs_preserveCaseas well.