Skip to content

parse numbers in NumericTextBox using CurrentCulture #1811

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

Schossi
Copy link
Contributor

@Schossi Schossi commented Sep 21, 2023

enables numpad usage for cultures that use comma for decimals

PR Details

Changes parsing and formatting culture in the NumericTextBox from InvariantCulture to CurrentCulture. Therefore decimal numbers in the property grid will be displayed using the current culture of the user and can be input using that same culture. Most importantly this enables culture settings like mine('en-AT') to use the numpad to input decimals with the comma key.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

enables numpad usage for cultures that use comma for decimals
@tebjan
Copy link
Member

tebjan commented Sep 21, 2023

Good initiative, but I certainly don't want to see any commas in my UI, but I live in a culture where comma is used. So the current culture should only be a fallback if the InvariantCulture parsing doesn't work...

@tebjan
Copy link
Member

tebjan commented Sep 21, 2023

Here is the number parsing code we use in vvvv:

    public static class ParseUtils
    {
        /// <summary>
        /// The VL allowed number style, its any but without thousands separator
        /// </summary>
        public const NumberStyles VLUserInputNumberStyle = NumberStyles.Any & ~NumberStyles.AllowThousands;
        static readonly CultureInfo CommaCulture = CultureInfo.GetCultureInfo("de-DE");

        public static bool TryParseValue<T>(string value, TryParseDelegate<T> tryMethod, out T result)
        {
            //try parsing a hex string
            var x = value.TrimStart('0').ToLower();
            if (x.StartsWith("x") || x.StartsWith("#"))
            {
                x = x.TrimStart('x', '#');
                if (tryMethod(x, NumberStyles.HexNumber, CultureInfo.CurrentCulture, out result))
                    return true;
            }
            //Try parsing in the current culture
            else if (tryMethod(value, VLUserInputNumberStyle, CultureInfo.CurrentCulture, out result) ||
                //or in neutral culture
                tryMethod(value, VLUserInputNumberStyle, CultureInfo.InvariantCulture, out result) ||
                //or as fallback a culture that has ',' as comma separator
                tryMethod(value, VLUserInputNumberStyle, CommaCulture, out result))
            {
                return true;
            }

            return false;
        }

        /// <summary>
        /// Tries to parse the string as float with the VL allowed Number style
        /// </summary>
        /// <param name="value"></param>
        /// <param name="result"></param>
        /// <returns></returns>
        public static bool TryParseFloat(string value, out float result)
        {
            return TryParseValue(value, float.TryParse, out result);
        }

        /// <summary>
        /// Tries to parse the string as integer with the VL allowed number style
        /// </summary>
        /// <param name="value"></param>
        /// <param name="result"></param>
        /// <returns></returns>
        public static bool TryParseInt(string value, out int result)
        {
            return TryParseValue(value, int.TryParse, out result);
        }
        ...
}

enables numpad usage for cultures that use comma for decimals
@Schossi
Copy link
Contributor Author

Schossi commented Sep 21, 2023

makes sense, i've adopted your code

i think unity works the same way, blender actually replaces the comma while typing which might be the best way to do this but might cause other issues as theres no really good hook for that in the wpf textbox afaik

@Eideren Eideren requested a review from tebjan September 24, 2023 12:33
@tebjan
Copy link
Member

tebjan commented Oct 16, 2023

Excellent, thanks!

@tebjan tebjan merged commit 241a1bd into stride3d:master Oct 16, 2023
if (x.StartsWith("x") || x.StartsWith("#"))
{
x = x.TrimStart('x', '#');
if (double.TryParse(x, NumberStyles.HexNumber, CultureInfo.CurrentCulture, out result))
Copy link
Member

@Kryptos-FR Kryptos-FR Oct 16, 2023

Choose a reason for hiding this comment

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

I don't think it's useful to use the culture for hexadecimal. To avoid confusion, I'd rather use NumberFormatInfo.InvariantInfo.

Suggested change
if (double.TryParse(x, NumberStyles.HexNumber, CultureInfo.CurrentCulture, out result))
if (double.TryParse(x, NumberStyles.HexNumber, NumberFormatInfo.InvariantInfo, out result))

Copy link
Member

@tebjan tebjan Oct 16, 2023

Choose a reason for hiding this comment

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

I thought about this too when i reviewed it, but I don't think that any culture has an influence on hex parsing. But your suggested change is more correct, indeed.

Copy link
Member

Choose a reason for hiding this comment

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

Should be fine. It probably doesn't change much.

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