-
Notifications
You must be signed in to change notification settings - Fork 335
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
Culture support for in operator and Sort function #2539
base: main
Are you sure you want to change the base?
Conversation
@@ -737,23 +738,25 @@ public static Func<IRContext, FormulaValue[], FormulaValue> StringInOperator(boo | |||
|
|||
var leftStr = (StringValue)left; | |||
var rightStr = (StringValue)right; | |||
|
|||
return new BooleanValue(irContext, rightStr.Value.IndexOf(leftStr.Value, exact ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase) >= 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does services
always have a CultureInfo value? In other places where I see CultureInfo being accessed (mostly in LibraryText.txt) it comes from the runner directly, not from the service provider.
return n1.Value.CompareTo(n2.Value) * compareToResultModifier; | ||
var n2 = b.sortValue as TPFxPrimitive; | ||
CultureInfo culture; | ||
if (n1.Value is string n1s && n2.Value is string n2s && (culture = runner.GetService<CultureInfo>()) != null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
…r-fx into gregli/culture-tr
The
in
operator andSort
function were not culturally aware, resulting in incorrect results for Turkish and I'm sure other languages.The
Lower
,Upper
,Proper
, andSortByColumns
functions are already culturally aware.Our regular expression functions are not culturally aware, but that is an interesting design choice that is the subject of #2538.
For testing, this PR adds the ability to change the culture in this local REPL only, with a
Language
function overload that takes a language tag as the first argument. Also added is a#SETUP
handler to specify the culture for a .txt test file.Removed
ToLower
from StringValue. It doesn't have enough context to do this on its own and be self contained, better to be very clear about how it is being done withCultureInfo
.