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.

Sign up for GitHub

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

Jump to bottom

Add an option to allow duplicate keys in mappings #52

Closed
dumbbell opened this issue Jan 13, 2022 · 3 comments · Fixed by #53
Closed

Add an option to allow duplicate keys in mappings #52

dumbbell opened this issue Jan 13, 2022 · 3 comments · Fixed by #53
Assignees
@dumbbell
Labels
enhancement

Comments

@dumbbell
Copy link
Collaborator

yamerl 0.9.0 fixed the situation where duplicate keys in mappings were not checked, and thus allowed. Some users relied on that behavior, even though it is invalid from a YAML specification point of view.

Both behaviors should be fine and there should be an option to enable the old behavior, meaning that duplicate keys would be kept when the constructure structure permits it.

See #39 and @bobkocisko comment.
@dumbbell dumbbell self-assigned this Jan 13, 2022
@dumbbell dumbbell mentioned this issue Jan 13, 2022
Closed
@flurin
Copy link
Contributor

flurin commented Jan 24, 2022

I expect the change that fixes #39 to return an error instead of just taking the last key. As this is not the case we would very much like to have an option to enable the old behaviour again (especially useful in comination with the detailed_constr option.)

flurin added a commit to flurin/yamerl that referenced this issue Jan 24, 2022
@flurin
Add allow_duplicate_keys for maps in construction
a60a462 
This will restore the old behaviour if detailed_constr has been enabled.

Fixes yakaz#52
@flurin flurin mentioned this issue Jan 24, 2022
Merged
@flurin
Copy link
Contributor

flurin commented Jan 24, 2022

@dumbbell I have created the #53 PR to add the option. This is my first bit of Erlang code so I hope it is up to par. Let me know what you think / what needs improvement to get this merged.

@dumbbell
Copy link
Collaborator Author

@flurin: Thank you so much! I will add further comments to the pull request directly.

flurin added a commit to flurin/yamerl that referenced this issue Jan 25, 2022
@flurin
Add keep_duplicate_keys for maps in construction
1c299ff 
This will restore the old behaviour if detailed_constr has been enabled.

Fixes yakaz#52
flurin added a commit to flurin/yamerl that referenced this issue Jan 25, 2022
@flurin
Add keep_duplicate_keys for maps in construction
f26ce3a 
This will restore the old behaviour if detailed_constr has been enabled.

Fixes yakaz#52
flurin added a commit to flurin/yamerl that referenced this issue Jan 25, 2022
@flurin
Add keep_duplicate_keys for maps in construction
fd5272e 
This will restore the old behaviour if detailed_constr has been enabled.

Fixes yakaz#52
flurin added a commit to flurin/yamerl that referenced this issue Jan 28, 2022
@flurin
Add keep_duplicate_keys for maps in construction
3c9a26a 
This will restore the old behaviour if detailed_constr or proplist based maps have been enabled.

Fixes yakaz#52
dumbbell added a commit that referenced this issue Jan 28, 2022
@dumbbell
src/yamerl.app.src: Bump version to 0.10.0
39981b9 
This introduces one new feature: it is now possible to keep duplicate
keys in mappings with the option `{keep_duplicate_keys, boolean()}`
(#52, #53). This flag restores the behavior yamerl had before the 0.9.0
release and the merge of #40.

Thank you to @flurin for his contribution!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dumbbell dumbbell

Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add keep_duplicate_keys for maps in construction flurin/yamerl
2 participants
@flurin @dumbbell