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

Merging Slices with WithAppendSlice/WithSliceDeepCopy need extra features like de-duplication. #259

Open
md2k opened this issue Dec 13, 2024 · 1 comment

Comments

@md2k
Copy link

md2k commented Dec 13, 2024

Hi all, we are heavily depends on terraform and we using provider named cloudposse/terraform-provider-utils their code based on mergo to provide merge capabilities of complex structures. We noticed a issue while structures which can have slices and as for us this behavior far from correct ( maybe this is a feature :) ).

Use case:

We have 2 inputs as yaml which are decoded into terraform structures.

NOTE: in some cases it may happen that we can have dups inside arrays

Config A

list:
  - value-1
  - value-2
  - value-3

Config B

list:
  - value-1
  - value-5
  - value-4

Results if we using WithAppendSlice

list:
  - value-1
  - value-2
  - value-3
  - value-1
  - value-5
  - value-4

Results if we using WithSliceDeepCopy

list:
  - value-1
  - value-2
  - value-3

Result with WithAppendSlice and WithSliceDeduplication

list:
  - value-1
  - value-2
  - value-3
  - value-5
  - value-4

NOTE: order preserved, sort not used to ensure merged lists preserving the same or close to original order.

WithSliceDeepCopy from my understanding supposed to copy items from both slices and merge them together, but this not happening and only Config A is seems copied. I'm not fully sure is that a goal for WithSliceDeepCopy or not?

In a same time WithAppendSlice almost meet our expectations, but as you can see above from result for it, we have duplicates, because both Configs include value-1.
It can be easily fixed by de-duplication of final slice, but current way of WithAppendSlice probably desired result for some cases.

Before to do any PRs i would like to ask if we can add to version 1 of mergo a modifier like WithSliceDeduplication which then can call extra logic within WithAppendSlice scope ?

All we need add extra helper function and condition to enable this function to end of this block: https://github.com/darccio/mergo/blob/master/merge.go#L171-L175

I did locally such functionality and its behavior looks good, plus using options approach we can preserve existing behavior, while with WithSliceDeduplication we can extend it without breaking anyones experience who already using `mergo.

@md2k
Copy link
Author

md2k commented Dec 13, 2024

I can see this is PR and feature for v2 in /pull/196 , but last message from 2023 and no movements about v2 since then, but deduplication feature is the one of very important part we missing with mergo

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

No branches or pull requests

1 participant