TFormula: support locale settings for which the decimal separator is "," #17327
Conversation
| TString value = TString::Format("%lf", (*constIt).second); | ||
| // #17225: we take into account LC_LOCALE settings for which the decimal separator | ||
| // is , instead of ., e.g. de_AT.UTF-8 | ||
| TString value = TString::Format("%lf", (*constIt).second).ReplaceAll(",", "."); |
There was a problem hiding this comment.
Shouldn't this be dependent on the value of LC_LOCALE? (in some local we might have 1,200.00 for one thousand two hundreds).
There was a problem hiding this comment.
it's a good observation. But how can we adapt to all possible locale?
There was a problem hiding this comment.
actually it seems that there are only 3 decimal separators: comma, period and arabic decimal separator (https://randombits.dev/articles/number-localization/locale-list)
There was a problem hiding this comment.
Thinking more about it... If we have an arabic decimal separator, it means that we also have arabic digits. At that point, I am unsure whether the decimal separator would be the first problem we hit...
There was a problem hiding this comment.
Actually, I think we need a larger conversation about local and how to handle them. It seems odds/confusion to all support local for this single functions. Until we have this conversation, I recommend we issue an explicit error when encountering a comma here.
Test Results 18 files 18 suites 4d 4h 24m 48s ⏱️ For more details on these failures, see this check. Results for commit c7a2716. ♻️ This comment has been updated with latest results. |
Fixes #17225