Skip to content

Fix inconistent minus sign with different culture#46588

Merged
buyaa-n merged 5 commits intodotnet:masterfrom
buyaa-n:xcontainer_number-sign
Jan 8, 2021
Merged

Fix inconistent minus sign with different culture#46588
buyaa-n merged 5 commits intodotnet:masterfrom
buyaa-n:xcontainer_number-sign

Conversation

@buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Jan 5, 2021

Fixes #45952

XContainer.cs using XmlConvert.ToString(...) overloads for converting some numeric types like float, double, decimal types which uses Invariant culture internally. For other numeral like int, long, short ets converted using Object.ToString() which converts the numbers with default culture which is causing the minus sign (-) converted differently for some cultures like Swedish, they all should converted by XmlConvert.ToString(...) which uses Invariant culture.

@buyaa-n buyaa-n requested a review from krwq January 5, 2021 17:32
@ghost
Copy link

ghost commented Jan 5, 2021

Tagging subscribers to this area: @buyaa-n, @krwq
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #45952

XContainer.cs using XmlConvert.ToString(...) overloads for converting some numeric types like float, double, decimal types which uses Invariant culture internally. For other numeral like int, long, short ets converted using Object.ToString() which converts the numbers with default culture which is causing the minus sign (-) converted differently for some cultures like Swedish, they all should converted by XmlConvert.ToString(...) which uses Invariant culture.

Author: buyaa-n
Assignees: -
Labels:

area-System.Xml

Milestone: -

{
newCulture = new CultureInfo("SE-SW");
}
catch (CultureNotFoundException) { /* Do nothing */ }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should it return in the catch block? ie on a machine where for some reason does not have this culture, you want it to silently continue and use the current culture instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, i want it continue even the culture not found, because i don't see much test in this class, so still is not useless, plus the issue could happen for other culture too, wherever the test run its worth testing it

@buyaa-n
Copy link
Contributor Author

buyaa-n commented Jan 7, 2021

@danmosemsft @jkotas please let me know if you have any other comments/concerns

@danmoseley
Copy link
Member

ltgm

@buyaa-n buyaa-n merged commit 2a39be0 into dotnet:master Jan 8, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Feb 7, 2021
@buyaa-n buyaa-n deleted the xcontainer_number-sign branch April 25, 2021 20:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The current thread culture affects XAttribute value serialization.

3 participants