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.

Sign up for GitHub

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

Jump to bottom

Merging with zero 0 does not work #255

Open
sebastian-garn opened this issue Jul 1, 2020 · 4 comments · Fixed by go-sprout/sprout#34 · May be fixed by #399
Open

Merging with zero 0 does not work #255

sebastian-garn opened this issue Jul 1, 2020 · 4 comments · Fixed by go-sprout/sprout#34 · May be fixed by #399

Comments

@sebastian-garn
Copy link

sebastian-garn commented Jul 1, 2020 •
edited
Loading

It seems like the value 0 is not merged properly. I have created the following test which currently fails.

func TestMergeZero(t *testing.T) {
	dict := map[string]interface{}{
		"src2": map[string]interface{}{
			"h": 10,
		},
		"src1": map[string]interface{}{
			"h": 0,
		},
		"dst": map[string]interface{}{},
	}
	tpl := `{{merge .dst .src1 .src2}}`
	_, err := runRaw(tpl, dict)
	if err != nil {
		t.Error(err)
	}
	expected := map[string]interface{}{
		"h": 0, // FAILS: I AM EXPECTING 0 BUT 10 IS WHAT I GET
	}
	assert.Equal(t, expected, dict["dst"])
}

Did I miss something?
@moorereason
Copy link

This appears to be the intended behavior. merge uses https://github.com/imdario/mergo under the hood which will overwrite zero values.

This was referenced May 13, 2024
Open
Merged
@huww98
Copy link

huww98 commented May 13, 2024

I've created #399 to fix this. But given this project has not updated for more than a year. I don't have high hopes.

@42atomys 42atomys mentioned this issue May 15, 2024
Merged
6 tasks
42atomys added a commit to go-sprout/sprout that referenced this issue May 15, 2024
@42atomys @huww98
fix: merge should preserve zero value in dest (#34)
ec86e0b 
## Description
Backport fix of sprig into sprout, initially provided by @huww98

## Fixes Masterminds/sprig#255
Masterminds/sprig#399

## Checklist
- [x] I have read the **CONTRIBUTING.md** document.
- [x] My code follows the code style of this project.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] I have updated the documentation accordingly.
- [ ] This change requires a change to the documentation on the website.

Co-authored-by: 胡玮文 <[email protected]>
@42atomys
Copy link

42atomys commented May 16, 2024 •
edited
Loading

Hello everyone 👋,

I wanted to let you know that this issue has been addressed in the fork of this project at go-sprout/sprout. The fix has been implemented starting from version v0.4.0.

For those looking for a solution, I recommend checking out the latest releases of the fork. This should help address the issue discussed in this thread.

Thank to @huww98 for the pull request !

@huww98
Copy link

huww98 commented May 16, 2024

Thank you. But we want to use this in helm. Not sure whether helm will switch to sprout finally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@moorereason @sebastian-garn @42atomys @huww98