[PT Run] Calculator plugin: Various improvements (#18159)

* crash fixes and error result

* small changes and test fixes

* improve exceptions and message

* double array crash fix

* overflowexception

* improve error handling

* varous improvements

* varous improvements

* fix spelling

* fix spelling

* Revert #16980

* add description

* error improvemenet

* Update tests

* spelling fixes

* small changes

* add settings

* last changes

* fix description

* update dev docs

* spell check

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/CalculateEngine.cs

@@ -25,8 +25,10 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
         /// Interpret
         /// </summary>
         /// <param name="cultureInfo">Use CultureInfo.CurrentCulture if something is user facing</param>
-        public CalculateResult Interpret(string input, CultureInfo cultureInfo)
+        public CalculateResult Interpret(string input, CultureInfo cultureInfo, out string error)
         {
+            error = default;
+
             if (!CalculateHelper.InputValid(input))
             {
                 return default;
@@ -43,10 +45,16 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
             // This could happen for some incorrect queries, like pi(2)
             if (result == null)
             {
+                error = Properties.Resources.wox_plugin_calculator_expression_not_complete;
                 return default;
             }
 
             result = TransformResult(result);
+            if (result is string)
+            {
+                error = result as string;
+                return default;
+            }
 
             if (string.IsNullOrEmpty(result?.ToString()))
             {
@@ -68,7 +76,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
             return Math.Round(value, RoundingDigits, MidpointRounding.AwayFromZero);
         }
 
-        private static object TransformResult(object result)
+        private static dynamic TransformResult(object result)
         {
             if (result.ToString() == "NaN")
             {
@@ -80,6 +88,12 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
                 return Properties.Resources.wox_plugin_calculator_expression_not_complete;
             }
 
+            if (result is double[,])
+            {
+                // '[10,10]' is interpreted as array by mages engine
+                return Properties.Resources.wox_plugin_calculator_double_array_returned;
+            }
+
             return result;
         }
     }

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/CalculateHelper.cs

@@ -28,12 +28,6 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
                 throw new ArgumentNullException(paramName: nameof(input));
             }
 
-            bool singleDigitFactorial = input.EndsWith("!", StringComparison.InvariantCulture);
-            if (input.Length <= 2 && !singleDigitFactorial)
-            {
-                return false;
-            }
-
             if (!RegValidExpressChar.IsMatch(input))
             {
                 return false;

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/ErrorHandler.cs

@@ -0,0 +1,57 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System;
+using System.Collections.Generic;
+using Wox.Plugin;
+using Wox.Plugin.Logger;
+
+namespace Microsoft.PowerToys.Run.Plugin.Calculator
+{
+    internal static class ErrorHandler
+    {
+        /// <summary>
+        /// Method to handles errors while calculating
+        /// </summary>
+        /// <param name="icon">Path to result icon.</param>
+        /// <param name="isGlobalQuery">Bool to indicate if it is a global query.</param>
+        /// <param name="queryInput">User input as string including the action keyword.</param>
+        /// <param name="errorMessage">Error message if applicable.</param>
+        /// <param name="exception">Exception if applicable.</param>
+        /// <returns>List of results to show. Either an error message or an empty list.</returns>
+        /// <exception cref="ArgumentException">Thrown if <paramref name="errorMessage"/> and <paramref name="exception"/> are both filled with their default values.</exception>
+        internal static List<Result> OnError(string icon, bool isGlobalQuery, string queryInput, string errorMessage, Exception exception = default)
+        {
+            string userMessage;
+
+            if (errorMessage != default)
+            {
+                Log.Error($"Failed to calculate <{queryInput}>: {errorMessage}", typeof(Calculator.Main));
+                userMessage = errorMessage;
+            }
+            else if (exception != default)
+            {
+                Log.Exception($"Exception when query for <{queryInput}>", exception, exception.GetType());
+                userMessage = exception.Message;
+            }
+            else
+            {
+                throw new ArgumentException("The arguments error and exception have default values. One of them has to be filled with valid error data (error message/exception)!");
+            }
+
+            return isGlobalQuery ? new List<Result>() : new List<Result> { CreateErrorResult(userMessage, icon) };
+        }
+
+        private static Result CreateErrorResult(string errorMessage, string iconPath)
+        {
+            return new Result
+            {
+                Title = Properties.Resources.wox_plugin_calculator_calculation_failed,
+                SubTitle = errorMessage,
+                IcoPath = iconPath,
+                Score = 300,
+            };
+        }
+    }
+}

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/Main.cs

@@ -5,14 +5,17 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
+using System.Linq;
 using System.Text;
+using System.Windows.Controls;
 using ManagedCommon;
+using Microsoft.PowerToys.Run.Plugin.Calculator.Properties;
+using Microsoft.PowerToys.Settings.UI.Library;
 using Wox.Plugin;
-using Wox.Plugin.Logger;
 
 namespace Microsoft.PowerToys.Run.Plugin.Calculator
 {
-    public class Main : IPlugin, IPluginI18n, IDisposable
+    public class Main : IPlugin, IPluginI18n, IDisposable, ISettingProvider
     {
         private static readonly CalculateEngine CalculateEngine = new CalculateEngine();
 
@@ -20,20 +23,51 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
 
         private string IconPath { get; set; }
 
-        public string Name => Properties.Resources.wox_plugin_calculator_plugin_name;
+        private bool _inputUseEnglishFormat;
+        private bool _outputUseEnglishFormat;
 
-        public string Description => Properties.Resources.wox_plugin_calculator_plugin_description;
+        public string Name => Resources.wox_plugin_calculator_plugin_name;
+
+        public string Description => Resources.wox_plugin_calculator_plugin_description;
 
         private bool _disposed;
 
+        public IEnumerable<PluginAdditionalOption> AdditionalOptions => new List<PluginAdditionalOption>()
+        {
+            new PluginAdditionalOption()
+            {
+                Key = "InputUseEnglishFormat",
+                DisplayLabel = Resources.wox_plugin_calculator_in_en_format,
+                DisplayDescription = Resources.wox_plugin_calculator_in_en_format_description,
+                Value = false,
+            },
+            new PluginAdditionalOption()
+            {
+                Key = "OutputUseEnglishFormat",
+                DisplayLabel = Resources.wox_plugin_calculator_out_en_format,
+                DisplayDescription = Resources.wox_plugin_calculator_out_en_format_description,
+                Value = false,
+            },
+        };
+
         public List<Result> Query(Query query)
         {
+            bool isGlobalQuery = string.IsNullOrEmpty(query.ActionKeyword);
+            CultureInfo inputCulture = _inputUseEnglishFormat ? new CultureInfo("en-us") : CultureInfo.CurrentCulture;
+            CultureInfo outputCulture = _outputUseEnglishFormat ? new CultureInfo("en-us") : CultureInfo.CurrentCulture;
+
             if (query == null)
             {
                 throw new ArgumentNullException(paramName: nameof(query));
             }
 
-            NumberTranslator translator = NumberTranslator.Create(CultureInfo.CurrentCulture, new CultureInfo("en-US"));
+            // Happens if the user has only typed the action key so far
+            if (string.IsNullOrEmpty(query.Search))
+            {
+                return new List<Result>();
+            }
+
+            NumberTranslator translator = NumberTranslator.Create(inputCulture, new CultureInfo("en-US"));
             var input = translator.Translate(query.Search.Normalize(NormalizationForm.FormKC));
 
             if (!CalculateHelper.InputValid(input))
@@ -44,27 +78,38 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
             try
             {
                 // Using CurrentUICulture since this is user facing
-                var result = CalculateEngine.Interpret(input, CultureInfo.CurrentUICulture);
+                var result = CalculateEngine.Interpret(input, outputCulture, out string errorMessage);
 
                 // This could happen for some incorrect queries, like pi(2)
                 if (result.Equals(default(CalculateResult)))
                 {
-                    return new List<Result>();
+                    // If errorMessage is not default then do error handling
+                    return errorMessage == default ? new List<Result>() : ErrorHandler.OnError(IconPath, isGlobalQuery, query.RawQuery, errorMessage);
                 }
 
                 return new List<Result>
                 {
-                    ResultHelper.CreateResult(result.RoundedResult, IconPath),
+                    ResultHelper.CreateResult(result.RoundedResult, IconPath, outputCulture),
                 };
-            } // We want to keep the process alive if any the mages library throws any exceptions.
+            }
+            catch (Mages.Core.ParseException)
+            {
+                // Invalid input
+                return ErrorHandler.OnError(IconPath, isGlobalQuery, query.RawQuery, Properties.Resources.wox_plugin_calculator_expression_not_complete);
+            }
+            catch (OverflowException)
+            {
+                // Result to big to convert to decimal
+                return ErrorHandler.OnError(IconPath, isGlobalQuery, query.RawQuery, Properties.Resources.wox_plugin_calculator_not_covert_to_decimal);
+            }
 #pragma warning disable CA1031 // Do not catch general exception types
             catch (Exception e)
 #pragma warning restore CA1031 // Do not catch general exception types
             {
-                Log.Exception("Exception when query for <{query}>", e, GetType());
+                // Any other crash occurred
+                // We want to keep the process alive if any the mages library throws any exceptions.
+                return ErrorHandler.OnError(IconPath, isGlobalQuery, query.RawQuery, default, e);
             }
-
-            return new List<Result>();
         }
 
         public void Init(PluginInitContext context)
@@ -94,12 +139,35 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
 
         public string GetTranslatedPluginTitle()
         {
-            return Properties.Resources.wox_plugin_calculator_plugin_name;
+            return Resources.wox_plugin_calculator_plugin_name;
         }
 
         public string GetTranslatedPluginDescription()
         {
-            return Properties.Resources.wox_plugin_calculator_plugin_description;
+            return Resources.wox_plugin_calculator_plugin_description;
+        }
+
+        public Control CreateSettingPanel()
+        {
+            throw new NotImplementedException();
+        }
+
+        public void UpdateSettings(PowerLauncherPluginSettings settings)
+        {
+            var inputUseEnglishFormat = false;
+            var outputUseEnglishFormat = false;
+
+            if (settings != null && settings.AdditionalOptions != null)
+            {
+                var optionInputEn = settings.AdditionalOptions.FirstOrDefault(x => x.Key == "InputUseEnglishFormat");
+                inputUseEnglishFormat = optionInputEn?.Value ?? inputUseEnglishFormat;
+
+                var optionOutputEn = settings.AdditionalOptions.FirstOrDefault(x => x.Key == "OutputUseEnglishFormat");
+                outputUseEnglishFormat = optionOutputEn?.Value ?? outputUseEnglishFormat;
+            }
+
+            _inputUseEnglishFormat = inputUseEnglishFormat;
+            _outputUseEnglishFormat = outputUseEnglishFormat;
         }
 
         public void Dispose()

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/NumberTranslator.cs

@@ -88,14 +88,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
 
         private static Regex GetSplitRegex(CultureInfo culture)
         {
-            // HACK: Specifically adding the decimal point here since some people expect that to work everywhere.
-            // This allows avoiding some unexpected errors users are getting when . is not part of the number representation.
-            // Users were getting errors where leading zeros were being removed from the decimal part of numbers like 56.0002.
-            // 56.0002 would be transformed into 56.2 due to it being translated as two different numbers and this would be accepted into the engine.
-            // Now, even if . is not part of the culture representation, users won't hit this error since the number will
-            // be passed as is to the calculator engine.
-            // This shouldn't add any regressions into accepted strings while it will have a behavior the users expect.
-            var splitPattern = $"((?:\\d|[a-fA-F]|\\.|{Regex.Escape(culture.NumberFormat.NumberDecimalSeparator)}";
+            var splitPattern = $"((?:\\d|[a-fA-F]|{Regex.Escape(culture.NumberFormat.NumberDecimalSeparator)}";
             if (!string.IsNullOrEmpty(culture.NumberFormat.NumberGroupSeparator))
             {
                 splitPattern += $"|{Regex.Escape(culture.NumberFormat.NumberGroupSeparator)}";

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/Properties/Resources.Designer.cs

@@ -19,7 +19,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
     // class via a tool like ResGen or Visual Studio.
     // To add or remove a member, edit your .ResX file then rerun ResGen
     // with the /str option, or rebuild your VS project.
-    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "16.0.0.0")]
+    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")]
     [global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
     [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()]
     public class Resources {
@@ -45,7 +45,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
                 return resourceMan;
             }
         }
-
+        
         /// <summary>
         ///   Overrides the current thread's CurrentUICulture property for all
         ///   resource lookups using this strongly typed resource class.
@@ -60,6 +60,15 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Failed to calculate the input.
+        /// </summary>
+        public static string wox_plugin_calculator_calculation_failed {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_calculation_failed", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Copy failed, please try later.
         /// </summary>
@@ -68,7 +77,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
                 return ResourceManager.GetString("wox_plugin_calculator_copy_failed", resourceCulture);
             }
         }
-
+        
         /// <summary>
         ///   Looks up a localized string similar to Copy this number to the clipboard.
         /// </summary>
@@ -78,6 +87,15 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Unsupported use of square brackets.
+        /// </summary>
+        public static string wox_plugin_calculator_double_array_returned {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_double_array_returned", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Expression wrong or incomplete (Did you forget some parentheses?).
         /// </summary>
@@ -88,14 +106,58 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
         }
         
         /// <summary>
-        ///   Looks up a localized string similar to Not a number (NaN).
+        ///   Looks up a localized string similar to Use English (United States) number format for input.
+        /// </summary>
+        public static string wox_plugin_calculator_in_en_format {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_in_en_format", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Ignores your system setting and expects numbers in the format &apos;1,000.50&apos;.
+        /// </summary>
+        public static string wox_plugin_calculator_in_en_format_description {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_in_en_format_description", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Calculation result is not a valid number (NaN).
         /// </summary>
         public static string wox_plugin_calculator_not_a_number {
             get {
                 return ResourceManager.GetString("wox_plugin_calculator_not_a_number", resourceCulture);
             }
         }
-
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Result value was either too large or too small for a decimal number.
+        /// </summary>
+        public static string wox_plugin_calculator_not_covert_to_decimal {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_not_covert_to_decimal", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Use English (United States) number format for output.
+        /// </summary>
+        public static string wox_plugin_calculator_out_en_format {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_out_en_format", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to Ignores your system setting and returns numbers in the format &apos;1000.50&apos;.
+        /// </summary>
+        public static string wox_plugin_calculator_out_en_format_description {
+            get {
+                return ResourceManager.GetString("wox_plugin_calculator_out_en_format_description", resourceCulture);
+            }
+        }
         
         /// <summary>
         ///   Looks up a localized string similar to Does mathematical calculations (e.g. 5*3-2)..
@@ -105,7 +167,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator.Properties {
                 return ResourceManager.GetString("wox_plugin_calculator_plugin_description", resourceCulture);
             }
         }
-
+        
         /// <summary>
         ///   Looks up a localized string similar to Calculator.
         /// </summary>

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/Properties/Resources.resx

@@ -124,7 +124,7 @@
     <value>Does mathematical calculations (e.g. 5*3-2).</value>
   </data>
   <data name="wox_plugin_calculator_not_a_number" xml:space="preserve">
-    <value>Not a number (NaN)</value>
+    <value>Calculation result is not a valid number (NaN)</value>
   </data>
   <data name="wox_plugin_calculator_expression_not_complete" xml:space="preserve">
     <value>Expression wrong or incomplete (Did you forget some parentheses?)</value>
@@ -135,4 +135,25 @@
   <data name="wox_plugin_calculator_copy_failed" xml:space="preserve">
     <value>Copy failed, please try later</value>
   </data>
+  <data name="wox_plugin_calculator_calculation_failed" xml:space="preserve">
+    <value>Failed to calculate the input</value>
+  </data>
+  <data name="wox_plugin_calculator_double_array_returned" xml:space="preserve">
+    <value>Unsupported use of square brackets</value>
+  </data>
+  <data name="wox_plugin_calculator_not_covert_to_decimal" xml:space="preserve">
+    <value>Result value was either too large or too small for a decimal number</value>
+  </data>
+  <data name="wox_plugin_calculator_in_en_format" xml:space="preserve">
+    <value>Use English (United States) number format for input</value>
+  </data>
+  <data name="wox_plugin_calculator_in_en_format_description" xml:space="preserve">
+    <value>Ignores your system setting and expects numbers in the format '1,000.50'</value>
+  </data>
+  <data name="wox_plugin_calculator_out_en_format" xml:space="preserve">
+    <value>Use English (United States) number format for output</value>
+  </data>
+  <data name="wox_plugin_calculator_out_en_format_description" xml:space="preserve">
+    <value>Ignores your system setting and returns numbers in the format '1000.50'</value>
+  </data>
 </root>

src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/ResultHelper.cs

@@ -12,12 +12,12 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
 {
     public static class ResultHelper
     {
-        public static Result CreateResult(CalculateResult result, string iconPath)
+        public static Result CreateResult(CalculateResult result, string iconPath, CultureInfo culture)
         {
-            return CreateResult(result.RoundedResult, iconPath);
+            return CreateResult(result.RoundedResult, iconPath, culture);
         }
 
-        public static Result CreateResult(decimal? roundedResult, string iconPath)
+        public static Result CreateResult(decimal? roundedResult, string iconPath, CultureInfo culture)
         {
             // Return null when the expression is not a valid calculator query.
             if (roundedResult == null)
@@ -28,15 +28,15 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
             return new Result
             {
                 // Using CurrentCulture since this is user facing
-                Title = roundedResult?.ToString(CultureInfo.CurrentCulture),
+                Title = roundedResult?.ToString(culture),
                 IcoPath = iconPath,
                 Score = 300,
                 SubTitle = Properties.Resources.wox_plugin_calculator_copy_number_to_clipboard,
-                Action = c => Action(roundedResult),
+                Action = c => Action(roundedResult, culture),
             };
         }
 
-        public static bool Action(decimal? roundedResult)
+        public static bool Action(decimal? roundedResult, CultureInfo culture)
         {
             var ret = false;
 
@@ -46,7 +46,7 @@ namespace Microsoft.PowerToys.Run.Plugin.Calculator
                 {
                     try
                     {
-                        Clipboard.SetText(roundedResult?.ToString(CultureInfo.CurrentCulture));
+                        Clipboard.SetText(roundedResult?.ToString(culture));
                         ret = true;
                     }
                     catch (ExternalException)