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

Fix twig compare #3186

Merged
merged 1 commit into from Nov 7, 2019

Conversation

@dpinheiro
Copy link

dpinheiro commented Nov 1, 2019

On twig 2.x the comparison between two variables was being done using the php operators (<, >, <=..), although on 3.x the function twig_compare was introduced and it had some issues

As an example
'200' > 399 was returning false on twig 2.x, but on 3.x it was returning true.

Digging into the code I found out the comparison binaries (GreaterBinary, LessBinary, etc) were doing the wrong comparison, and the function twig_compare was also returning an wrong value if both variables had the same type

Now it's following the convention -1 if first var is less, 0 if they are equal, or 1 if first var is greater

This is my first PR, so please comment if anything is wrong

@stof

This comment has been minimized.

Copy link
Member

stof commented Nov 4, 2019

For the CI setup, it will be fixed by #3187

@dpinheiro dpinheiro force-pushed the dpinheiro:fix/compare branch from 472ceb9 to 7271b38 Nov 4, 2019
@dpinheiro

This comment has been minimized.

Copy link
Author

dpinheiro commented Nov 4, 2019

Rebase done

@xabbuh
xabbuh approved these changes Nov 6, 2019
@fabpot fabpot force-pushed the dpinheiro:fix/compare branch from 7271b38 to 0d81c6e Nov 7, 2019
@fabpot

This comment has been minimized.

Copy link
Contributor

fabpot commented Nov 7, 2019

Thank you @dpinheiro.

fabpot added a commit that referenced this pull request Nov 7, 2019
This PR was squashed before being merged into the 3.x branch (closes #3186).

Discussion
----------

Fix twig compare

On twig 2.x the comparison between two variables was being done using the php operators (<, >, <=..), although on 3.x the function `twig_compare` was introduced and it had some issues

As an example
`'200' > 399` was returning `false` on twig 2.x, but on 3.x it was returning `true`.

Digging into the code I found out the comparison binaries (`GreaterBinary`, `LessBinary`, etc) were doing the wrong comparison, and the function `twig_compare` was also returning an wrong value if both variables had the same type

Now it's following the convention `-1` if first var is less, 0 if they are equal, or 1 if first var is greater

This is my first PR, so please comment if anything is wrong

Commits
-------

0d81c6e Fix twig compare
@fabpot fabpot merged commit 0d81c6e into twigphp:3.x Nov 7, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
5 participants
You can’t perform that action at this time.