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

Render the esi tags with a template to be able to customize it depending... #13

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Render the esi tags with a template to be able to customize it depending... #13

wants to merge 3 commits into from

Conversation

jezdez
Copy link
Contributor

@jezdez jezdez commented Jan 25, 2013

... on the context (e.g. pass in GET vars).

@crccheck
Copy link
Member

Hi, just wondering if you can explain to me what this does in more detail. To me, it just looks like this makes it easy for projects to have their own "esi/esi_tag.html" if they need to customize it on a project-wide level.

@jezdez
Copy link
Contributor Author

jezdez commented Jan 30, 2013

@crccheck Yeah, exactly. Right now the template is hardcoded which makes it hard to modify how the URL is rendered. E.g. I may want to append a user specific GET var to be able to cache those includes on Varnish side.

if self.asvar:
url = context[self.asvar]

render_context = copy.copy(context)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why fully copy the context instead of doing a context.push() before rendering the esi tag template and then popping the context afterwards?

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

Successfully merging this pull request may close these issues.

3 participants