Skip to content

Commit

Permalink
tpl/collections: Fix handling of different interface types in Slice
Browse files Browse the repository at this point in the history
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
  • Loading branch information
bep committed Oct 2, 2018
1 parent ce264b9 commit 10ac2ec
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 20 deletions.
4 changes: 4 additions & 0 deletions hugolib/resource_chain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
41 changes: 22 additions & 19 deletions tpl/collections/collections.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
64 changes: 63 additions & 1 deletion tpl/collections/collections_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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)

Expand Down

0 comments on commit 10ac2ec

Please sign in to comment.