Skip to content

fix(decode): use invariant culture for decimal strings - #245

Merged
MangelMaxime merged 2 commits into
thoth-org:mainfrom
ThisFunctionalTom:fix/decimal-decoding-invariant-culture
Jun 3, 2026
Merged

fix(decode): use invariant culture for decimal strings#245
MangelMaxime merged 2 commits into
thoth-org:mainfrom
ThisFunctionalTom:fix/decimal-decoding-invariant-culture

Conversation

@ThisFunctionalTom

Copy link
Copy Markdown
Contributor

Summary

This fixes decimal decoding from JSON strings when the current process culture uses a comma decimal separator.

Decode.decimal was using the current culture for string parsing, which meant values like "0.7833" could fail under cultures such as de-DE even though JSON numbers use . as the decimal separator.

Changes

  • parse decimal strings with CultureInfo.InvariantCulture
  • apply the fix in both Thoth.Json and Thoth.Json.Core
  • add a regression test that switches the current culture to de-DE and verifies "0.7833" still decodes successfully

Validation

  • .\\build.bat test newtonsoft
  • .\\build.bat test system-text-json

@njlr

njlr commented May 7, 2026

Copy link
Copy Markdown
Contributor

Is there a test case with a negative number?
Not every culture & number style accepts - IIRC

@ThisFunctionalTom

Copy link
Copy Markdown
Contributor Author

Is there a test case with a negative number? Not every culture & number style accepts - IIRC

Didn't know that :-).

Have changed the test to use negative number and bigger then 1000 for the 1000 separator and test it with different cultures.

@ThisFunctionalTom

Copy link
Copy Markdown
Contributor Author

One question. I just read the documenation of the easybuild.commitlinter and easybuild.changeloggen.

Should I squash the two commits and force-push so that the changelog is clean?

@MangelMaxime

Copy link
Copy Markdown
Contributor

One question. I just read the documenation of the easybuild.commitlinter and easybuild.changeloggen.

Should I squash the two commits and force-push so that the changelog is clean?

You don't need to do that because when accepting the PR I am squashing everything instead of merging.

@MangelMaxime
MangelMaxime merged commit 7e3cf65 into thoth-org:main Jun 3, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants