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

Support Maps in ReflectiveConfigGroup #2828

Closed
wants to merge 2 commits into from

Conversation

marecabo
Copy link
Contributor

@marecabo marecabo commented Oct 10, 2023

This change allows to store Maps of primitive Java types within ReflectiveConfigGroup using the @Parameter annotation and closes #2815

Keys will always be quoted and properly XML-excaped when written to the config file.

Example

@Comment("map of LocalDate")
@Parameter
public Map<Character, LocalDate> localDateMapField = Map.of('d', LocalDate.of(2023, 10, 9));

will result in

<!-- map of LocalDate -->
<param name="localDateMapField" value="{&quot;d&quot;:[2023,10,9]}" />

@marecabo marecabo marked this pull request as ready for review October 11, 2023 15:05
@marecabo marecabo force-pushed the maps-in-config-group branch from 11c9e5e to 5e5c5d8 Compare October 13, 2023 09:20
@marecabo marecabo requested a review from michalmac October 13, 2023 09:55
@marecabo marecabo force-pushed the maps-in-config-group branch from 5e5c5d8 to 5b1d4c7 Compare October 13, 2023 12:14
@marecabo marecabo force-pushed the maps-in-config-group branch from 5b1d4c7 to 3be97c6 Compare October 14, 2023 09:19
Copy link
Member

@michalmac michalmac left a comment

Choose a reason for hiding this comment

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

This PR is technically correct and looks good to me. I just wonder if this feature will be used often enough to justify the additional complexity of the code. Especially, the need of escaping (e.g. value="{&quot;d&quot;:[2023,10,9]}") may be discouraging.

@jfbischoff jfbischoff closed this Nov 1, 2024
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.

Support Maps of Java primitives in ReflectiveConfigGroup
3 participants