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.

Sign up for GitHub

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

Jump to bottom

Templates in templates #26

Closed
wants to merge 2 commits into from
Closed

Templates in templates #26

wants to merge 2 commits into from

Conversation

kylepaulsen
Copy link

Add recursiveRender option to allow templates to have templates.

I know this is kind of a crazy change, but I'd thought I'd share it with you in case you'd allow it. I feel like this feature really gives me a lot of power for very little set up (see the test case). Feel free to change whatever or ask me to change things.

Thanks anyway for the code.

@kylepaulsen kylepaulsen mentioned this pull request Oct 2, 2014
Closed
@mayo
Copy link

mayo commented Oct 2, 2014

@kylepaulsen Interesting solution. This would add template inheritance-like functionality in libraries that don't support it out of the box, like handlebars. For the ones that do, though, for eg. swig (that's what the test case is written in), jade, and few others, you can already do this like so:

src/index.html

---
template: secondary.html
title: title from src file
---

body

templates/secondary.html

{% extends 'primary.html' %}
{% block body %}
<p>This is some content from the secondary template.</p>
{{ parent }}
<p>More content from the secondary template.</p>
{% endblock %}

templates/primary.html

<html>
<head>
  <title>{{ title }}</title>
</head>
<body>
{% block body %}
{{ contents }}
{% endblock %}
</body>
</html>

@kylepaulsen
Copy link
Author

@mayo, Ah! I never tried using the inheritance features of jade or swig with the metalsmith-templates plugin - I was just really confused about how that would work. But yeah, I was using handlebars when I was looking for this functionality. Thanks for the example code. I may switch to swig or jade.

@kylepaulsen
Copy link
Author

Although one thing this does that I'm not sure can be done currently, is that it renders content like the "inPlace" option does in the content file and then follows and renders the templates.

@ismay
Copy link

ismay commented Oct 30, 2014

@kylepaulsen, if you want to be able to extend templates with Handlebars, you could also use the handlebars-layouts library. It already works for extending files in the templates folder (but does not support rendering content from the src folder, which'll have to be changed from metalsmith-templates).

See issue #22 for further discussion (hope I'm not misunderstanding your PR btw.)

@krawaller
Copy link

I think maybe many use cases for this are solved by the master template proposed in #21?

@ismay
Copy link

ismay commented Nov 1, 2014

@krawaller The master template would allow for one extra level of nesting. But extending templates would allow for unlimited nesting. So I'd say that the master template pr is a little too specific compared to extending templates.

The way I see it this pr actually adds two types of functionality:

  1. Extending templates
  2. Rendering content like inPlace does, whilst still applying the template specified in the front-matter

I think 1. does not have to be solved from within metalsmith-templates. The functionality is already there with swig, jade, etc. or can be added with custom helpers (with handlebars-layouts for example). No need to complicate metalsmith-templates with something that already exists outside of it, in my opinion.

Now 2. is actually not possible without changing metalsmith-templates (or adding an extra step that just processes the content files, with consolidate.js or something, which isn't DRY at all). It would make sense to me to ditch 1. and focus on 2., that's where the real problem lies in my opinion (fixed with an option like processContent or by modifying inPlace).

@ismay ismay mentioned this pull request Nov 1, 2014
Closed
@wbyoung
Copy link

wbyoung commented Jan 5, 2015

After using a few other static site generators, this is something that I was expecting would work out of the box.

I don't think that the ability to have this work with engines like Handlebars is the only advantage here. I've switched to Swig for the project I'm working on to get some of the functionality that's provided here, but being able to define new variables within the front-matter of templates would allow solutions that (as far as I can tell) aren't possible by just using Swig as it doesn't have something similar to a with template.

Coupled with the ability to support recursive template building with any template engine, I think this is a must-have feature for metalsmith-templates.

Obviously, @kylepaulsen is not alone in wanting this to happen. #27 and #21 are requesting similar functionality. This PR seems to me like it's heading down the right track & addresses the issues from both of those PRs. It feels very natural & is very similar to the way that Jekyll handles templates. It's the way that I assumed things would work.

Thanks for all the hard work on this, by the way @ianstormtaylor and @kylepaulsen.

@muchweb muchweb mentioned this pull request Feb 7, 2015
Closed
@joakim
Copy link

joakim commented Jun 9, 2015

I agree with what's being said here, and hope this PR will get some love.

@ismay
Copy link

ismay commented Jun 9, 2015

@joakim Something like this can also be done with handlebars-layouts, see here for an example using metalsmith-templates. Works very well.

This was referenced Jul 11, 2015
Closed
Closed
@ianstormtaylor
Copy link
Contributor

Hey thanks for this sorry for the delay. Now that https://github.com/superwolff has split out this plugin into two separate plugins—layouts and in-place—I'm going to deprecate the metalsmith-templates plugin, so that we can use the simpler ones instead. I'm going to close this PR, but feel free to re-open there!

Added a deprecation notice to the readme.

(I think the idea of recursive layouts is genius btw. Very elegant.)

@wbyoung wbyoung mentioned this pull request Jul 21, 2015
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@kylepaulsen @mayo @ismay @krawaller @wbyoung @joakim @ianstormtaylor