Add support for "custom" transformer/merges

[vdemeester]

I'm not sure the name transformer is not the right one.

But on docker/cli#569 we need to have a way to merge specific types differently that the default. This PR does that, on top of #45 👼

[the4thamigo-uk]

I was thinking about doing something very similar to support issue #46. It would be nice to combine these ideas.

I have some general thoughts on this PR though, especially in relation to making it more generic :

• I think it would be cleaner if, instead of having a transformer for each type, to simply specify a single transformer, as input to the public functions. Clients can then implement their own logic i.e. for Fail on conflict #46 I want to define a transformer that returns an error if the source and destination values are both not-nil and not-blank. With this PR, I would have to write a transformer func(dst reflect.Value, src reflect.Value) error for every type for which I want to apply that rule. Also I think, you could in principle, rewrite the existing overwrite behaviour with a transformer of this type.
• Also I think it would be useful if instead of returning just error we returned (continue bool, err error). So that a transformer can decide whether to use the default merge code by specifying continue == true. This would be useful for issue Fail on conflict #46 and other kinds of transformation logic.
• This PR modifies the existing public functions. It would be nice if we only added functions, to support existing users of the library.

[vdemeester]

@the4thamigo-uk thx :)

I think it would be cleaner if, instead of having a transformer for each type, to simply specify a single transformer, as input to the public functions. Clients can then implement their own logic i.e. for #46 I want to define a transformer that returns an error if the source and destination values are both not-nil and not-blank. With this PR, I would have to write a transformer func(dst reflect.Value, src reflect.Value) error for every type for which I want to apply that rule.

Actually you could have only a single transformer that has all the logic. Transformer(reflect.Type) func(dst, src reflect.Value) error doesn't force you to return different function for each type 👼. The way it's done now, you can either do one single transformer or compose then upstream (in the client code). But really, both would be fine to me.

Also I think it would be useful if instead of returning just error we returned (continue bool, err error). So that a transformer can decide whether to use the default merge code by specifying continue == true. This would be useful for issue #46 and other kinds of transformation logic.

I like that idea 👼

This PR modifies the existing public functions. It would be nice if we only added functions, to support existing users of the library.

Right, but it uses variadic arguments (can be 0 args then), and thus it support existing user of the library 👼. That's why there is no change in test files for example 😉. If we go for only one transformer, this is a different story and I do agree, I don't want to break current user of the library 👼

[the4thamigo-uk]

Actually you could have only a single transformer that has all the logic. Transformer(reflect.Type) func(dst, src reflect.Value) error doesn't force you to return different function for each type 👼. The way it's done now, you can either do one single transformer or compose then upstream (in the client code). But really, both would be fine to me.

Well, ok it doesnt force you, but it implies that behaviour I think and it leads to a more constraints on the kind of rules you can write (and adds complexity). Also, you cant list a rule that is based on the types of both the source and destination types, unless everything is pushed into the transformer, so why not just have the transformer and forget about the list/slice. Personally I would go for simplicity...

Right, but it uses variadic arguments (can be 0 args then), and thus it support existing user of the library 👼. That's why there is no change in test files for example 😉. If we go for only one transformer, this is a different story and I do agree, I don't want to break current user of the library 👼

This is true but this isn't sufficient to make it compatible. i.e. an existing client code could have stored the merge function in a value, but since the function types are now different it is a breaking change.

[the4thamigo-uk]

Also #5 should be implementable with this change.

[darccio]

I'll check your proposal this weekend. Thanks!

[darccio]

Although this is a backwards-compatibility breaking change, I think it adds flexibility to Mergo. I trust Docker's crew and the test looks solid, so it's in :)

[darccio]

BTW, @vdemeester, please, could you add an example in README? Thanks!