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

Slice and Maps are handled differently #35

Closed
FrenchBen opened this issue Jan 4, 2017 · 6 comments · Fixed by #45
Closed

Slice and Maps are handled differently #35

FrenchBen opened this issue Jan 4, 2017 · 6 comments · Fixed by #45

Comments

@FrenchBen
Copy link

Say you have something similar to the example you provided in the README , but instead deal with maps and slices, giving you something like this:

package main

import (
	"fmt"
	"github.com/imdario/mergo"
)

type Foo struct {
	A map[string]string
	B []string
}

func main() {
	src := Foo{
		A: make(map[string]string),
		B: []string{"something"},
	}
	src.A["first"] = "1"

	dest := Foo{
		A: make(map[string]string),
		B: []string{"else"},
	}
	dest.A["second"] = "2"
	fmt.Println(src)
	// Will print
	// {map[first:1] [something]}
	fmt.Println(dest)
	// Will print
	// {map[second:2] [else]}

	mergo.Merge(&dest, src)

	fmt.Println(dest)
	// Will print
	// {map[second:2 first:1] [else]}
	// Expected print
	// {map[second:2 first:1] [something else]}
}

Why is it that the behavior of Merge is to append to maps but completely over-write the slice?

@darccio
Copy link
Owner

darccio commented Mar 26, 2017

First of all, sorry. Second, I will take care of this this next week.

@wanghq
Copy link

wanghq commented Oct 7, 2017

@imdario any conclusion on this?

@darccio
Copy link
Owner

darccio commented Oct 9, 2017

I'm working out issues and PRs right now (finally, I know). I'll update this as soon as possible.

@the4thamigo-uk
Copy link

This also is important behaviour for us so we have to roll our own merge function at the moment. Id love to use mergo though as it is more complete.

@darccio
Copy link
Owner

darccio commented Oct 19, 2017

Thanks for the feedback. I'll check this out soon.

@darccio
Copy link
Owner

darccio commented Oct 19, 2017

I must check again map handling in order to be sure that this is the way to go. PR #45 looks good to me but I don't feel sure about this change yet.

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

Successfully merging a pull request may close this issue.

4 participants