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 shorthand include support #5

Open
donovanglover opened this issue Feb 12, 2018 · 9 comments
Open

Add shorthand include support #5

donovanglover opened this issue Feb 12, 2018 · 9 comments

Comments

@donovanglover
Copy link

Usually when working with static site generators, there are two common directories: one for includes and one for the layouts that use these includes. Example:

{% include "includes/header.html" %}
...
{% include "includes/footer.html" %}

Since this type of pattern is so common, I propose the following shorthand:

{% include header %}
...
{% include footer %}

How does it work? If no explicit path is given (i.e. no quotation marks), look inside the default includes directory (customizable by the user), then:

  1. If a filename (sans extension) matches the given include, use it. If more than one file is matched, raise an error that the include is ambiguous.
  2. Otherwise, raise a Crinja::TemplateNotFoundError like usual.
@straight-shoota
Copy link
Owner

Thanks for suggesting this, however I'm not so sure this would be a valuable improvement as is.

First of all, including files from different directories is not a big deal. You can easily add includes and to the loader path to make these files easily accessible without a path. But as I understand, that's not the real issue here.

I want to avoid changes that would make Crinja templates incompatible to the original Jinja2 template language. I think it has great value for both to be compatible (at least as good as possible), so people who already know Jinja don't need to learn different semantics and templates can be easily reused. I wouldn't dogmatically exclude any changes diverging from Jinja, but if one is to be accepted, it should come with a very reasonable benefit.

The feature you're describing with just plain template names is similar to how Liquid handles includes. The way this works in Jinja and Crinja is however more logical and powerful: The include tag receives an argument with the name of the template to include. From a language design perspective, this name should be a string. If applied directly, this should be expressed a string literal, not a keyword. Additionally, this behaviour allows to pass in any expression as include name.

{% include header %} actually loads the include by name based on the value of the variable header. I don't think there is a good reason to give up dynamic name resolution just to save some quotes when using a string literal.

That being said, there is actually an experimental config setting liquid_compatibility_mode which enables a behaviour similar to the way liquid works. It basically works by checking if the value supplied as name is undefined and if so, uses the expression itself as name. That's obviously very hacky. I've been testing how much effort it would be to render Liquid templates with Crinja (surprisingly, it's really quite easy). But it's very unlikely to stay like this.

@donovanglover
Copy link
Author

Thanks for the thorough reply. I don't quite understand how FileSystemLoader works. The main issue is that I want to avoid duplicating includes/ everywhere in the templates. It seems irrelevant since all my includes are already in includes/, and the duplication doesn't add much to what's happening.

I tried using a variable header as you described and it works great. The only problem I could think of is when another key has the same name as an include, although I could always namespace the other variables.

@straight-shoota
Copy link
Owner

The default include path is ., but if all your includes are in the includes folder, you can just use that one. An example of using FileSystemLoader is in the readme. The constructor for that class can even take an argument of paths, so you can easily define multiple locations to look for files.

This loader, for example will first try to resolve the include name in . and if it can't find it, in ./includes. That's essentially keeping the . as base include path but having a short cut for includes in the includes folder.

crinja.loader = FileSystemLoader.new([".", "./includes"])

@donovanglover
Copy link
Author

Thanks! I got it to work, but when I use this method HTML tags like <p> are converted to &lt;p&gt; for variables like {{ content }}. How would I get the old behavior?

@straight-shoota
Copy link
Owner

What do you mean with old behaviour?

Crinja has autoescape enabled by default for .html and .xml files. That means, the value of content will be automatically escaped before it is printed to the output. If you want it to print as raw html, you can put it through the raw filter: {{ content | raw }}.

To disable autoescape entirely, set crinja.config.autoescape = false. Though I wouldn't recommend this, because then you'll have to manually escape all the places where you don't want to accidentally print HTML code (e.g. protecting against XSS attacks).

@donovanglover
Copy link
Author

Thanks. I use the output from markd as input to Crinja. It's weird that the variables weren't escaped before, though. I did some testing and found out that HTML is only escaped with get_template. I was using a string before which explains the different behavior.

require "crinja"

data = {"world" => "<span>world</span>!"}

puts Crinja::Template.new("Hello {{ world }}").render(data)
# => "Hello <span>world</span>!"

crinja = Crinja.new
crinja.loader = Crinja::Loader::FileSystemLoader.new([".", "./includes"])

puts crinja.from_string("Hello {{ world }}").render(data)
# => "Hello <span>world</span>!"

puts crinja.get_template("test.html").render(data)
# => "Hello &lt;span&gt;world&lt;/span&gt;!"

{{ content | raw }} would definitely be safer, I agree. I'm trying to use that instead but get an error that it isn't registered. I assume that I have to do something with register_defaults? I'm asking a lot of questions, but it seems like Crinja is more powerful than I initially thought.

@straight-shoota
Copy link
Owner

Oh, my bad. It's safe, not raw. I got confused while talking about differences 😄 The latter is the Liquid filter.

If you parse a template directly from a string, Crinja can't know it is intended to be HTML content. Therefore it won't automatically enable autoescape. If you load a template from a file, the default setting determines the escape behaviour based in the file extension.
To enable autoescape with templates loaded using from_string, you can set crinja.config.autoescape = true.

@donovanglover
Copy link
Author

Thanks, that makes sense.

Everything is working now, thanks for the help!

@straight-shoota
Copy link
Owner

You're welcome.

I'll try to improve the documentation on these things based on this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants