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.

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

Allow Scratch.Add with a mix of types #5361

Closed
stp-ip opened this issue Oct 26, 2018 · 4 comments · Fixed by #5364
Closed

Allow Scratch.Add with a mix of types #5361

stp-ip opened this issue Oct 26, 2018 · 4 comments · Fixed by #5364
Milestone

Comments

@stp-ip
Copy link

stp-ip commented Oct 26, 2018

With the change in v0.49 to restrict scratch to the string type only it broke quite a bit of existing behavior using scratch as a variable (this was necessary before variable overwrite was a thing and also has other benefits).

One behavior that broke was the possibility to assign hugo.PageOutput to scratch.

One occurrence is this line: https://github.com/okkur/syna/blob/master/layouts/partials/helpers/fragments.html#L85

Error output:

Building sites … ERROR 2018/10/26 15:14:48 [en] page "/okkur/syna/exampleSite/content/dev/empty-subitems/index.md": render of "page" failed: execute of template failed: template: _default/single.html:4:6: executing "main" at <partial "helpers/fra...>: error calling partial: "/okkur/syna/layouts/partials/helpers/fragments.html:86:17": execute of template failed: template: partials/helpers/fragments.html:86:17: executing "partials/helpers/fragments.html" at <$page_scratch.Add>: error calling Add: append element type mismatch: expected string, got *hugolib.PageOutput

We assing a hugo.PageOutput to scratch instead of a plain string.

Related to #5275

@bep bep added this to the v0.50 milestone Oct 26, 2018
@bep bep added the Bug label Oct 26, 2018
@bep
Copy link
Member

bep commented Oct 26, 2018

Thanks for the report. I see how this was an unintended change, and I will see if I can fix it, but I suspect that this is both very rare and almost always a sign that something else is wrong.

From what I read from your template, you start out by creating a string slice:

{{- $page_scratch.Set "sections" (slice "/") -}}

Then you do some looping and continue to add Page objects to that string slice. I suspect that you need to adjust the logic to work either with strings or Page objects -- I don't see where mixing those would help you.

@stp-ip
Copy link
Author

stp-ip commented Oct 26, 2018

We'll see how a fix makes sense. Another idea would be to move to the new variable overwrite in Go templates, but that's a much bigger change.

What's your general stance on this. It's a bug, but works as it should be now more than it did before or Hugo should preserve the old way instead of breaking this?

@bep
Copy link
Member

bep commented Oct 26, 2018

It's a bug,

Sure, it's a bug. Scratch should be able to append strings and pages in the same slice. I will fix it. But I also claim that there is a logical flaw in your template -- the new append will not help you with that. But that is a discussion for the forum.

@bep bep changed the title Restricting scratch to string type breaks old behavior Allow Scratch.Add with a mix of types Oct 27, 2018
bep added a commit to bep/hugo that referenced this issue Oct 27, 2018
The type handling in these was improved in Hugo 0.49, but this also meant that it was no longer possible to start out with a string slice and later append `Page` etc. to it.

This commit makes sure that the old behaviour is now possible again by falling back to a `[]interface{}` as a last resort.

Fixes gohugoio#5361
@bep bep closed this as completed in #5364 Oct 27, 2018
bep added a commit that referenced this issue Oct 27, 2018
The type handling in these was improved in Hugo 0.49, but this also meant that it was no longer possible to start out with a string slice and later append `Page` etc. to it.

This commit makes sure that the old behaviour is now possible again by falling back to a `[]interface{}` as a last resort.

Fixes #5361
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants