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

tpl: intersect needs some love #1952

Closed
moorereason opened this Issue Mar 10, 2016 · 7 comments

Comments

Projects
None yet
5 participants
@moorereason
Contributor

moorereason commented Mar 10, 2016

The following test cases fail when added to tpl/template_funcs_test.go:TestIntersect:

{[]string{"a", "b", "c"}, []string{"a", "b", "b"}, []string{"a", "b"}},
{[]interface{}{"a", "b", "c"}, []interface{}{"a", "b", "b"}, []interface{}{"a", "b"}},

Test results:

--- FAIL: TestIntersect (0.00s)
        template_funcs_test.go:791: [0] got [a b b] but expected [a b]
        template_funcs_test.go:791: [1] got [] but expected [a b]

Regarding test 0, in is used by intersect to avoid adding duplicates to the new set. In this case, in is receiving a reflect.Value struct, but the reflect.Struct case is not handled by in.

Regarding test 1, intersect does not handle the reflect.Interface case. See Questions about "intersect" function forum discussion.

Tests were done in Go 1.6 and Hugo HEAD.

@moorereason moorereason added the Bug label Mar 10, 2016

@moorereason moorereason added this to the v0.16 milestone Mar 10, 2016

moorereason added a commit to moorereason/hugo that referenced this issue Mar 14, 2016

tpl: Send actual values to in from intersect
The `intersect` function uses `in` to avoid adding duplicates to the
resulting set.  We were passing `reflect.Value` items when we should
have been using `Value.Interface()` to send the actual data structure.
This fixes that.

See gohugoio#1952

moorereason added a commit to moorereason/hugo that referenced this issue Mar 14, 2016

tpl: Add initial interface logic to intersect func
Add initial interface support to the `intersect` template function.
This will allow the use of JSON data structures with intersect.

I say "initial" because this commit only works when `l1` is an interface.

See gohugoio#1952

bep added a commit that referenced this issue Mar 17, 2016

tpl: Send actual values to in from intersect
The `intersect` function uses `in` to avoid adding duplicates to the
resulting set.  We were passing `reflect.Value` items when we should
have been using `Value.Interface()` to send the actual data structure.
This fixes that.

See #1952
@robertbasic

This comment has been minimized.

Show comment
Hide comment
@robertbasic

robertbasic Apr 3, 2016

Contributor

@moorereason @bep is this issue addressed? (I'm looking for things to contribute to, so just want to avoid working on already fixed stuff).

Contributor

robertbasic commented Apr 3, 2016

@moorereason @bep is this issue addressed? (I'm looking for things to contribute to, so just want to avoid working on already fixed stuff).

@moorereason

This comment has been minimized.

Show comment
Hide comment
@moorereason

moorereason Apr 3, 2016

Contributor

@robertbasic,
The first test above is resolved (#1972).

I have a fix for the second test. It's in one of my personal branches (8e81d2e). However, there's still a piece missing, which this test will hit:

{[]string{"a", "b", "c"}, []interface{}{"a", "b", "b"}, []string{"a", "b"}},

If you want to take my branch as a starting point and finish it out, go for it! I probably won't have time to finish it for a few days.

Contributor

moorereason commented Apr 3, 2016

@robertbasic,
The first test above is resolved (#1972).

I have a fix for the second test. It's in one of my personal branches (8e81d2e). However, there's still a piece missing, which this test will hit:

{[]string{"a", "b", "c"}, []interface{}{"a", "b", "b"}, []string{"a", "b"}},

If you want to take my branch as a starting point and finish it out, go for it! I probably won't have time to finish it for a few days.

@moorereason moorereason modified the milestones: future, v0.16 May 7, 2016

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Feb 28, 2017

Member

This issue has been automatically marked as stale because it has not been commented on for at least four months.

The resources of the Hugo team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, and you feel that it is still valuable, please open a proposal at https://discuss.gohugo.io/.

This issue will automatically be closed in four months if no further activity occurs. Thank you for all your contributions.

Member

bep commented Feb 28, 2017

This issue has been automatically marked as stale because it has not been commented on for at least four months.

The resources of the Hugo team are limited, and so we are asking for your help.

If this is a bug and you can still reproduce this error on the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, and you feel that it is still valuable, please open a proposal at https://discuss.gohugo.io/.

This issue will automatically be closed in four months if no further activity occurs. Thank you for all your contributions.

@bep bep added the Stale label Feb 28, 2017

@bep

This comment has been minimized.

Show comment
Hide comment
@bep

bep Mar 1, 2017

Member

Note/Update: This issue is marked as stale, and I may have said something earlier about "opening a thread on the discussion forum". Please don't.

If this is a bug and you can still reproduce this error on the latest release or the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.

Member

bep commented Mar 1, 2017

Note/Update: This issue is marked as stale, and I may have said something earlier about "opening a thread on the discussion forum". Please don't.

If this is a bug and you can still reproduce this error on the latest release or the master branch, please reply with all of the information you have about it in order to keep the issue open.

If this is a feature request, and you feel that it is still relevant and valuable, please tell us why.

@bep bep added Keep and removed Stale labels Mar 27, 2017

@jacksingleton

This comment has been minimized.

Show comment
Hide comment
@jacksingleton

jacksingleton Mar 29, 2017

I think I've ran into this issue.

When trying to implement related content functionality I thought I'd be able to write:

{{ $has_common_context := intersect .Params.contexts ($.Scratch.Get "context") }}

However, this yields [] even when the slices share common elements.

upon closer inspection ({{ printf "%#v" ... }}) I see that:

.Params.contexts = []string{"blah"}

while

$.Scratch.Get "context" = []interface {}{"blah"}

Anyone know of a work around?

jacksingleton commented Mar 29, 2017

I think I've ran into this issue.

When trying to implement related content functionality I thought I'd be able to write:

{{ $has_common_context := intersect .Params.contexts ($.Scratch.Get "context") }}

However, this yields [] even when the slices share common elements.

upon closer inspection ({{ printf "%#v" ... }}) I see that:

.Params.contexts = []string{"blah"}

while

$.Scratch.Get "context" = []interface {}{"blah"}

Anyone know of a work around?

@jacksingleton

This comment has been minimized.

Show comment
Hide comment
@jacksingleton

jacksingleton Mar 29, 2017

For the record, I suspect this was happening because I was populating "context" in Scratch using slice (in order to normalize situations where only one context exists).

I worked around it by giving up on normalizing a single value into an array of one value, instead branching into a conditional and using in or intersect depending on whether it's a single value or an array:

{{ if $.Scratch.Get "context" }}
  {{ $.Scratch.Set "has_common_context" (in .Params.contexts ($.Scratch.Get "context")) }}
{{ else }}
  {{ $.Scratch.Set "has_common_context" (intersect .Params.contexts ($.Scratch.Get "contexts")) }}
{{ end }}
{{ $has_common_context := ($.Scratch.Get "has_common_context") }}

Note: "contexts" instead of "context" in the else branch.

Ugly, but it works.

jacksingleton commented Mar 29, 2017

For the record, I suspect this was happening because I was populating "context" in Scratch using slice (in order to normalize situations where only one context exists).

I worked around it by giving up on normalizing a single value into an array of one value, instead branching into a conditional and using in or intersect depending on whether it's a single value or an array:

{{ if $.Scratch.Get "context" }}
  {{ $.Scratch.Set "has_common_context" (in .Params.contexts ($.Scratch.Get "context")) }}
{{ else }}
  {{ $.Scratch.Set "has_common_context" (intersect .Params.contexts ($.Scratch.Get "contexts")) }}
{{ end }}
{{ $has_common_context := ($.Scratch.Get "has_common_context") }}

Note: "contexts" instead of "context" in the else branch.

Ugly, but it works.

@samozzy

This comment has been minimized.

Show comment
Hide comment
@samozzy

samozzy Apr 11, 2017

I'm experiencing this issue, but intersect seems entirely broken. The following happens:

  {{ $a := (slice 1 2 3 ) }}
  {{ $b := (slice 1 3 4 ) }}
  {{ $c := intersect $a $b }}
  a: {{ $a }} {{ printf "%#v" $a }} //Result: [1 2 3] []interface {}{1, 2, 3}
  b: {{ $b }} {{ printf "%#v" $b }} //Result:[1 3 4] []interface {}{1, 3, 4}
  c: {{ $c }} {{ printf "%#v" $c }} //Result: [] []interface {}{}

Expected behaviour: $c is [1 3]

Any ideas?

samozzy commented Apr 11, 2017

I'm experiencing this issue, but intersect seems entirely broken. The following happens:

  {{ $a := (slice 1 2 3 ) }}
  {{ $b := (slice 1 3 4 ) }}
  {{ $c := intersect $a $b }}
  a: {{ $a }} {{ printf "%#v" $a }} //Result: [1 2 3] []interface {}{1, 2, 3}
  b: {{ $b }} {{ printf "%#v" $b }} //Result:[1 3 4] []interface {}{1, 3, 4}
  c: {{ $c }} {{ printf "%#v" $c }} //Result: [] []interface {}{}

Expected behaviour: $c is [1 3]

Any ideas?

moorereason added a commit to moorereason/hugo that referenced this issue May 2, 2017

@bep bep closed this in #3328 May 18, 2017

bep added a commit that referenced this issue May 18, 2017

@ghost ghost referenced this issue May 22, 2017

Open

spf13/hugo v0.21 released #10

tychoish added a commit to tychoish/hugo that referenced this issue Aug 13, 2017

tpl: Send actual values to in from intersect
The `intersect` function uses `in` to avoid adding duplicates to the
resulting set.  We were passing `reflect.Value` items when we should
have been using `Value.Interface()` to send the actual data structure.
This fixes that.

See gohugoio#1952
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment