-
Notifications
You must be signed in to change notification settings - Fork 582
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 a template_path option to Mojolicious::Renderer::render #1782
base: main
Are you sure you want to change the base?
Conversation
…w to pass the path to a specific template file.
Hi Mojolicious maintainer Even if you don’t have the time to review this now, could you tell me if this PR seems reasonable and whether it could accepted or if I must find a different approach? Thanks, |
Since this introduces a new reserved stash value the PR is at the very least incomplete. |
How does this interact with the template cache? Is it protected from accessing files outside of registered template directories? |
The new reserved stash value is now documented in Mojolicious/Controller.pm, let me know if it should be added somewhere else. As far as I can tell, the interaction with the cache is reasonable: templates specified with template_path will be cached using their (relative or absolute) path as the cache key. In theory this could conflict with a template specified with If needed, that can probably be fixed by adding some prefix to the name returned by Mojolicious::Renderer::template_name() for these templates specified by this new mechanism. |
Template cache names would be an full_path_to_template_file instead of file name currently. There are many things from old times that need requirements to rewrite template cache names. |
Do you have examples of something that requires being fixed? If there are issues with the PR I can try to improve it but I’m not sure that I understand the problem that you describe. |
|
Can you elaborate on what you mean? As far as I understand, templates are cached by names today and this PR does not change that. For templates referenced with It is possible that this PR could actually help the author of that #1630 question. Even if it does not, I still don’t understand what features could be broken by this PR . |
And would change that. |
Are you saying that this existing issue of cache key collision should be fixed in this PR? Otherwise I’m not sur what you mean. As far as fixing that bug, I don’t think that I would know how to fix it correctly and it is also mostly independent of this PR. I suppose that a discussion should also happen on whether fixing it is worth it. Given that the condition to trigger that bug – dynamically altering the template directory – seems like a very specific usage pattern, the answer to that discussion is not obvious to me and I’m not the right one to lead this discussion. So, to go back to my PR, I guess the questions that I amm asking you (the Mojolicious maintainers) are the following ones:
Does that sound reasonable? What is needed to move forward with this PR (or to close it if it is not judged useful)? |
Add a template_path option to Mojolicious::Renderer::render that allow to pass the path to a specific template file.
Summary
Currently only templates found inside directories registered with Mojolicious::Renderer::paths can be used. This PR allows to use arbitrary template files.
Motivation
This is useful to reference template files that may have clashing names within different directories (so using relative paths from registered directory would be ambiguous).
In my case, I want to allow my user to provide path to plugins directories (themes for a website) that may each contain a "template/view.html.ep" files for example (so they would all look the same if added the the renderer paths).
References
PR #1780 is actually a work-around for the same feature request (loading an arbitrary file in memory and then rendering it as an inline template with a layout). While I think that the other PR has merit on its own, this here is actually a more straightforward solution to my problem which has the advantage of not changing an existing API so it should not break existing users.