Skip to content

Comments

views use TT2's INCLUDE_PATH #955

Closed
veryrusty wants to merge 6 commits intomasterfrom
bugfix/views_tt_inc_path
Closed

views use TT2's INCLUDE_PATH #955
veryrusty wants to merge 6 commits intomasterfrom
bugfix/views_tt_inc_path

Conversation

@veryrusty
Copy link
Member

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

  set views => 'foobar';

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

  1. Let TT2 do the template searching, ( override our default methods for view_pathname and layout_pathname ), and
  2. Sets the default INCLUDE_PATH given to TT2 to be a coderef, allowing the views setting to be modified at runtime.

@xsawyerx
Copy link
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.
@xsawyerx xsawyerx force-pushed the bugfix/views_tt_inc_path branch from 30d2fa6 to 9615c6c Compare October 13, 2015 12:21
@xsawyerx
Copy link
Member

@veryrusty I rebased the branch off master and pushed force. It fails a test now (t/scope_problems/keywords_before_template_hook.t). Could you please take a look at that?

@veryrusty
Copy link
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)

@veryrusty
Copy link
Member Author

PR #1037 provides an alternative (and simpler) solution. I plan on closing this PR and deleting the branch in ~24 hours.

@veryrusty
Copy link
Member Author

Closing in preference of #1037.

@abeverley
Copy link
Contributor

Please see discussion in #1093

@veryrusty veryrusty mentioned this pull request Jan 9, 2016
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.

config key "views" maybe broken with tt?

3 participants