Closed
Conversation
Member
|
Feels awkward but I trust you on this, as I didn't really have time to look into that. Merging? |
Data::Dumper is not used in D2::Core::Role::Template.
Previously we were (typically) passing absolute paths to templates (the ABSOLUTE flag is true in the default tt2 config) which worked in most cases. However, setting 'views' to a path that was not absolute would cause our TemplateToolkit wrapper to concatenate the views path and the tempalte name, pass that to TT2 which would try and do exactly the same. That resulted in templates not being found. Instead, only use TT2's code for template searching. Override the default implementations of view_pathname and layout_pathname so we do not to the path concatenation twice. Also removes the check that the template exists - TT2 will do that for us too! Ref. #951
We no longer concatenate the views path with the template filename. Ensure the tests match.
By using a dynamic generator for TT2's INCLUDE_PATH, changes to the "views" setting will be honoured when rendering templates after the TT2 object is instantiated. (Otherwise the INCLUDE_PATH is static.)
Ensure INCLUDE_PATH is not static.
30d2fa6 to
9615c6c
Compare
Member
|
@veryrusty I rebased the branch off master and pushed force. It fails a test now ( |
Member
Author
|
@xsawyerx - I'm planning on looking at this during the pre-conference hackathon (doing the alternate implementation which doesn't set INCLUDE_PATH) |
Member
Author
|
PR #1037 provides an alternative (and simpler) solution. I plan on closing this PR and deleting the branch in ~24 hours. |
Member
Author
|
Closing in preference of #1037. |
Contributor
|
Please see discussion in #1093 |
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously we were usually passing absolute paths for templates (and have ABSOLUTE => 1 set) when rendering output with TemplateToolkit, which worked for most use-cases.
However, as reported in #951 (with further discussion on IRC), if you set a path 'fragment' for a view, such as
The both Dancer2::Template::TemplateToolkit and TT2's code for handling multiple INCLUDE_PATHs were both concatenating the views directory with the template name to be rendered. (i.e. looking for a template like
foobar/foobar/index.tt).This Pr updates the following to resolve #951
view_pathnameandlayout_pathname), andviewssetting to be modified at runtime.