-
Notifications
You must be signed in to change notification settings - Fork 36
derive: look for templates in caller directory, too #556
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
base: main
Are you sure you want to change the base?
Conversation
67c633b to
864235a
Compare
| Because top-level content from the child template is thus ignored, the `extends` | ||
| tag doesn't support whitespace control: | ||
|
|
||
| ```html | ||
| {%- extends "base.html" +%} | ||
| ``` | ||
|
|
||
| The above code is rejected because we used `-` and `+`. For more information | ||
| about whitespace control, take a look [here](#whitespace-control). |
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.
The replaced text was outdated since #241 / askama v0.13. Oops.
| The path to include must be a string literal, so that it is known at | ||
| compile time. Askama will try to find the specified template relative | ||
| to the including template's path before falling back to the absolute | ||
| template path. Use `include` within the branches of an `if`/`else` | ||
| block to use includes more dynamically. |
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.
I have no idea what the sentence ↓ meant, so I removed it. :)
Use
includewithin the branches of anif/elseblock to use includes more dynamically.
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.
Nice, thanks for implementing this! You've done this before I even had a chance to tackle this! (which is nice!)
I think there was a slight misunderstanding about what kind of macro we are talking about. In #551, the issue I've created, I meant we should look for templates in the Jinja/Askama macro caller directory (i.e. {% call macro() %})—you can see exactly what I meant by looking at the (failing) test case I've added here. This PR, on the other hand, adds a lookup in the Rust macro caller directory. Both fix my use case, however, so I'm satisfied either way!
Just one comment from me about the implementation.
| "template {:?} not found in directories {:?}", | ||
| path, self.dirs, | ||
| path.display(), | ||
| self.dirs, |
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.
Is this accurate? We check more directories than just self.dirs (even before the changes in the PR). I believe this should be changed so that this displays self.dirs along with start_at and caller_dir(), so it displays accurate information to the library user.
|
Since I opened #558, I'll wait for it to be merged first before approving this one if you don't mind. ^^' |
Resolves #551.
Cc. @m4tx.