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

Fix the indentation of default values during ADL serialization #653

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

J3173
Copy link
Collaborator

@J3173 J3173 commented Jan 24, 2025

Follow-up of #651

When serializing Archetypes, indentation was applied incorrectly to default values. Spaces were added to multiline ODIN strings to each line after the first and serialization of unparsed JSON got extra unnecessary indentation applied.

For the indentation to work correctly the process is split into three cases:

  • Unparsed values in DefaultValueContainers (e.g. when no metamodels or mappers are available or the format is unknown): These will be serialized as-is in the DefaultValueContainer. For ODIN this will be unformatted, as any formatting is lost during parsing. For JSON and other formats the serialization will be the same as in the original ADL, keeping any original indentation without adding any extra.
  • Parsed values serialized as JSON: This will use the current process of serializing the default value to JSON and then indenting each line in the serialized JSON to the correct level, as this can be safely done with JSON.
  • Parsed values serialized as ODIN: This will use the new ODINPrettyPrinter introduced in Fix indenting of multiline ODIN strings in ADL #651 to directly serialize the ODIN with the correct indentation.

This will make the roundtrip serialization of unparsed values and parsed values as ODIN idempotent, as the seriazization of parsed values as JSON already was.

@J3173 J3173 self-assigned this Jan 24, 2025
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

Attention: Patch coverage is 91.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 71.99%. Comparing base (88daaf1) to head (1daa7b3).

Files with missing lines Patch % Lines
...izer/adl/constraints/CComplexObjectSerializer.java 91.30% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master     #653      +/-   ##
============================================
+ Coverage     71.94%   71.99%   +0.04%     
- Complexity     7087     7093       +6     
============================================
  Files           672      672              
  Lines         23137    23149      +12     
  Branches       3755     3755              
============================================
+ Hits          16647    16665      +18     
+ Misses         4745     4742       -3     
+ Partials       1745     1742       -3     

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

@J3173 J3173 requested a review from MattijsK January 27, 2025 16:29
Copy link
Collaborator

@MattijsK MattijsK left a comment

Choose a reason for hiding this comment

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

Also looks great!

@J3173 J3173 merged commit 78ff09f into master Jan 28, 2025
4 checks passed
@J3173 J3173 deleted the fix-default-value-indentation branch January 28, 2025 09:08
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.

2 participants