Task #6757

Feature #26665: Fluid: Implement String comparison

support literal strings in boolean arguments

Added by Peter Niederlag over 11 years ago. Updated about 9 years ago.

Status:
Resolved
Priority:
Should have
Assignee:
-
Category:
Core
Target version:
-
Start date:
2010-03-10
Due date:
% Done:

100%

Estimated time:
Has patch:
No

Description

the 'if' viewhelper should support comparison on strings.

This can easily be fixed by adding [a-zA-Z] on $booleanExpressionTextNodeCheckerRegularExpression in Core/Parser/SyntaxTree/ViewHelperNode.php, see attached patch. if works like a charm with this patch.

However I fear, something else probably breaks badly?


Files

fluid-if-viewhelper.diff (617 Bytes) fluid-if-viewhelper.diff Peter Niederlag, 2010-03-10 10:31

Related issues

Related to TYPO3.Fluid - Feature #5485: Missing operators "===", "!==" and "!" in fluid comparism mechanismRejected2009-11-24

Actions
Has duplicate TYPO3.Fluid - Bug #7039: if ViewHelper: conditions always trueRejected2010-03-28

Actions
#1

Updated by Sebastian Kurfuerst over 11 years ago

  • Project changed from 534 to TYPO3.Fluid
#2

Updated by Sebastian Kurfuerst about 11 years ago

note to myself: we need proper selenium tests for the if ViewHelper.

After thinking about this right now, I think this could actually work. I always thought that the above clashes with the object accessor syntax like {..}, but as they are clearly identified by the braces, there should not be any problem.

So, after proper testing and test coverage, I think this could work out.

Greets,
Sebastian

#3

Updated by Bastian Waidelich about 11 years ago

  • Category set to Core
  • Status changed from New to Accepted
  • Assignee set to Bastian Waidelich
  • Branch set to v4 + v5
#4

Updated by Bastian Waidelich about 11 years ago

  • Subject changed from VieHelper if : support literal strings to support literal strings in boolean arguments
#5

Updated by Christopher Hlubek about 11 years ago

+1 for implementing that from me. I would like string literals surrounded by single quotes better.

#6

Updated by Karsten Dambekalns almost 11 years ago

+1, I am amazed this isn't possible: <f:if condition="{foo} !== 'bar'"><f:then> ...

#7

Updated by Bastian Waidelich over 10 years ago

  • Assignee deleted (Bastian Waidelich)
#8

Updated by Sebastian Kurfuerst over 10 years ago

  • Tracker changed from Bug to Story
  • Priority changed from Should have to Must have
  • Target version set to 1.0 beta 1
#9

Updated by Sebastian Kurfuerst over 10 years ago

  • Tracker changed from Story to Feature
#10

Updated by Sebastian Kurfuerst over 10 years ago

  • Parent task set to #26665
  • Has patch set to No
#11

Updated by Sebastian Kurfuerst about 10 years ago

  • Priority changed from Must have to Should have
  • Target version changed from 1.0 beta 1 to 1230
#12

Updated by Karsten Dambekalns almost 10 years ago

  • Target version deleted (1230)
#13

Updated by Gerrit Code Review about 9 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#14

Updated by Alexander Berl about 9 years ago

I just took the initial idea further and required string literals to be written in single quotes.
This is necessary for allowing white spaces in the strings.
I also added basic unit tests for this behaviour and all other unit tests still pass.
It also works nicely in an extbase project so far, but a functional test probably won't hurt, I just couldn't get functional tests running in Zend Studio yet.

However there are still some questions that I'd like feedback on:

1) Should double quotes also be allowed? IMO it's probably problematic anyway, since boolean expressions are mostly used in viewhelper attributes, which are (double) quoted normally, so the double quotes would have to be escaped. On the other hand, it is standard behaviour to be able to quote with both single and double quotes in most programming languages.

2) What about comparison of string literals against constant numbers? Since the numbers are parsed as string numerics, the comparator compares two strings which will never be equal unless you compare for example '5' == 5.
Also 'string' == 0 will resolve to TRUE in PHP, while with the current solution this will resolve to FALSE, so this might be unexpected behaviour.
This could be solved by casting unquoted numerics to float/int, but I'm not sure how it would work for comparison with variables holding numbers.

#15

Updated by Gerrit Code Review about 9 years ago

Patch set 2 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#16

Updated by Gerrit Code Review about 9 years ago

Patch set 3 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#17

Updated by Alexander Berl about 9 years ago

Ok, so after quite some fiddling with this and the Viewhelpertest package, I come to the following conclusions:

1) proper string -> number comparison is definitely a must have, since else the boolean node behaves differently when comparing a string to a constant number than comparing a string to variable with a constant number value, ie.
('foo' == 1) !== ('foo' == {constantOne})
and this is a no-go IMO (and also leads to un-green-able Viewhelpertest).

2) adding string -> number comparison is not simply achieved by casting the text node, as the input must always be cast on evaluation (text nodes may only be created from strings), but there the distinguishing information of numeric string ("{'foo' == '0'}") or numeric literal ("{'foo' == 0}") is lost, since quotes are stripped beforehand in the boolean node.
Hence, the introduction of a NumericNode (or NumberNode?) is needed.

I will push a changeset shortly.

#18

Updated by Gerrit Code Review about 9 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#19

Updated by Gerrit Code Review about 9 years ago

Patch set 4 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#20

Updated by Gerrit Code Review about 9 years ago

Patch set 5 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#21

Updated by Gerrit Code Review about 9 years ago

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13554

#22

Updated by Gerrit Code Review about 9 years ago

Patch set 1 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13555

#23

Updated by Gerrit Code Review about 9 years ago

Patch set 6 for branch master has been pushed to the review server.
It is available at http://review.typo3.org/13421

#24

Updated by Alexander Berl about 9 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

Also available in: Atom PDF