mergo
mergo copied to clipboard
darccio
Breaked merge of bool pointers in 0.3.8
Hello, First of all, I would like to thanks contributors for this great library!
We use mergo a lot in our project for merging structs. We have bools there and I'm aware about the issues with merging false values (which can be empty or can be set by user) to the true values. This is why we're using pointers to booleans. Following code worked well with 0.3.7 version and was giving us correct values when overriding *true with *false:
package main
import (
"fmt"
"github.com/imdario/mergo"
)
type Foo struct {
A *bool
B string
}
func main() {
src := Foo{
A: func(v bool) *bool { return &v }(false),
B: "src",
}
dest := Foo{
A: func(v bool) *bool { return &v }(true),
B: "dest",
}
mergo.Merge(&dest, src, mergo.WithOverride)
fmt.Println(*dest.A, dest.B)
// Should print "false src" because of override
}
But 0.3.8 version breaks this correct result of false src for this code and gives us wrong true src value now.
Can you please take a look on this issue? Thank you very much!
Workaround:
package main
import (
"reflect"
"github.com/imdario/mergo"
"github.com/sanity-io/litter"
)
type nullTransformer struct {
}
func (t *nullTransformer) Transformer(typ reflect.Type) func(dst, src reflect.Value) error {
if typ.Kind() == reflect.Ptr {
return func(dst, src reflect.Value) error {
if dst.CanSet() && !src.IsNil() {
dst.Set(src)
}
return nil
}
}
return nil
}
type Container struct {
Test *bool
}
func main() {
bTrue, bFalse := true, false
from, to := &Container{&bFalse}, &Container{&bTrue}
if err := mergo.Merge(to, from, mergo.WithOverride, mergo.WithTransformers(&nullTransformer{})); err != nil {
panic(err)
}
litter.Dump(to)
}
@xt99 The previous behavior caused a bug fixed by #125. Please use mergo.WithOverwriteWithEmptyValue to override with empty values in the next release 0.3.10.
I think mergo.WithOverwriteWithEmptyValue does not fix this. A pointer to false isn't an empty value.
Can we please reopen this issue? mergo.WithOverwriteWithEmptyValue does not fix this.
@pckhoi @normanjaeckel Reopened, but I'm not able to dedicate time to fix it.
@tbnguyen1407 No, sorry. My current job is time-consuming and I'm not able to deal with issues without PRs.
@tbnguyen1407 Still the same situation. Would this code from @octobered help? https://github.com/octobered/mergo/commit/9e3abad214fff9790dd8b1fa2e7b0b1124d52de5
