tfussell / xlnt

:bar_chart: Cross-platform user-friendly xlsx library for C++11+
Other
1.49k stars 418 forks source link

locale aware double->string conversions #467

Closed Crzyrndm closed 4 years ago

Crzyrndm commented 4 years ago

Unlike recent PR's from me, this is almost entirely a correctness improvement. With this branch, local runs of the test suite have no failures even when the locale is changed to one which uses the comma character as its decimal separator (i.e. passes with set_locale(LC_ALL, "de-DE") on windows. the german locale name may be different on other platforms)

This is done by removing all usages of the following functions when applied to floating point types (e.g. double) to convert them to a string representation

Instead these number_serialiser::serialise and number_serialiser::serialise_short use the same technique of coverting the comma to a decimal point when required as was used in #421 to affect the reverse transformation (string->double)

I'm not sure of the best way to add a locale check to CI. Duplicating one of the round trip tests (all formats is probably the best one for this) and modifying the locale while it runs would work, but there's still the issue of different names on different platforms, and which (if any) alternative locales are installed by default

locale tests on windows can be affected by adding the following to the start of the test main setlocale(LC_ALL, "de-DE"); default locale can be restored by putting "C" as the locale name

coveralls commented 4 years ago

Coverage Status

Coverage increased (+0.003%) to 83.575% when pulling 06801f7d362ffec4933f56dd83c007a3c549a9e2 on Crzyrndm:experimental/serialisation into d1d9553d4efa4b5bc9f4258cc90fda35fa7d2e27 on tfussell:master.