Add more lenient coercing - #1172
Conversation
Sorry, something went wrong.
|
discussion here: https://hubspot.slack.com/archives/CU7RT16A3/p1711737996514909?thread_ts=1711643951.818689&cid=CU7RT16A3 |
Sorry, something went wrong.
| if (value.matches(".*\\..*,.*")) { | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Prefer precompiled regex expressions
Sorry, something went wrong.
| if (!value.matches("\\d*\\.\\d*")) { | ||
| return value; | ||
| } |
There was a problem hiding this comment.
Prefer precompiled regex expressions
Sorry, something went wrong.
| if (value instanceof String) { | ||
| String sanitizedValue = trimDecimal(trimCommas((String) value)); | ||
| return super.coerceToByte(sanitizedValue); | ||
| } |
There was a problem hiding this comment.
Don't think we should do this when coercing to byte
Sorry, something went wrong.
| assertThat( | ||
| jinjava.renderForResult("{{ \"150.25,0\" is ge 4 }}", new HashMap<>()).getErrors() | ||
| ) | ||
| .isNotEmpty(); |
There was a problem hiding this comment.
Some languages have decimals and commas flipped and should be handled in the opposite manner
Sorry, something went wrong.
There was a problem hiding this comment.
Hmm, would your suggested approach here be to make the TruthyTypeConverter constructor take in a JinjavaContext param in order to access in the current language?
Sorry, something went wrong.
There was a problem hiding this comment.
or perhaps just a Locale
Sorry, something went wrong.
| if (value instanceof String) { | ||
| String sanitizedValue = trimCommas((String) value); | ||
| return super.coerceToDouble(sanitizedValue); | ||
| } |
There was a problem hiding this comment.
I'd prefer these to use similar logic as in IntFilter and FloatFilter. Make use of NumberFormat#parse
Sorry, something went wrong.
Reviewers
Requested changes must be addressed to merge this pull request.
strings can be implicitly coerced to numeric values, so comparisons like:
{% if "1000" > 5%}works. However evaluating:
{% if "1,000" > 5%}or:
{% if "1000.25" > 5%}throws an error. This adds some additional cleaning to allow formatted number strings to be parsed.
Notably it does not remove decimals when comparing to floats, since those are valuable and:
{% if "1000.25" > 50.0 %}already works.
It also only removes characters after decimals if its a properly formatted decimal number (so containing no non-digit characters and only one decimal point)