From 10ac2ec4466090957e1f6897906ddeb1e0b13673 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Tue, 2 Oct 2018 17:35:33 +0200 Subject: [PATCH] tpl/collections: Fix handling of different interface types in Slice In Hugo `0.49` we improved type support in `slice`. This has an unfortunate side effect in that `resources.Concat` now expects something that can resolve to `resource.Resources`. This worked for most situations, but when you try to `slice` different `Resource` objects, you would be getting `[]interface {}` and not `resource.Resources`. And `concat` would fail: ```bash error calling Concat: slice []interface {} not supported in concat. ``` This commit fixes that by simplifying the type checking logic in `Slice`: * If the first item implements the `Slicer` interface, we try that * If the above fails or the first item does not implement `Slicer`, we just return the `[]interface {}` Fixes #5269 --- hugolib/resource_chain_test.go | 4 ++ tpl/collections/collections.go | 41 +++++++++--------- tpl/collections/collections_test.go | 64 ++++++++++++++++++++++++++++- 3 files changed, 89 insertions(+), 20 deletions(-) diff --git a/hugolib/resource_chain_test.go b/hugolib/resource_chain_test.go index 045c9517a..1f55976f0 100644 --- a/hugolib/resource_chain_test.go +++ b/hugolib/resource_chain_test.go @@ -238,6 +238,10 @@ T1: Content: {{ $combined.Content }}|RelPermalink: {{ $combined.RelPermalink }}| {{ $combinedText := . | resources.Concat "bundle/concattxt.txt" }} T2: Content: {{ $combinedText.Content }}|{{ $combinedText.RelPermalink }} {{ end }} +{{/* https://github.com/gohugoio/hugo/issues/5269 */}} +{{ $css := "body { color: blue; }" | resources.FromString "styles.css" }} +{{ $minified := resources.Get "css/styles1.css" | minify }} +{{ slice $css $minified | resources.Concat "bundle/mixed.css" }} `) }, func(b *sitesBuilder) { b.AssertFileContent("public/index.html", `T1: Content: ABC|RelPermalink: /bundle/concat.txt|Permalink: http://example.com/bundle/concat.txt|MediaType: text/plain`) diff --git a/tpl/collections/collections.go b/tpl/collections/collections.go index 491421631..1d817077f 100644 --- a/tpl/collections/collections.go +++ b/tpl/collections/collections.go @@ -522,33 +522,36 @@ func (ns *Namespace) Slice(args ...interface{}) interface{} { first := args[0] firstType := reflect.TypeOf(first) - allTheSame := firstType != nil - if allTheSame && len(args) > 1 { + if firstType == nil { + return args + } + + if g, ok := first.(collections.Slicer); ok { + v, err := g.Slice(args) + if err == nil { + return v + } + + // If Slice fails, the items are not of the same type and + // []interface{} is the best we can do. + return args + } + + if len(args) > 1 { // This can be a mix of types. for i := 1; i < len(args); i++ { if firstType != reflect.TypeOf(args[i]) { - allTheSame = false - break + // []interface{} is the best we can do + return args } } } - if allTheSame { - if g, ok := first.(collections.Slicer); ok { - v, err := g.Slice(args) - if err == nil { - return v - } - } else { - slice := reflect.MakeSlice(reflect.SliceOf(firstType), len(args), len(args)) - for i, arg := range args { - slice.Index(i).Set(reflect.ValueOf(arg)) - } - return slice.Interface() - } + slice := reflect.MakeSlice(reflect.SliceOf(firstType), len(args), len(args)) + for i, arg := range args { + slice.Index(i).Set(reflect.ValueOf(arg)) } - - return args + return slice.Interface() } type intersector struct { diff --git a/tpl/collections/collections_test.go b/tpl/collections/collections_test.go index 7b15151d2..8f122569c 100644 --- a/tpl/collections/collections_test.go +++ b/tpl/collections/collections_test.go @@ -643,16 +643,76 @@ func TestShuffleRandomising(t *testing.T) { } var _ collections.Slicer = (*tstSlicer)(nil) +var _ collections.Slicer = (*tstSlicerIn1)(nil) +var _ collections.Slicer = (*tstSlicerIn2)(nil) +var _ testSlicerInterface = (*tstSlicerIn1)(nil) +var _ testSlicerInterface = (*tstSlicerIn1)(nil) + +type testSlicerInterface interface { + Name() string +} + +type testSlicerInterfaces []testSlicerInterface + +type tstSlicerIn1 struct { + name string +} + +type tstSlicerIn2 struct { + name string +} type tstSlicer struct { name string } +func (p *tstSlicerIn1) Slice(in interface{}) (interface{}, error) { + items := in.([]interface{}) + result := make(testSlicerInterfaces, len(items)) + for i, v := range items { + switch vv := v.(type) { + case testSlicerInterface: + result[i] = vv + default: + return nil, errors.New("invalid type") + } + + } + return result, nil +} + +func (p *tstSlicerIn2) Slice(in interface{}) (interface{}, error) { + items := in.([]interface{}) + result := make(testSlicerInterfaces, len(items)) + for i, v := range items { + switch vv := v.(type) { + case testSlicerInterface: + result[i] = vv + default: + return nil, errors.New("invalid type") + } + } + return result, nil +} + +func (p *tstSlicerIn1) Name() string { + return p.Name() +} + +func (p *tstSlicerIn2) Name() string { + return p.Name() +} + func (p *tstSlicer) Slice(in interface{}) (interface{}, error) { items := in.([]interface{}) result := make(tstSlicers, len(items)) for i, v := range items { - result[i] = v.(*tstSlicer) + switch vv := v.(type) { + case *tstSlicer: + result[i] = vv + default: + return nil, errors.New("invalid type") + } } return result, nil } @@ -675,6 +735,8 @@ func TestSlice(t *testing.T) { {[]interface{}{nil}, []interface{}{nil}}, {[]interface{}{5, "b"}, []interface{}{5, "b"}}, {[]interface{}{tstNoStringer{}}, []tstNoStringer{tstNoStringer{}}}, + {[]interface{}{&tstSlicerIn1{"a"}, &tstSlicerIn2{"b"}}, testSlicerInterfaces{&tstSlicerIn1{"a"}, &tstSlicerIn2{"b"}}}, + {[]interface{}{&tstSlicerIn1{"a"}, &tstSlicer{"b"}}, []interface{}{&tstSlicerIn1{"a"}, &tstSlicer{"b"}}}, } { errMsg := fmt.Sprintf("[%d] %v", i, test.args)