use invariant culture to parse inputs to converters - #10006
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
we were passing the culture before but this was usually set to en-us EVEN in localized enviornments unless something set it explicitly wrap with try catch and return some default conversion
mjkkirschner
changed the title
QilongTang
reviewed
Sep 25, 2019
src/DynamoCoreWpf/UI/Converters.cs
Outdated
| @@ -1414,15 +1414,29 @@ public object ConvertBack(object value, Type targetType, object parameter, Syste | |||
| } | |||
| } | |||
|
|
|||
| //TODO remove(this is not used anywhere) | |||
There was a problem hiding this comment.
Can you add Dynamo 3.0? Will be easier to identify when the time comes :)
QilongTang
reviewed
Sep 25, 2019
QilongTang
approved these changes
Sep 25, 2019
mjkkirschner
commented
Sep 25, 2019
mjkkirschner
commented
Sep 25, 2019
| @@ -1414,15 +1414,29 @@ public object ConvertBack(object value, Type targetType, object parameter, Syste | |||
| } | |||
| } | |||
|
|
|||
| //TODO remove(this is not used anywhere) in Dynamo 3.0 | |||
There was a problem hiding this comment.
@saintentropy this is the converter I was talking about .... is this the one you were thinking of? I can't find this particular one used anywhere.
mjkkirschner
added a commit
to mjkkirschner/Dynamo
that referenced
this pull request
Sep 25, 2019
* use invariant culture to parse inputs - we were passing the culture before but this was usually set to en-us EVEN in localized enviornments unless something set it explicitly wrap with try catch and return some default conversion * Update src/DynamoCoreWpf/UI/Converters.cs * Update src/DynamoCoreWpf/UI/Converters.cs (cherry picked from commit 3dc4202)
mjkkirschner
added a commit
that referenced
this pull request
Sep 25, 2019
* use invariant culture to parse inputs - we were passing the culture before but this was usually set to en-us EVEN in localized enviornments unless something set it explicitly wrap with try catch and return some default conversion * Update src/DynamoCoreWpf/UI/Converters.cs * Update src/DynamoCoreWpf/UI/Converters.cs (cherry picked from commit 3dc4202)
mjkkirschner
added a commit
that referenced
this pull request
Sep 30, 2019
* use invariant culture to parse inputs to converters - (#10006) * use invariant culture to parse inputs - we were passing the culture before but this was usually set to en-us EVEN in localized enviornments unless something set it explicitly wrap with try catch and return some default conversion * Update src/DynamoCoreWpf/UI/Converters.cs * Update src/DynamoCoreWpf/UI/Converters.cs (cherry picked from commit 3dc4202) * merge conflicts
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
we were passing the culture before but this was usually set to en-us EVEN in localized environments
unless something set it explicitly
wrap with try catch and return some default conversion
Purpose
This one has a bit of a history.
issue from user:
#6261
fix:
#6724
fix:
#9994
When this was fixed originally by @sm6srw it appears that unlike what it implies in his PR - unintentionally the culture might have always been set to
en-usas @angelowang points out WPF passes en-us culture to the converters EVEN if the environment is localized. (I tried both localizing dynamo and localizing windows and got en-us everytime!)SO - this means that the original fix three years ago, was close, but in certain situations it lets the host change the culture - we don't want that.
Our strings are hardcoded in en-us format in our xaml, and I think that is okay, these converters are not really for public consumption. SO this pr does the following.
I've added similar logic to two other converters that @sm6srw added culture to. It appears to me the only time strings are used on these converters is from hardcoded xaml values, or the tests.
I still need to run the self serve tests.
Declarations
Check these if you believe they are true
*.resxfilesReviewers
@QilongTang
@sm6srw
FYIs
@angelowang