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

Feature: Implement complex expressions #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jonathanverner
Copy link

I found it annoying that the {% if %} tag only allowed simple expressions (no and, or, ...)
which forced me to nest if blocks. Similarly I found it convenient to be able to write

{{ year-1 }}

instead of populating the scope with a new variable $prev_year. These two commits are trying to implement the two features.

The first commit implements a simple stack-based expression parser/evaluator which allows compound expressions in the {% if %} tag. The second commit, which I am less sure of, implements expression evaluation for variable blocks.

I am not quite sure the second commit doesn't break something (although all unit tests pass and I didn't notice any breakage) because it changes the way the parseArguments method works. That method is not used in many places, thought.

The way it works now is as follows:

When parsing a variable block in parseArguments the code iterates over the list of tokens until it reach a filter_start token or the end. At that point it adds an expression_end token to the result and continue exactly as in the original code. So the difference in parseArguments is only that a new expression_end token is added to the returned array.

The rest of the work is done in the render method of the VariableNode where we first call Evaluator::eval_expression (the expression parser/evaluator) to evaluate the expression and then
we apply any potential filters.

  {% if person.age > legal_age and person.age < (legal_age+2)* 2 %}
     You are neither young nor old
  {% else %}
     You are either young or old
  {% endif %}
  {{ year-1 }}, {{ 2*pi*(r+10) }}, ...
@speedmax
Copy link
Owner

Thanks for this work. Personally, I like the idea of supporting more expression.

There might be too much logic in the template level. It's a little bit overdone given not many designer can understand the concept of a mod (%)

Building lightweight view-model or presenter objects is probably a much better way to go. Here is a example of some h2o template objects in ruby, you can implement something pretty similar in php as well https://gist.github.com/speedmax/4507b56d0e8f7d97492b

@jonathanverner
Copy link
Author

Did you mean you do not like the idea of supporting more expressions? (Otherwise I don't understand your arguments against them :-) )

Personally, I don't think adding arithmetical/boolean expressions is putting too much logic into the template, but that is my opinion :-) Also note that even the wiki says that you can join boolean expressions with and and or (which you currently can't!) and it seems natural to extend it
to simple arithmetic.

Regarding the lightweight view-model, for the particular use case I had in mind they seem to be overkill: writing a view model for year just so that I can include the previous (or next) year...
Any designer can + or -. As for mod (%) I wouldn't underestimate them :-) Oh, and mod is, in
fact, quite useful: if I wanted to group a list by threes, instead of by twos, I could do

{% for item in list %}
  {% if loop.counter mod 3 = 0 %}
     <hr/>
  {% endif %}
{% endfor %}

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.

2 participants