[patch] Include tag

[robert@…]

This is the include tag I am using in the new-admin branch and in my own projects.
The reasons why {% include "path/to/template" %} is better / more useful than {%ssi /absolute/path/to/template parsed %}:

ssi is a wierd name that only makes sense if you know the history of httpds.

"Server side? This whole thing is server side, right?"

Absolute paths are useless for redistribution, do not allow overriding in different template paths.

"parsed" is a bit arbitrary and meaningless to the target audience for templates.

In {% include "path" %}, constant strings get special cased and hoisted up into the calling template instance rather than being looked up every time. Variables can also be used.

[robert@…]

The patch

[L.Plant.98@…]

Questions about the patch:
1) what is the point of this line in your current patch?:

646: parsed = False

2) It looks like the behaviour depends on whether the path argument is quoted or not. If that's right, is it really a good idea? and shouldn't it be documented? The two are modes are sufficiently different to warrant either a parameter or different tags -- maybe 'insert' or 'readfile' for static files, 'include' for templates.

Cheers,

Luke

[rjwittams]

yep, that assignment on line 646 is pointless.

The constant include optimisation doesn't actually change behaviour, just makes it slightly more efficient.

The point is that this takes a variable as its first argument. If that variable happens to be a constant string,
it is loaded only once rather than being loaded every time it is encountered ( ie in a loop).

ie {% include path_var %} is slightly less efficient than {% include 'path/to/template' %}

So it would kind of remove the point of the optimisation to split this into two tags. To be honest, I would be in support of getting rid of all non-quoted free-form arguments. They are confusing.

And this doesn't deal specially with static files either. Everything is treated as a template. I agree that that might be
worth another tag, but I've not come across a need for it. So files with no tags or variables will work, but they will be one big TextNode and a bit of time will be wasted parsing them.

[L.Plant.98@…]

OK, ignore my second point, I didn't read the patch very carefully (or did so too late in the day!). I think there does need to be some more consistency about arguments and quoting in the template tags.

[rjwittams]

No worries, it needed more explanation. Cheers for catching the pointless assignment.

[L.Plant.98@…]

Patch updated for subversion rev 880 plus

[rjwittams]

updated patch - moved tag to template/loaders.py along with other loader tags.

[hugo]

When playing with this tag, I sometimes got weird errors when using constant names for the included template about NoneType not having get_node_by_type or something like that. It was easily fixed by changing the ConstantIncludeNode from:

class ConstantIncludeNode(Node): 
    def __init__(self, template_path): 
        try: 
            t = get_template(template_path) 
            self.nodelist = t.nodelist 
        except Exception, e: 
            self.nodelist = None

    def render(self, context): 
        if self.nodelist: 
            return self.nodelist.render(context) 
        else: 
            return '' 

to:

class ConstantIncludeNode(Node): 
    def __init__(self, template_path): 
        try: 
            t = get_template(template_path) 
            self.template = t
        except Exception, e: 
            self.template = None 

    def render(self, context): 
        if self.template: 
            return self.template.render(context) 
        else: 
            return '' 

But I am all +1 for applying something like this, because it's much more usefull than ssi.

[rjwittams]

Updated to fix hugos bug

[jhf@…]

Fix minor bugs to work properly with revision 1080

[Adrian Holovaty]

(In [1349]) Fixed #598 -- Added {% include %} template tag. Added docs and unit tests. Thanks, rjwittams