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

Custom transformer can't get around isEmptyValue logic #118

Closed
johnSchnake opened this issue May 15, 2019 · 4 comments
Closed

Custom transformer can't get around isEmptyValue logic #118

johnSchnake opened this issue May 15, 2019 · 4 comments
Labels

Comments

@johnSchnake
Copy link

I was specifically trying to write a transformer so that I could differentiate between nil slices (the zero value) and an empty slice. However, the transformer doesnt get applied because of this line https://github.com/imdario/mergo/blob/master/merge.go#L66

Which says it only applies the custom transform if the dst value is not isEmptyValue which mergo defines as len(slice)==0

So even when I define a custom transform I can't implement this as I wanted. What is the rational for the extra empty check? It seemed intuitive to me that the user's transformation would always get applied when it matched the type. Seems like this is related to #89 and other bool/zero value issues.

@johnSchnake
Copy link
Author

This is phrased as a question I suppose but my intent is more closely represented by:

I hit a piece of logic which wasn't clear to me that I couldn't/shouldn't be able to override. I would like a way to either:

  • apply transforms regardless of the isEmptyValue logic
  • override that isEmptyValue logic in general so I could provide a new option such as DifferentiateEmptyAndNil

And the follow up is that if there is hard 'no' to both of those ideas (I'm happy to try and implement the fixes) then I must be missing some understanding about why that logic exists and is non-overridable (and I'd love to be enlightened).

Thanks! Let me know if you are open to one or both of those ideas and I can try to implement. Obviously the former is much more trivial than a completely new override for handling empty/nil.

@chrislovecnm
Copy link

Did you figure out a solution for this?

@johnSchnake
Copy link
Author

No, I worked around this entirely by saving the values I needed, running mergo, then applying fixups to the merge logic manually.

@darccio
Copy link
Owner

darccio commented May 17, 2020

Sorry for not getting this earlier. Please, can you provide an example? Since this issue was created, some features have landed in master, but I'm not sure what you are trying to accomplish. I get the idea of what you want but not the context where it would be used.

Mergo is getting too much complex and the corner cases are getting harder to understand without more explanation.

@darccio darccio closed this as completed Jul 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants