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

Improved compatibility with twig templates #14

Merged
merged 2 commits into from
May 26, 2014
Merged

Conversation

hason
Copy link
Contributor

@hason hason commented May 24, 2014

Fixes: #12, #13

'[_TWITAL_[',
']_TWITAL_]'
);
protected $placeholders = array();
Copy link
Owner

Choose a reason for hiding this comment

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

Store data into a class variable, sounds a little bit strange, and makes this event subscriber not stateless.
State of current template is not stored into a template, but is partially stored into $placeholders variable.

Some future implementation with multiple calls to compiler.pre_load event can't be possible.
I know that this is a "border" case, but i want to hear your opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I store only Twig expressions that are independent on Twital as same as in https://github.com/arnaud-lb/MtHaml/blob/1.4.0/lib/MtHaml/Filter/Markdown.php

@goetas
Copy link
Owner

goetas commented May 26, 2014

Is not my intention to be too strict (closed to changes), but some changes will allow to do things which is really far from http://en.wikipedia.org/wiki/Template_Attribute_Language principles.

This changes will make Twital really close to Twig.

If a developer want to use syntax like this:

<label{% for attrname, attrvalue in label_attr %} {{ attrname }}="{{ attrvalue }}"{% endfor %}>foo</label>

i will suggest to use directly Twig.

Twital has been created to avoid control structures between tags and attributes thats improve readability.

@goetas
Copy link
Owner

goetas commented May 26, 2014

Why not create an extension called FullCompatibilityTwigExtension which ships your FixTwigExpressionSubscriber?

Doing this, the core of Twital can remain tiny and clean. Users that want to use a full compatibility have simply to enable it.

$twital>addExtension(new FullCompatibilityTwigExtension());

(This feature should be documented...)

@hason
Copy link
Contributor Author

hason commented May 26, 2014

@goetas I implemented your great idea.

return array(
// operators

array('<div>{% if foo > 5 and bar < 8 and bar & 4 %}foo{% endif %}</div>'),
Copy link
Owner

Choose a reason for hiding this comment

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

Tests starting from line 75 to 87 should stay into CoreNodesTest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests are duplicated.

goetas added a commit that referenced this pull request May 26, 2014
Added extension which improves compatibility with twig templates
@goetas goetas merged commit a942977 into goetas:master May 26, 2014
@goetas
Copy link
Owner

goetas commented May 26, 2014

Great!

@hason hason deleted the twig branch May 26, 2014 18:50
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.

Error in HTML5Adapter?
2 participants