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

Add transform.Unmarshal func #5428

Closed
bep opened this issue Nov 10, 2018 · 9 comments · Fixed by #5552
Closed

Add transform.Unmarshal func #5428

bep opened this issue Nov 10, 2018 · 9 comments · Fixed by #5552

Comments

@bep
Copy link
Member

bep commented Nov 10, 2018

So we can do:

{{ $mydata := resources.Get "mydata.json" | unmarshal }}

The above will obviously make even more sense/value once we get remote support in Get.

  • Use metadecoders.Unmarshal
  • Add support for CSV to the above method, as that will make this in line with what we have in getJSON/getCSV.
  • Check that we have MIME types for all supported formats.

/cc @regisphilibert

@bep bep added this to the v0.52 milestone Nov 10, 2018
@regisphilibert
Copy link
Member

Awesome! This would guess the language ! Even better than GetJSON then.

remote support in Get

Slightly off topic but I’m not sure of what this is referring to as GetJSON already supports remote (http) as I understand it.

@bep
Copy link
Member Author

bep commented Nov 10, 2018

Awesome! This would guess the language !

It's a fairly educated guess ...

GetJSON already supports remote (http) as I understand it.

It does. But resources.Get does not. This may look like we're doing some double work, but the getJSON API is a little clunky and not very flexible. Having these produce something that has a common known interface (Resource) makes them ... pipe-able. And we get a simpler code. I hope that we eventually can "hide"/retire getJSON.

@regisphilibert
Copy link
Member

regisphilibert commented Nov 10, 2018

I may only be asking because of a lack of understanding of the "unmarshaling" process but why not have this available as a resource variable or method, like .Resize for images, rather than a new global .resources transformation function which (I think ?) would only be able to process a few resources (json, csv, yaml, toml) ?

@bep
Copy link
Member Author

bep commented Nov 10, 2018

Instead of me spending time on the "why", could you elaborate on the "why not"? What is the benefit of your way vs my way?

@regisphilibert
Copy link
Member

My way does not introduce a new global transformation resource method.

Mainly because I don't think unmarshaling is going to transform the resource and make available the transformed file on disk like SASS or minify does.
For me, if I understand correctly, its use case sounds more like .Content or .Permalink than resources.Minify or .resources.Concat.
It does not manipulate the resource as much as it interprets its content.

Then again, I may be missing something.

@bep bep changed the title Add resources.Unmarshal transformation Add resources.Unmarshal func Nov 10, 2018
@bep
Copy link
Member Author

bep commented Nov 10, 2018

You are right in that it doesn't transform the Resource into another Resource -- so it's a function. So, this discussion is: Method vs function.

I would say that my main reasons to prefer the latter is:

  • "pipability": {{ $mydata := resources.Get "mydata.json" | unmarshal }} vs {{ $mydata := (resources.Get "mydata.json").Unmarshal }}
  • Also the reason .Content is part of the Resource interface is because all resources support it. Unmarshal will be a very special thing for some text resources (it would make no sense for CSS, to take one example, and we can make better error messages if you try to do so).
  • You may argue that image Resize is different, but that is a set of very specific image related methods, e.g. an API, so I think that is a little different.

@bep bep changed the title Add resources.Unmarshal func Add transform.Unmarshal func Nov 10, 2018
@bep
Copy link
Member Author

bep commented Nov 10, 2018

@regisphilibert I have changed the title of this issue. We already have transform.Remarshal (which we use in the docs site code selector).

So, adding transform.Unmarshal would fit nicely into that. It could accept both strings and Resource objects.

@regisphilibert
Copy link
Member

Awesome! Now everything can be unmarshalled, even front matter params (which could very well hold JSON strings with some remote data sources)!

@bep bep modified the milestones: v0.52, v0.53 Nov 28, 2018
@bep bep modified the milestones: v0.53, v0.54 Dec 6, 2018
@bep bep modified the milestones: v0.54, v0.53 Dec 21, 2018
bep added a commit to bep/hugo that referenced this issue Dec 21, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 22, 2018
bep added a commit to bep/hugo that referenced this issue Dec 23, 2018
@bep bep closed this as completed in #5552 Dec 23, 2018
bep added a commit that referenced this issue Dec 23, 2018
@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 22, 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