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 the partial template func to return any type, not just strings #5783

Closed
bep opened this Issue Mar 24, 2019 · 5 comments

Comments

Projects
None yet
2 participants
@bep
Copy link
Member

bep commented Mar 24, 2019

This replaces #5222

The partial func we have today is mostly used to build macros that print something.

I have thought long and hard about how we could find a simple way for the partial or a similar to return values to the caller. Now I have found a way.

Some important parts about it is:

  • It should not be something completely new (e.g. we're not adding a new directory below /layouts)
  • It should work with both partial and partialCached
  • We may consider adding aliases for partial, but it should be the same thing.

What I have in mind would look like this in the partial:

{{ return (index site.RegularPages .) }}

From the template that uses this "partial function":

{{ $page := partial "getpage-by-index" 42 }}
{{ with $page }}
{{ .Title }}
{{ end }}

The only real question I have about the above is error handling. I kind of like the simplicity of the return syntax above, so we may just add a new Result wrapper object that could be created with some error utility methods, e.g:

{{ $v := newResult }}
// if error
{{ $v.Errorf "invalid index %d" . }}
// else 
{{ $v.Set  (index site.RegularPages .) }}
{{ return $v }}

Or something like that. We could possibly wait with the the "error handling" problem and just implement the "return part" of the above.

Thoughts?

/cc @regisphilibert @onedrawingperday @anthonyfok @budparr @kaushalmodi and gang

@bep bep self-assigned this Mar 24, 2019

@bep bep added the Enhancement label Mar 24, 2019

@bep bep added this to the v0.55 milestone Mar 24, 2019

@regisphilibert

This comment has been minimized.

Copy link

regisphilibert commented Mar 24, 2019

I am a strong supporter of the possibility to use custom returning functions in Hugo.

Yet, regarding this critical need, I still strongly think a new thing should be added rather than tweaking a universally used one. And here's why:

  1. The partial function is accepted as a an "output" function. It outputs something (and one can decide to store that output in a variable).
  2. The partial function is currently limited to its very own context, which remains at the sole discretion of the partial's user as it is passed as an argument.

In order to make this outputting function also a "returning" function, I understand that several things are needed:

  • Introducing methods without a context, thus introducing a reserved variable $_out, which may already be in use in several Hugo projects.
  • Introducing a returned object containing at least two variables .Results and .Error.
    The following would therefor not be possible: {{ with partial "getpage-by-index" 42 }}{{ .Title }}{{ end }}
  • Educating users about this new kind of partials whose "singularity" is only detectable through code reading or usage. (Is layouts/partials/format-title a returning or outputting partial, do I need to test for .Result or not ?)

On the other hand...

If a new thing was introduced, à la shortcode, it could:

  1. take arguments as a regular function does rather than by wrapping arguments in a single argument (dict "first_arg" 45 "second_arg" "other").
  2. sport its own context and methods (.Get, .Set, .Result, .Error etc...)
  3. be clearly identify-able by theme users
@bep

This comment has been minimized.

Copy link
Member Author

bep commented Mar 24, 2019

@regisphilibert do you understand my proposed solution?

It boils down to one sentence:

Allow the partial template func to return any type, not just strings.

The new thing is a new keyword, return (no reserved variable).

Also note that any proposed alternative solution would be limited by the restriction of Go templates, meaning "no regular function" with proper arguments. So, you can name it something else, but that would be purely a cosmetic change that would add a lot of work for no good reason.

@regisphilibert

This comment has been minimized.

Copy link

regisphilibert commented Mar 24, 2019

I think that one sentence worked better for me yes.

Then my only comment would be, and I think I made that point in the other issue, do we really need error handling this bad? Couldn't it be the responsibility of the function writer to test scenarios and return false ?

I'm trying to avoid having to test for .Result before using the returned value, but again, I may be missing something :)

@bep

This comment has been minimized.

Copy link
Member Author

bep commented Mar 24, 2019

@regisphilibert I think I messed up this issue by talking about error handling. I think it helps progress if we just delay that problem. People can use errorf to fail build etc.

Also note that we will probably add support for some "other" scripting language in the future, but that is a harder problem.

The solution to this particular issue ("Allow the partial template func to return any type, not just strings.") has a tremendous value/work ratio.

Just to be clear. As a caller you need to know what the partial will return, but if you do, this would be the common way to call it:

{{ $page := partial "getpage-by-index" 42 }}
{{ with $page }}
{{ .Title }}
{{ end }}

@bep bep changed the title Allow partials to return values Allow the partial template func to return any type, not just strings Mar 24, 2019

@regisphilibert

This comment has been minimized.

Copy link

regisphilibert commented Mar 24, 2019

The solution to this particular issue ("Allow the partial template func to return any type, not just strings.") has a tremendous value/work ratio.

I agree. I've used jsonify to serve this purpose with {{ $data := partial "get-something" | unmarshal }}, but it's a hack and is limited to simple maps ans slice, not complex objects as pages and collections of pages.

Addressing a proper way to return any kind of data through partial would be really great.

bep added a commit to bep/hugo that referenced this issue Mar 24, 2019

bep added a commit to bep/hugo that referenced this issue Mar 24, 2019

bep added a commit to bep/hugo that referenced this issue Mar 25, 2019

bep added a commit to bep/hugo that referenced this issue Mar 25, 2019

bep added a commit to bep/hugo that referenced this issue Mar 25, 2019

Allow the partial template func to return any type
This commit adds support for return values in partials.

This means that you can now do this and similar:

    {{ $v := add . 42 }}
    {{ return $v }}

Partials without a `return` statement will be rendered as before.

Fixes gohugoio#5783

@bep bep closed this in #5788 Apr 2, 2019

bep added a commit that referenced this issue Apr 2, 2019

tpl: Allow the partial template func to return any type
This commit adds support for return values in partials.

This means that you can now do this and similar:

    {{ $v := add . 42 }}
    {{ return $v }}

Partials without a `return` statement will be rendered as before.

This works for both `partial` and `partialCached`.

Fixes #5783

nguyenvanduocit added a commit to 12bitvn/hugo that referenced this issue Apr 5, 2019

tpl: Allow the partial template func to return any type
This commit adds support for return values in partials.

This means that you can now do this and similar:

    {{ $v := add . 42 }}
    {{ return $v }}

Partials without a `return` statement will be rendered as before.

This works for both `partial` and `partialCached`.

Fixes gohugoio#5783
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.