Skip to content

Commit

Permalink
[PT Run] Calculator plugin: Various improvements (#18159)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
htcfreek authored Jun 2, 2022
1 parent 9e4a58e commit 465df35
Show file tree
Hide file tree
Showing 13 changed files with 363 additions and 56 deletions.
36 changes: 36 additions & 0 deletions doc/devdocs/modules/launcher/plugins/calculator.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,22 @@ The Calculator plugin as the name suggests is used to perform calculations on th

![Image of Calculator plugin](/doc/images/launcher/plugins/calculator.png)

## Optional plugin settings

* We have the following settings that the user can configure to change the behavior of the plugin:

| Key | Default value | Name | Description |
|--------------|-----------|------------|------------|
| `InputUseEnglishFormat` | `false` | Use English (United States) number format for input | Ignores your system setting and expects numbers in the format '1,000.50' |
| `OutputUseEnglishFormat` | `false` | Use English (United States) number format for output | Ignores your system setting and returns numbers in the format '1000.50' |

* The optional plugin settings are implemented via the [`ISettingProvider`](/src/modules/launcher/Wox.Plugin/ISettingProvider.cs) interface from `Wox.Plugin` project. All available settings for the plugin are defined in the [`Main`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/Main.cs) class of the plugin.

## Technical details

### [`BracketHelper`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/BracketHelper.cs)
- This helper validates the bracket usage in the input string.

### [`CalculateHelper`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/CalculateHelper.cs)
- The [`CalculateHelper.cs`](src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/CalculateHelper.cs) class checks to see if the user entered query is a valid input to the calculator and only if the input is valid does it perform the operation.
- It does so by matching the user query to a valid regex.
Expand All @@ -18,6 +34,26 @@ var result = CalculateEngine.Interpret(query.Search, CultureInfo.CurrentUICultur
- The class which encapsulates the result of the computation.
- It comprises of the `Result` and `RoundedResult` properties.

### [`ErrorHandler`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator/ErrorHandler.cs)
- The class which encapsulates the code to log errors and format the user message.
- It returns an error result if the user searches with the activation command. This error result is shown to the user.

### Score
The score of each result from the calculator plugin is `300`.


## [Unit Tests](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests)
We have a [Unit Test project](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests) that executes various test to ensure that the plugin works as expected.

### [`BracketHelperTests`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/BracketHelperTests.cs)
- The [`BracketHelperTests.cs`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/BracketHelperTests.cs) class contains tests to validate that brackets are handled correctly.

### [`ExtendedCalculatorParserTests`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/ExtendedCalculatorParserTests.cs)
- The [`ExtendedCalculatorParserTests.cs`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/ExtendedCalculatorParserTests.cs) class contains tests to validate that the input is parsed correctly and the result is correct.

### [`NumberTranslatorTests`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/NumberTranslatorTests.cs)
- The [`NumberTranslatorTests.cs`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/NumberTranslatorTests.cs) class contains tests to validate that each number is converted correctly based on the defined locals.

### [`QueryTests`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/QueryTests.cs)
- The [`QueryTests.cs`](/src/modules/launcher/Plugins/Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests/QueryTests.cs) class contains tests to validate that the user gets the correct results when searching.

Original file line number Diff line number Diff line change
Expand Up @@ -32,21 +32,21 @@ public void Interpret_ThrowError_WhenCalledNullOrEmpty(string input)
var engine = new CalculateEngine();

// Act
Assert.ThrowsException<ArgumentNullException>(() => engine.Interpret(input, CultureInfo.CurrentCulture));
Assert.ThrowsException<ArgumentNullException>(() => engine.Interpret(input, CultureInfo.CurrentCulture, out _));
}

[DataTestMethod]
[DataRow("42")]
[DataRow("test")]
[DataRow("pi(2)")] // Incorrect input, constant is being treated as a function.
[DataRow("e(2)")]
[DataRow("[10,10]")] // '[10,10]' is interpreted as array by mages engine
public void Interpret_NoResult_WhenCalled(string input)
{
// Arrange
var engine = new CalculateEngine();

// Act
var result = engine.Interpret(input, CultureInfo.CurrentCulture);
var result = engine.Interpret(input, CultureInfo.CurrentCulture, out _);

// Assert
Assert.AreEqual(default(CalculateResult), result);
Expand Down Expand Up @@ -87,7 +87,7 @@ public void Interpret_NoErrors_WhenCalledWithRounding(string input, decimal expe

// Act
// Using InvariantCulture since this is internal
var result = engine.Interpret(input, CultureInfo.InvariantCulture);
var result = engine.Interpret(input, CultureInfo.InvariantCulture, out _);

// Assert
Assert.IsNotNull(result);
Expand All @@ -111,7 +111,7 @@ public void Interpret_QuirkOutput_WhenCalled(string input, decimal expectedResul

// Act
// Using InvariantCulture since this is internal
var result = engine.Interpret(input, CultureInfo.InvariantCulture);
var result = engine.Interpret(input, CultureInfo.InvariantCulture, out _);

// Assert
Assert.IsNotNull(result);
Expand All @@ -135,7 +135,7 @@ public void Interpret_DifferentCulture_WhenCalled(string input, decimal expected
var engine = new CalculateEngine();

// Act
var result = engine.Interpret(input, cultureInfo);
var result = engine.Interpret(input, cultureInfo, out _);

// Assert
Assert.IsNotNull(result);
Expand Down Expand Up @@ -184,7 +184,7 @@ public void Interpret_MustReturnResult_WhenResultIsZero(string input)

// Act
// Using InvariantCulture since this is internal
var result = engine.Interpret(input, CultureInfo.InvariantCulture);
var result = engine.Interpret(input, CultureInfo.InvariantCulture, out _);

// Assert
Assert.IsNotNull(result);
Expand All @@ -210,7 +210,7 @@ public void Interpret_MustReturnExpectedResult_WhenCalled(string input, decimal

// Act
// Using InvariantCulture since this is internal
var result = engine.Interpret(input, CultureInfo.InvariantCulture);
var result = engine.Interpret(input, CultureInfo.InvariantCulture, out _);

// Assert
Assert.IsNotNull(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Moq" Version="4.16.1" />
<PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="6.0.0">
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
<PrivateAssets>all</PrivateAssets>
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
</PackageReference>
<PackageReference Include="MSTest.TestAdapter" Version="2.2.3" />
<PackageReference Include="MSTest.TestFramework" Version="2.2.3" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,12 +132,12 @@ public void Translate_RemoveNumberGroupSeparator_WhenCalled(string decimalSepara
}

[DataTestMethod]
[DataRow("12.0004", "12.0004")]
[DataRow("12,0004", "12.0004")]
[DataRow("0xF000", "0xF000")]
public void Translate_NoRemovalOfLeadingZeroesOnEdgeCases(string input, string expectedResult)
{
// Arrange
var translator = NumberTranslator.Create(new CultureInfo("pt-PT"), new CultureInfo("en-US"));
var translator = NumberTranslator.Create(new CultureInfo("de-de"), new CultureInfo("en-US"));

// Act
var result = translator.Translate(input);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
// 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.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Moq;
using Wox.Plugin;

namespace Microsoft.PowerToys.Run.Plugin.Calculator.UnitTests
{
[TestClass]
public class QueryTests
{
[DataTestMethod]
[DataRow("=pi(9+)", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=pi(9)", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=pi,", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=log()", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=0xf0x6", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=0xf,0x6", "Expression wrong or incomplete (Did you forget some parentheses?)")]
[DataRow("=2^96", "Result value was either too large or too small for a decimal number")]
[DataRow("=+()", "Calculation result is not a valid number (NaN)")]
[DataRow("=[10,10]", "Unsupported use of square brackets")]
public void ErrorResultOnInvalidKeywordQuery(string typedString, string expectedResult)
{
// Setup
Mock<Main> main = new ();
Query expectedQuery = new (typedString, "=");

// Act
var result = main.Object.Query(expectedQuery).FirstOrDefault().SubTitle;

// Assert
Assert.AreEqual(expectedResult, result);
}

[DataTestMethod]
[DataRow("pi(9+)")]
[DataRow("pi(9)")]
[DataRow("pi,")]
[DataRow("log()")]
[DataRow("0xf0x6")]
[DataRow("0xf,0x6")]
[DataRow("2^96")]
[DataRow("+()")]
[DataRow("[10,10]")]
public void NoResultOnInvalidGlobalQuery(string typedString)
{
// Setup
Mock<Main> main = new ();
Query expectedQuery = new (typedString);

// Act
var result = main.Object.Query(expectedQuery).Count;

// Assert
Assert.AreEqual(result, 0);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,10 @@ public class CalculateEngine
/// 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;
Expand All @@ -43,10 +45,16 @@ public CalculateResult Interpret(string input, CultureInfo cultureInfo)
// 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()))
{
Expand All @@ -68,7 +76,7 @@ public static decimal Round(decimal value)
return Math.Round(value, RoundingDigits, MidpointRounding.AwayFromZero);
}

private static object TransformResult(object result)
private static dynamic TransformResult(object result)
{
if (result.ToString() == "NaN")
{
Expand All @@ -80,6 +88,12 @@ private static object TransformResult(object result)
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;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,6 @@ public static bool InputValid(string input)
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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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,
};
}
}
}
Loading

0 comments on commit 465df35

Please sign in to comment.