Add Tilt::Mapping#pipeline for handling template pipelines

[jeremyevans]

This makes it easy to combine multiple template engines into a single processing pipeline, by taking the output of one template and using it as the input to another template, returning the final output as the result of the pipeline.

As this is self contained, it could easily be implemented as an external gem. However, I think the functionality is useful and you may want to ship it with Tilt, so I thought I'd send a pull request here before trying to package it as an external gem.

[judofyr]

Interesting. What's the use case? I'm a bit hesitant to add more code to maintain if it won't be used a lot. That sad, this is a very tiny bit of code…

[jeremyevans]

A good use case for this is mentioned in the docs, with something like foo.scss.erb. This allows you to preprocess the scss with erb. There are other similar cases, like foo.coffee.erb. Both of these are supported by the Rails asset pipeline, specifically mentioned in the Rails docs (http://guides.rubyonrails.org/asset_pipeline.html#preprocessing), and actively used by a lot of Rails users, so this feature will make it easier for Rails users to transition to a different framework that uses Tilt for rendering, such as Sinatra or Roda.

Another use case for this is this allows you to add local variable support to any template type that doesn't support local variables, by preprocessing it with another template type that does support local variables, such as erb or str.

[judofyr]

There's already Mapping#templates_for which gets you from "foo.bar.scss.erb" to the template classes. My main concern with this API is how to use it in practice. How does the caller know where the file extension ends and the template extensions start (e.g. foo.css.erb). How should the caller cache the generated template class? This API also uses one convention for passing in options, but does that fit well with how the caller is storing template options already?

I think it would be more useful if:

• It was possible to add default options in Mapping. It's something that every caller already implements.
• Pipelined template classes are cached within a single Mapping.
• The user-facing API is mapping.multinew(filename) (or something similar to mapping.new).

[judofyr]

Sorry if I'm unclear. My main concern with the proposed API is that it doesn't seem much easier for the framework to use it over templates_for. It still needs to cache the pipeline and make sure options are added properly.

[jeremyevans]

This doesn't really replace templates_for, though it certainly could use that internally instead of manually splitting the extension string. All templates_for does is return an array of template classes, so if the caller wants to use templates_for and have a pipeline, they have to manually construct the pipeline. This constructs the pipeline for them.

For callers, because they need to register each pipeline they want to use individually, it shouldn't be the case that they are confused where file extensions end and templates extensions start. By default, for a file like foo.scss.erb or foo.css.scss.erb, Tilt is only going to render it with erb. This allows the user to do mapping.pipeline('scss.erb') to register the scss.erb extension, so that foo.scss.erb and foo.css.scss.erb will both be rendered by erb and then scss.

The mapping.pipeline caller does not need to cache/store the generated class, as the generated class is cached/registered in the mapping. The main reason the generated class is returned is so it can be assigned to a constant to give it a name.

mapping.pipeline allows you to set default options for the template when constructing the Pipeline subclass when registering a pipeline. It's fairly easy to add support for per template options, and merge them into the default options, similar to how the other template classes work. I can certainly add that if this is something you plan to support.

I don't think that mapping.multinew(filename) is a good API for this, at least if you mean you can pass in any filename and it will determine which template processors to run. Then you run into the issue you mentioned with not knowing where the file extensions end and where the template extensions start. Even if we wanted to support that, this code generates a separate class for each pipeline type, and generating a class per template is probably a bad idea. Plus, the whole point of this is works transparently, so that you can just call mapping.new and get a template that will use a pipeline.

I'm not sure if I understood all of the issues you had with this, but if I've missed anything, please let me know.

[judofyr]

Oh, I completely missed that it actually registers within the mapping. I thought it just returned the template class directly. That changes it a bit.

The question is then: do we want callers to specify the pipelines ahead of time? I haven't seen any Ruby frameworks where you must configure template pipelines before you can use them. I can see some advantages (more explicit), but it does not seem like what the current frameworks want to use.

[jeremyevans]

I prefer the explicit approach of registering the pipelines ahead of time. Trying to work without registering in advance raises ambiguity issues and is probably worse from a performance perspective.

We could lazy register some common pipelines (e.g. scss.erb, coffee.erb) if you want this to work out of the box for common cases.

[judofyr]

I'm really thorn on this. Personally I agree with you (explicit is better than implicit), but I don't see Rails/Padrino/Other frameworks using this API as-is.

[jeremyevans]

Fair enough. Do you want some more time to think about it, or should I publish it as an external gem? You could always integrate the external gem later if you want.

[judofyr]

External gem is fine. I've delayed this decision to Tilt 2.1, which will be released after I resolve all of the pull requests for new template classes.

[jeremyevans]

I guess I was a bit late in updating this, but I added an external gem for this last October: https://rubygems.org/gems/tilt-pipeline

[nonnenmacher]

Thanks Jeremy !

It could be much helpful to have this mentioned in the documentation as an example as it is a pain to discover that it already as been described in issue #121 with a candid solution like

def render_multi(file)
  Tilt.templates_for(file).inject(nil) do |data, tmpl|
    blk = proc { data } if data
    tmpl.new(file, &blk).render
  end
end

that is far less elegant than yours !

Also I rediscover an old issue that I raised one ay I was facing the same use case (issue #287). I will definitely add your gem to my toolbox, tanks again ;-)

And I concur about the fact that this PR, should really be added as part of the main gem IMHO.

[jeremyevans]

@judofyr Any additional thoughts on this? I don't have a problem adding this to tilt, but if you are still leaning against, I'll just close this. People can always use the external tilt-pipeline gem if they want this.

[nonnenmacher]

does this means you'll also remove the paragraphe about this PR on tilt-pipeline gem ?

We use a lot of such pipelines but no problem to use tilt-pipeline instead of what we coded along with the spirit of the PR.

[jeremyevans]

does this means you'll also remove the paragraphe about this PR on tilt-pipeline gem ?

Yes, I'll do that after this is merged or closed.

[jeremyevans]

I've rebased this on master and made the requested updates, other than Pipeline.for (let me know if you want that).

[judofyr]

LGTM!