webform_conditional_operator_string_not_equal() just does the inverse of webform_conditional_operator_string_equal(), so it should be a simple wrapper around the latter.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | webform-comparison_functions-2307631-5.patch | 1.86 KB | liam morland |
| #3 | webform-comparison_functions-2307631-3.patch | 2.27 KB | liam morland |
| #1 | webform_webform_conditional_operator_string_not_equal_2307631.patch | 852 bytes | liam morland |
Comments
Comment #1
liam morlandFix.
Comment #2
danchadwick commentedGreat work.
Looking at the code,
string equal returns TRUE when the rule value matches any one of input values (A or B or C)
string not equal returns TRUE when the rule values matches none of the input values.(~A AND ~B AND ~C).
These are indeed boolean complements of each other, so I agree with this patch. :)
However, I'd like to see the same clean-up done to other operator pairs:
string contains and string does not contain
numeric equal and numeric non equal
Also numeric greater than and numeric less than don't reference equal, and therefore ignore the epsilon value of 0.000001 and don't special-case a blank input value (empty string). An empty string casts to float 0.0 in > and <, but is always considered inequal and never equal (even to 0). This is not consistent. Presumably there would be some refactoring of the 4 numeric functions.
Comment #3
liam morlandThis patch fixes all the *_not_* functions and in webform_conditional_operator_numeric_equal() removes the special handling of empty string, which is now just cast to float.
Comment #4
danchadwick commentedI think we still want the special handling for empty. Empty should not equal anything, me thinks. And I think empty should be neither > nor < anything, as well as being != to everything.
Comment #5
liam morlandDan, I think we have scope creep. This issue is about replacing redundant code with negated calls to another function. Fixing string handling in numeric comparisons should be a separate issue, which I an happy to work on. Focused patch attached.
Comment #7
danchadwick commentedAdded a missing semi-colon, tested, and committed to 7.x-4.x and 8.x.
Comment #10
danchadwick commentedUm, thanks testbot. Not so much.