Skip to content
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

Twig Extension, remove type array, add .twig ext, add none filter #26

Merged
merged 6 commits into from Jul 15, 2016
Merged

Conversation

vonboth
Copy link
Contributor

@vonboth vonboth commented Jul 13, 2016

No description provided.

@WyriHaximus
Copy link
Collaborator

I'm not inclined to reject this PR but interested in why the removal of the array type hint.

@vonboth
Copy link
Contributor Author

vonboth commented Jul 14, 2016

Hi
if you follow the link you will see the error PHP shows because of the type hint.
The function expects an array but the Token parser passes in a string.
Removing the type hint solves this. But I haven't checked any side effects...

The filter returns and stops further filtering propagation.
Use the filter in CakePHP/Twig in combination with Helper functions returning Objects instead of strings.
E.g. $this->Html->addCrumb('Some Crumb') becomes {{ Html.addCrumb('Some Crumb') | none }}
@WyriHaximus WyriHaximus changed the title Twig Extension, remove type array Twig Extension, remove type array, add .twig ext, add none filter Jul 14, 2016
@@ -40,6 +40,7 @@ public function getFilters()
new \Twig_SimpleFilter('isMultibyte', 'Cake\Utility\String::isMultibyte'),
new \Twig_SimpleFilter('utf8', 'Cake\Utility\String::utf8'),
new \Twig_SimpleFilter('ascii', 'Cake\Utility\String::ascii'),
new \Twig_SimpleFilter('none', function($string) {return;}),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WyriHaximus
Copy link
Collaborator

The function expects an array but the Token parser passes in a string.
Removing the type hint solves this. But I haven't checked any side effects...

Right that makes sense, I'll merge this PR and tag a new release after you fixed the code style issue. If you don't prefer to do that I gladly merge it and do it no self 😄 .

@vonboth
Copy link
Contributor Author

vonboth commented Jul 15, 2016

Hej, all done. Cheers

@WyriHaximus WyriHaximus merged commit b994cfb into cakephp:master Jul 15, 2016
@WyriHaximus
Copy link
Collaborator

Thanks! I'll tag it later today 👍

@WyriHaximus
Copy link
Collaborator

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.

None yet

2 participants