From 047af7cfe5e9aa740b85e0f9974a2d31a0ef4c08 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 27 Aug 2020 21:34:45 -0500 Subject: [PATCH] tpl: Extend merge to accept multiple parameters Fixes #7595 --- tpl/collections/init.go | 23 +++++-- tpl/collections/merge.go | 30 +++++++-- tpl/collections/merge_test.go | 118 +++++++++++++++++++--------------- 3 files changed, 106 insertions(+), 65 deletions(-) diff --git a/tpl/collections/init.go b/tpl/collections/init.go index 17ca16543..1e6412610 100644 --- a/tpl/collections/init.go +++ b/tpl/collections/init.go @@ -116,10 +116,12 @@ func init() { [][2]string{ { `{{ (querify "foo" 1 "bar" 2 "baz" "with spaces" "qux" "this&that=those") | safeHTML }}`, - `bar=2&baz=with+spaces&foo=1&qux=this%26that%3Dthose`}, + `bar=2&baz=with+spaces&foo=1&qux=this%26that%3Dthose`, + }, { `Search`, - `Search`}, + `Search`, + }, }, ) @@ -186,15 +188,22 @@ func init() { ns.AddMethodMapping(ctx.Merge, []string{"merge"}, [][2]string{ - {`{{ dict "title" "Hugo Rocks!" | collections.Merge (dict "title" "Default Title" "description" "Yes, Hugo Rocks!") | sort }}`, - `[Yes, Hugo Rocks! Hugo Rocks!]`}, - {`{{ merge (dict "title" "Default Title" "description" "Yes, Hugo Rocks!") (dict "title" "Hugo Rocks!") | sort }}`, - `[Yes, Hugo Rocks! Hugo Rocks!]`}, + { + `{{ dict "title" "Hugo Rocks!" | collections.Merge (dict "title" "Default Title" "description" "Yes, Hugo Rocks!") | sort }}`, + `[Yes, Hugo Rocks! Hugo Rocks!]`, + }, + { + `{{ merge (dict "title" "Default Title" "description" "Yes, Hugo Rocks!") (dict "title" "Hugo Rocks!") | sort }}`, + `[Yes, Hugo Rocks! Hugo Rocks!]`, + }, + { + `{{ merge (dict "title" "Default Title" "description" "Yes, Hugo Rocks!") (dict "title" "Hugo Rocks!") (dict "extra" "For reals!") | sort }}`, + `[Yes, Hugo Rocks! For reals! Hugo Rocks!]`, + }, }, ) return ns - } internal.AddTemplateFuncsNamespace(f) diff --git a/tpl/collections/merge.go b/tpl/collections/merge.go index c65e9dd90..029416142 100644 --- a/tpl/collections/merge.go +++ b/tpl/collections/merge.go @@ -17,17 +17,35 @@ import ( "reflect" "strings" - "github.com/gohugoio/hugo/common/maps" - "github.com/gohugoio/hugo/common/hreflect" + "github.com/gohugoio/hugo/common/maps" "github.com/pkg/errors" ) -// Merge creates a copy of dst and merges src into it. -// Currently only maps supported. Key handling is case insensitive. -func (ns *Namespace) Merge(src, dst interface{}) (interface{}, error) { +// Merge creates a copy of the final parameter and merges the preceeding +// parameters into it in reverse order. +// Currently only maps are supported. Key handling is case insensitive. +func (ns *Namespace) Merge(params ...interface{}) (interface{}, error) { + if len(params) < 2 { + return nil, errors.New("merge requires at least two parameters") + } + var err error + result := params[len(params)-1] + + for i := len(params) - 2; i >= 0; i-- { + result, err = ns.merge(params[i], result) + if err != nil { + return nil, err + } + } + + return result, nil +} + +// merge creates a copy of dst and merges src into it. +func (ns *Namespace) merge(src, dst interface{}) (interface{}, error) { vdst, vsrc := reflect.ValueOf(dst), reflect.ValueOf(src) if vdst.Kind() != reflect.Map { @@ -60,14 +78,12 @@ func caseInsensitiveLookup(m, k reflect.Value) (reflect.Value, bool) { if strings.EqualFold(k.String(), key.String()) { return m.MapIndex(key), true } - } return reflect.Value{}, false } func mergeMap(dst, src reflect.Value) reflect.Value { - out := reflect.MakeMap(dst.Type()) // If the destination is Params, we must lower case all keys. diff --git a/tpl/collections/merge_test.go b/tpl/collections/merge_test.go index c18664e25..dbe92d8b2 100644 --- a/tpl/collections/merge_test.go +++ b/tpl/collections/merge_test.go @@ -15,82 +15,108 @@ package collections import ( "bytes" - "fmt" "reflect" - "runtime" - "strings" "testing" "github.com/gohugoio/hugo/common/maps" - + "github.com/gohugoio/hugo/deps" "github.com/gohugoio/hugo/parser" - "github.com/gohugoio/hugo/parser/metadecoders" qt "github.com/frankban/quicktest" - "github.com/gohugoio/hugo/deps" ) func TestMerge(t *testing.T) { - ns := New(&deps.Deps{}) simpleMap := map[string]interface{}{"a": 1, "b": 2} for i, test := range []struct { name string - dst interface{} - src interface{} + params []interface{} expect interface{} isErr bool }{ { "basic", - map[string]interface{}{"a": 1, "b": 2}, - map[string]interface{}{"a": 42, "c": 3}, - map[string]interface{}{"a": 1, "b": 2, "c": 3}, false}, + []interface{}{ + map[string]interface{}{"a": 42, "c": 3}, + map[string]interface{}{"a": 1, "b": 2}, + }, + map[string]interface{}{"a": 1, "b": 2, "c": 3}, false, + }, + { + "multi", + []interface{}{ + map[string]interface{}{"a": 42, "c": 3, "e": 11}, + map[string]interface{}{"a": 1, "b": 2}, + map[string]interface{}{"a": 9, "c": 4, "d": 7}, + }, + map[string]interface{}{"a": 9, "b": 2, "c": 4, "d": 7, "e": 11}, false, + }, { "basic case insensitive", - map[string]interface{}{"a": 1, "b": 2}, - map[string]interface{}{"A": 42, "c": 3}, - map[string]interface{}{"a": 1, "b": 2, "c": 3}, false}, + []interface{}{ + map[string]interface{}{"A": 42, "c": 3}, + map[string]interface{}{"a": 1, "b": 2}, + }, + map[string]interface{}{"a": 1, "b": 2, "c": 3}, false, + }, { "nested", - map[string]interface{}{"a": 1, "b": map[string]interface{}{"d": 1, "e": 2}}, - map[string]interface{}{"a": 42, "c": 3, "b": map[string]interface{}{"d": 55, "e": 66, "f": 3}}, - map[string]interface{}{"a": 1, "b": map[string]interface{}{"d": 1, "e": 2, "f": 3}, "c": 3}, false}, + []interface{}{ + map[string]interface{}{"a": 42, "c": 3, "b": map[string]interface{}{"d": 55, "e": 66, "f": 3}}, + map[string]interface{}{"a": 1, "b": map[string]interface{}{"d": 1, "e": 2}}, + }, + map[string]interface{}{"a": 1, "b": map[string]interface{}{"d": 1, "e": 2, "f": 3}, "c": 3}, false, + }, { // https://github.com/gohugoio/hugo/issues/6633 "params dst", - maps.Params{"a": 1, "b": 2}, - map[string]interface{}{"a": 42, "c": 3}, - maps.Params{"a": int(1), "b": int(2), "c": int(3)}, false}, + []interface{}{ + map[string]interface{}{"a": 42, "c": 3}, + maps.Params{"a": 1, "b": 2}, + }, + maps.Params{"a": int(1), "b": int(2), "c": int(3)}, false, + }, { "params dst, upper case src", - maps.Params{"a": 1, "b": 2}, - map[string]interface{}{"a": 42, "C": 3}, - maps.Params{"a": int(1), "b": int(2), "c": int(3)}, false}, + []interface{}{ + map[string]interface{}{"a": 42, "C": 3}, + maps.Params{"a": 1, "b": 2}, + }, + maps.Params{"a": int(1), "b": int(2), "c": int(3)}, false, + }, { "params src", - map[string]interface{}{"a": 1, "c": 2}, - maps.Params{"a": 42, "c": 3}, - map[string]interface{}{"a": int(1), "c": int(2)}, false}, + []interface{}{ + maps.Params{"a": 42, "c": 3}, + map[string]interface{}{"a": 1, "c": 2}, + }, + map[string]interface{}{"a": int(1), "c": int(2)}, false, + }, { "params src, upper case dst", - map[string]interface{}{"a": 1, "C": 2}, - maps.Params{"a": 42, "c": 3}, - map[string]interface{}{"a": int(1), "C": int(2)}, false}, + []interface{}{ + maps.Params{"a": 42, "c": 3}, + map[string]interface{}{"a": 1, "C": 2}, + }, + map[string]interface{}{"a": int(1), "C": int(2)}, false, + }, { "nested, params dst", - maps.Params{"a": 1, "b": maps.Params{"d": 1, "e": 2}}, - map[string]interface{}{"a": 42, "c": 3, "b": map[string]interface{}{"d": 55, "e": 66, "f": 3}}, - maps.Params{"a": 1, "b": maps.Params{"d": 1, "e": 2, "f": 3}, "c": 3}, false}, - {"src nil", simpleMap, nil, simpleMap, false}, + []interface{}{ + map[string]interface{}{"a": 42, "c": 3, "b": map[string]interface{}{"d": 55, "e": 66, "f": 3}}, + maps.Params{"a": 1, "b": maps.Params{"d": 1, "e": 2}}, + }, + maps.Params{"a": 1, "b": maps.Params{"d": 1, "e": 2, "f": 3}, "c": 3}, false, + }, + {"src nil", []interface{}{nil, simpleMap}, simpleMap, false}, // Error cases. - {"dst not a map", "not a map", nil, nil, true}, - {"src not a map", simpleMap, "not a map", nil, true}, - {"different map types", simpleMap, map[int]interface{}{32: "a"}, nil, true}, - {"all nil", nil, nil, nil, true}, + {"dst not a map", []interface{}{nil, "not a map"}, nil, true}, + {"src not a map", []interface{}{"not a map", simpleMap}, nil, true}, + {"different map types", []interface{}{map[int]interface{}{32: "a"}, simpleMap}, nil, true}, + {"all nil", []interface{}{nil, nil}, nil, true}, } { test := test @@ -101,9 +127,7 @@ func TestMerge(t *testing.T) { c := qt.New(t) - srcStr, dstStr := fmt.Sprint(test.src), fmt.Sprint(test.dst) - - result, err := ns.Merge(test.src, test.dst) + result, err := ns.Merge(test.params...) if test.isErr { c.Assert(err, qt.Not(qt.IsNil), errMsg) @@ -112,14 +136,6 @@ func TestMerge(t *testing.T) { c.Assert(err, qt.IsNil) c.Assert(result, qt.DeepEquals, test.expect, errMsg) - - // map sort in fmt was fixed in go 1.12. - if !strings.HasPrefix(runtime.Version(), "go1.11") { - // Verify that the original maps are preserved. - c.Assert(fmt.Sprint(test.src), qt.Equals, srcStr) - c.Assert(fmt.Sprint(test.dst), qt.Equals, dstStr) - } - }) } } @@ -172,9 +188,9 @@ V22 = "v22_2" qt.DeepEquals, map[string]interface{}{ "V1": "v1_1", "V2": "v2_2", - "V2s": map[string]interface{}{"V21": "v21_1", "V22": "v22_2"}}) + "V2s": map[string]interface{}{"V21": "v21_1", "V22": "v22_2"}, + }) } - } func TestCaseInsensitiveMapLookup(t *testing.T) {