-
-
Notifications
You must be signed in to change notification settings - Fork 224
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
feature: Add embed node #888
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a really cool (and useful!) feature, it's sort of like components in a way. Had a few thoughts and nits.
.config | ||
.find_template(embed.path, Some(&self.input.path))?; | ||
let mut embedded_context = | ||
Context::new(self.input.config, &self.input.path, embed.nodes.as_slice())?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's missing errors for doing anything outside block tags. Some of these don't actually do anything (extends for example gets overwritten), but you're allowed to write macro and include nodes here, which seems weird (this is similar behavior to how normal extended templates work, so maybe not a real issue?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I see why this is problematic, but it's consistent. I think this is something, that should be fixed/improved in general.
Another real problem, that I mentioned in the issue (#488) is, that the compiler is not including the file when the embed
is inside a block
in an extend
ed file. This problem however also exists for include
s and I'm not sure how to approach this.
Co-authored-by: Matthew Taylor <[email protected]>
In memory of my old issue/feature request in https://github.com/djc/askama/issues/488, here is finally the implementation of it. Tests are included of cause.
I have (not yet) updated the documentation for it. If this feature and implementation is fine, I'm open for updating the documentation to describe the new possibilities.Edit: I did update the documentation now, as I think it might be easier to understand, what it's actually doing.