Smart thousands and decimals for Calculator #3859
Conversation
## Core Logic * **Advanced Number Parsing:** We now process numbers with various decimal and thousand-separator formats (e.g., `1,234.56` and `1.234,56`). We distinguish between separator types based on their position and surrounding digits. * **Context-Aware Output Formatting:** We now mirror the output format based on the user's input. If a query includes thousand separators, the result will also have them. The decimal separator in the result will match the one used in the query. ## Code Cleanup * **Deleted Unused File:** `NumberTranslator.cs` was unused and therefore removed. * **Removed Redundant UI Code:** The `CalculatorSettings_Loaded` event handler in `CalculatorSettings.xaml.cs` (and its XAML registration) was removed. The functionality was already handled automatically by data binding. ## Maintainability * **Added Code Documentation:** An XML summary comment was added to the new `NormalizeNumber` method in `Main.cs` to clarify its purpose. So, the plugin is now much more flexible, and will accept whatever format the user gives it regardless of Windows region settings.
|
🥷 Code experts: jjw24, Jack251970 jjw24, Jack251970 have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: ✨ Comment |
|
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
📝 Walkthrough## Walkthrough
The changes introduce advanced, locale-aware number parsing and formatting within the Calculator plugin by replacing the previous culture conversion logic with a new, thread-safe approach. A per-query parsing context, new normalization and formatting methods, and updated regex utilities are implemented. The legacy `NumberTranslator` class is removed, and plugin metadata and localization strings are updated.
## Changes
| File(s) | Change Summary |
|---------------------------------------------------------------------------------------------|------------------------------------------------------------------------------------------------------------------|
| Plugins/Flow.Launcher.Plugin.Calculator/Main.cs | Added `ParsingContext` class, implemented number normalization and formatting methods, updated query logic and validation. |
| Plugins/Flow.Launcher.Plugin.Calculator/MainRegexHelper.cs | Added new compiled regex methods for matching numbers and thousand groups. |
| Plugins/Flow.Launcher.Plugin.Calculator/NumberTranslator.cs | Deleted entire `NumberTranslator` class and related methods. |
| Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml.cs | Removed unused `using System.Windows;` directive. |
| Plugins/Flow.Launcher.Plugin.Calculator/plugin.json | Updated plugin description and author metadata; no code changes. |
| Plugins/Flow.Launcher.Plugin.Calculator/Languages/en.xaml | Updated plugin description localization string to clarify number formatting capabilities. |
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant Main
participant ParsingContext
participant MainRegexHelper
User->>Main: Query(input)
Main->>ParsingContext: Create instance
Main->>MainRegexHelper: GetNumberRegex()
Main->>Main: For each matched number, call NormalizeNumber
Main->>ParsingContext: Update decimal/group separator info
Main->>Main: Evaluate normalized expression
Main->>Main: Format result via FormatResult using ParsingContext
Main->>User: Return formatted resultEstimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested reviewers
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
123-205: Well-implemented number normalization logic.The structural analysis approach for detecting separators is clever and handles most real-world scenarios effectively. The fallback to system culture for ambiguous cases is appropriate.
Consider documenting edge cases like multiple consecutive separators (e.g., "1,,234") in the XML comment, as the current regex would capture these but they would likely fail in the engine.
109-112: Consider more specific error handling.The catch-all exception handler could mask important errors. Consider logging exceptions or handling specific exception types.
-catch (Exception) +catch (Exception ex) { - // ignored + // Log the exception for debugging + Context.API.LogException(nameof(Calculator), "Failed to calculate expression", ex); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(6 hunks)Plugins/Flow.Launcher.Plugin.Calculator/NumberTranslator.cs(0 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml(0 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml.cs(0 hunks)Plugins/Flow.Launcher.Plugin.Calculator/plugin.json(1 hunks)
🧠 Learnings (3)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/plugin.json (1)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (3)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Calculator/plugin.json
[warning] 6-6:
cxfksword is not a recognized word. (unrecognized-spelling)
💤 Files with no reviewable changes (3)
- Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml.cs
- Plugins/Flow.Launcher.Plugin.Calculator/NumberTranslator.cs
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/plugin.json (1)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (3)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Calculator/plugin.json
[warning] 6-6:
cxfksword is not a recognized word. (unrecognized-spelling)
🔇 Additional comments (2)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
207-230: Clean and effective result formatting.The method correctly formats the output to match the user's input style, maintaining consistency in the user experience.
Plugins/Flow.Launcher.Plugin.Calculator/plugin.json (1)
5-7: Metadata updates accurately reflect new functionality.The description clearly communicates the enhanced number format support, and the version bump appropriately indicates new features.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
124-212: Well-designed number parsing with comprehensive format detection.The
NormalizeNumbermethod implements sophisticated logic for detecting and normalizing various international number formats. The structural analysis for identifying thousand separators and the culture-based fallback for ambiguous cases is well-thought-out.The method handles the key scenarios effectively:
- Mixed separators (e.g., "1,234.56" vs "1.234,56")
- Culture-aware disambiguation for patterns like "1,234"
- Proper normalization for the Mages engine
Consider adding input validation for edge cases like consecutive separators or malformed patterns, though the current implementation should handle typical user input well.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(6 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
🔇 Additional comments (5)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (5)
4-4: Good optimization of regex patterns.The addition of
System.Linqimport and the static compiled regex patterns address the performance concerns from previous reviews. TheThousandGroupRegexpattern correctly uses word boundaries and lookaheads for thousand separator insertion, andNumberRegexis now efficiently compiled once.Also applies to: 27-28
39-47: Excellent solution for thread safety.The
ParsingContextclass design effectively addresses the thread safety concerns raised in previous reviews by encapsulating query-specific state in a local instance rather than using instance fields.
71-71: Clean integration of the new parsing approach.The Query method effectively integrates the new parsing logic while maintaining thread safety and the existing API. The flow from normalization through calculation to formatting is well-structured.
Also applies to: 75-75, 88-88
214-237: Excellent result formatting that mirrors input format.The
FormatResultmethod correctly implements the requirement to format results using the same separators as detected in the input. The logic for swapping decimal and group separators based on the detected format is well-implemented.The fallback to
GetDecimalSeparator()when no input format is detected ensures consistent behavior with existing settings.
1-307: Outstanding refactor that addresses all previous concerns.This refactor successfully addresses the thread safety and performance issues raised in previous reviews while significantly enhancing the calculator's ability to handle international number formats. The code is well-structured, thoroughly documented, and maintains backward compatibility.
Key improvements:
- Thread safety through local
ParsingContextinstances- Optimized static regex patterns
- Comprehensive number format detection
- Smart result formatting that mirrors input format
- Enhanced support for various decimal and thousand separator combinations
| "Description": "Perform mathematical calculations (including hexadecimal values)", | ||
| "Author": "cxfksword", | ||
| "Version": "1.0.0", | ||
| "Description": "Perform mathematical calculations (including hexadecimal values). Use ',' or '.' as thousand separator or decimal place.", |
There was a problem hiding this comment.
@jjw24 Not sure if this is needed here. Could you please take a look?
There was a problem hiding this comment.
I thought it was significant enough to make it clear to the user???
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
All functions work well for me |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
27-28: Good use of static compiled regexes for performance.The static regex declarations with
RegexOptions.Compiledproperly address the performance concerns from previous reviews. However, theNumberRegexpattern[\d\.,]+can still match invalid sequences like "..." or ",,," which could lead to unexpected behavior in number parsing.Consider refining the pattern to be more restrictive:
-private static readonly Regex NumberRegex = new Regex(@"[\d\.,]+", RegexOptions.Compiled); +private static readonly Regex NumberRegex = new Regex(@"\d+(?:[.,]\d+)*", RegexOptions.Compiled);
124-212: Comprehensive number parsing with good structural analysis.The
NormalizeNumbermethod implements sophisticated logic to distinguish between decimal and group separators using structural analysis and cultural fallbacks. The approach of using the last separator when both are present and checking for 3-digit groups is sound.Consider extracting the group separator detection logic into a separate method for better readability:
+private bool IsGroupSeparatorCandidate(string[] parts) +{ + return parts.Length > 1 && parts.Skip(1).All(p => p.Length == 3); +}Then use it in both comma and dot handling sections to reduce duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(6 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: taooceros
PR: Flow-Launcher/Flow.Launcher#2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the `Flow.Launcher.csproj` file is dynamically updated during the CI/CD process.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
🔇 Additional comments (3)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (3)
39-47: Excellent solution for thread safety concerns.The
ParsingContextclass effectively addresses the thread safety issues mentioned in previous reviews by localizing the formatting state per query instead of using instance fields. This design ensures that concurrent queries won't interfere with each other's number formatting context.
71-71: Well-implemented thread-safe design.The query method properly implements the thread-safe approach by creating a local
ParsingContextinstance and passing it to the relevant methods. This eliminates the race condition issues that existed with instance fields.Also applies to: 75-75, 88-88
214-237: Clean and effective result formatting logic.The
FormatResultmethod properly mirrors the user's input formatting by applying the detected decimal separator and group separators. The logic for determining the group separator as the opposite of the decimal separator is simple and effective.
…b.com/dcog989/Flow.Launcher into Calculator-accepts-flexible-separator
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
Flow.Launcher.Core/packages.lock.json(8 hunks)Flow.Launcher.Infrastructure/packages.lock.json(7 hunks)Flow.Launcher.Plugin/packages.lock.json(1 hunks)Flow.Launcher/packages.lock.json(15 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(6 hunks)
🧠 Learnings (4)
Flow.Launcher.Infrastructure/packages.lock.json (4)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Flow.Launcher.Core/packages.lock.json (2)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
Flow.Launcher/packages.lock.json (3)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
✅ Files skipped from review due to trivial changes (1)
- Flow.Launcher.Plugin/packages.lock.json
🧰 Additional context used
🧠 Learnings (4)
Flow.Launcher.Infrastructure/packages.lock.json (4)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Flow.Launcher.Core/packages.lock.json (2)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
Flow.Launcher/packages.lock.json (3)
Learnt from: taooceros
PR: #2616
File: Flow.Launcher/Flow.Launcher.csproj:7-7
Timestamp: 2024-10-08T15:52:58.573Z
Learning: In the Flow Launcher project, the version number in the Flow.Launcher.csproj file is dynamically updated during the CI/CD process.
Learnt from: Jack251970
PR: #3572
File: Flow.Launcher/App.xaml.cs:214-216
Timestamp: 2025-07-06T12:21:37.947Z
Learning: In Flow Launcher, the UpdatePluginManifestAsync method in PluginsManifest.cs already has comprehensive internal try-catch handling that logs exceptions and returns false on failure rather than throwing, making external try-catch wrappers unnecessary.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
🔇 Additional comments (7)
Flow.Launcher.Infrastructure/packages.lock.json (1)
32-37: LGTM! Package dependency updates look consistent.The dependency updates are well-coordinated across the project, adding new functionality with
InputSimulator,NHotkey.Wpf, andSharpVectors.Wpfwhile upgradingMemoryPackto the latest patch version.The downgrades of
System.Drawing.CommonandMicrosoft.Win32.SystemEventsfrom 9.0.2 to 7.0.0 appear intentional for compatibility alignment across the project.Also applies to: 69-77, 93-98, 99-107, 162-166
Flow.Launcher.Core/packages.lock.json (1)
14-18: LGTM! Core package updates are consistent and appropriate.The dependency updates maintain consistency with the Infrastructure project while adding new capabilities:
SemanticVersioningaddition suggests enhanced version managementStreamJsonRpcupgrade to 2.21.10 brings latest fixes and improvementsFSharp.Coreupgrade to 9.0.201 keeps F# components currentThe transitive dependencies align perfectly with the Infrastructure changes.
Also applies to: 32-37, 49-53, 87-91, 97-105, 179-191, 197-201
Flow.Launcher/packages.lock.json (1)
17-22: LGTM! Comprehensive UI enhancement package additions.This update introduces significant new capabilities:
- MdXaml packages (1.27.0): Adds comprehensive markdown rendering with support for animations, HTML, plugins, and SVG
- TaskScheduler (2.12.1): Enables advanced task scheduling functionality
- Fody upgrade: Keeps IL weaving capabilities current
The extensive new transitive dependencies are expected given the rich UI rendering capabilities being added.
Also applies to: 23-32, 33-42, 43-54, 55-60, 61-71, 144-154
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (4)
27-28: LGTM! Efficient static regex patterns.The static regex compilation with
RegexOptions.Compiledprovides optimal performance for repeated use during query processing.
39-47: Excellent thread safety improvement!The
ParsingContextclass effectively addresses the previous thread safety concerns by localizing parsing state per query instead of using instance fields. This is a clean and safe approach.
71-71: LGTM! Clean integration of thread-safe parsing context.The local
ParsingContextinstance and its usage throughout the query processing maintains thread safety while keeping the code readable.Also applies to: 75-75, 88-88
130-220: Ensure robust number‐format handling with targeted testsThere's no existing coverage for
NormalizeNumber, so let’s validate its heuristics and guard against misparsing:• Add unit tests for these scenarios in
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(inside your test project):
– “1.000” in locales where “.” is decimal vs grouping
– Inputs with multiple decimals (“1.2.3”) or commas (“1,2,3”)
– Mixed separators (“1,234.56” vs “1.234,56”)
– Leading/trailing separators (“.123”, “123.”, “,123”, “123,”)
– Locale‐fallback cases where system group separator ≠ user intent• Verify that for each case:
–context.InputUsesGroupSeparatorsandcontext.InputDecimalSeparatorare set correctly
– The returned string is parseable bydecimal.Parse(…, CultureInfo.InvariantCulture)By explicitly covering these edge cases, we can confirm the method handles all international formats as intended.
…b.com/dcog989/Flow.Launcher into Calculator-accepts-flexible-separator
Removes external dependency for NumberGroupSeparator so it builds anywhere.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
124-220: Comprehensive number parsing logic with minor edge case concerns.The
NormalizeNumbermethod implements sophisticated logic to handle various international number formats. The approach of analyzing separator counts and positions is sound, and the fallback to system culture for ambiguous cases is appropriate.However, consider these potential edge cases:
- Invalid sequences: The regex
[\d\.,]+can still match invalid patterns like "1...2" or "1,,,2"- Leading/trailing separators: Patterns like ".," or ",." aren't explicitly handled
- Very large numbers: No validation for overflow scenarios
Consider adding input validation at the start of the method:
private string NormalizeNumber(string numberStr, ParsingContext context) { + // Skip processing if the string contains invalid separator sequences + if (numberStr.Contains("..") || numberStr.Contains(",,") || + numberStr.Contains(".,") || numberStr.Contains(",.")) + { + return numberStr; // Let Mages engine handle the error + } + var systemGroupSep = CultureInfo.CurrentCulture.NumberFormat.NumberGroupSeparator;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(6 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Plugins/Flow.Launcher.Plugin.Calculator/Views/CalculatorSettings.xaml.cs
🧰 Additional context used
🧠 Learnings (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (2)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
🔇 Additional comments (7)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (7)
4-4: LGTM: System.Linq import added for Count method usage.The addition of
System.Linqis necessary for theCount()method used in theNormalizeNumbermethod.
27-28: Good: Static regex instances improve performance.The static regex instances address the previous performance concern about creating regex objects on every query. The
RegexOptions.Compiledflag is correctly applied for better performance.
39-47: Excellent: ParsingContext addresses thread safety concerns.The introduction of
ParsingContextas a local class effectively resolves the previous thread safety issues by keeping query-specific state local to each query execution rather than as instance fields.
71-71: Well-implemented: Query method properly uses local context.The changes correctly instantiate a local
ParsingContextand pass it through the processing pipeline, ensuring thread safety while maintaining clean separation of concerns.Also applies to: 75-75, 88-88
222-244: Solid result formatting implementation.The
FormatResultmethod correctly applies the detected input format to the output, maintaining consistency with user expectations. The logic properly handles both integer and decimal results, and the conditional group separator application is appropriate.
246-251: Simplified group separator logic for consistency.The
GetGroupSeparatormethod now uses a straightforward opposite-separator approach rather than relying on system culture, which ensures consistent behavior across different locales and simplifies testing.
253-271: Streamlined validation logic.The simplified
CanCalculatemethod removes the culture-specific decimal separator validation, which aligns well with the new flexible number parsing approach. The remaining validations (length, character set, bracket matching) are still appropriate.
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Calculator plugin with smart number parsing and formatting capabilities that automatically detect and mirror user input formats. The plugin now intelligently handles different thousand separators and decimal separators (like 1,234.56 vs 1.234,56) regardless of system locale settings.
Key changes include:
- Advanced number parsing logic that distinguishes between separator types based on position and context
- Context-aware output formatting that mirrors the user's input format
- Code cleanup including removal of unused files and redundant UI code
Reviewed Changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Main.cs | Complete rewrite of number parsing with new NormalizeNumber method and context-aware formatting |
| plugin.json | Updated description and added co-author to reflect new functionality |
| NumberTranslator.cs | Removed unused file |
| MainRegexHelper.cs | Removed unused file (regex patterns moved to Main.cs) |
| CalculatorSettings.xaml.cs | Removed unused import |
| Language files (24 files) | Fixed spelling error in localization keys from "caculator" to "calculator" |
Comments suppressed due to low confidence (3)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs:290
- Variable name 'matchs' is misspelled. It should be 'matches'.
var matches = RegBrackets.Matches(query);
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs:136
- [nitpick] Lambda parameter 'f' is ambiguous. Consider using a more descriptive name like 'c' or 'character'.
int dotCount = numberStr.Count(f => f == '.');
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs:137
- [nitpick] Lambda parameter 'f' is ambiguous. Consider using a more descriptive name like 'c' or 'character'.
int commaCount = numberStr.Count(f => f == ',');
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (1)
133-223: Sophisticated parsing logic that handles international formats well.The
NormalizeNumbermethod implements comprehensive logic for detecting various number formats. The four-case approach (mixed, dots-only, commas-only, no separators) with ambiguity resolution using system culture is well-designed.Consider breaking this method into smaller helper methods for better maintainability:
private string NormalizeNumber(string numberStr, ParsingContext context) { var dotCount = numberStr.Count(f => f == '.'); var commaCount = numberStr.Count(f => f == ','); if (dotCount > 0 && commaCount > 0) return HandleMixedSeparators(numberStr, context); if (dotCount > 0) return HandleDotsOnly(numberStr, context, dotCount); if (commaCount > 0) return HandleCommasOnly(numberStr, context, commaCount); return numberStr; // No separators }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
Plugins/Flow.Launcher.Plugin.Calculator/Languages/ar.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/cs.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/da.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/de.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/en.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/es-419.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/es.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/fr.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/he.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/it.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/ja.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/ko.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/nb.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/nl.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/pl.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/pt-br.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/pt-pt.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/ru.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/sk.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/sr.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/tr.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/uk-UA.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/vi.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/zh-cn.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Languages/zh-tw.xaml(1 hunks)Plugins/Flow.Launcher.Plugin.Calculator/Main.cs(8 hunks)Plugins/Flow.Launcher.Plugin.Calculator/MainRegexHelper.cs(0 hunks)
💤 Files with no reviewable changes (1)
- Plugins/Flow.Launcher.Plugin.Calculator/MainRegexHelper.cs
✅ Files skipped from review due to trivial changes (25)
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/cs.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/ar.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/fr.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/nb.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/it.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/ru.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/sr.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/de.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/zh-cn.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/vi.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/da.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/ko.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/pt-br.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/zh-tw.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/es.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/pt-pt.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/pl.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/en.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/ja.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/tr.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/he.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/sk.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/nl.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/uk-UA.xaml
- Plugins/Flow.Launcher.Plugin.Calculator/Languages/es-419.xaml
🧰 Additional context used
🧠 Learnings (1)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (5)
Learnt from: Jack251970
PR: #3791
File: Flow.Launcher.Core/Plugin/PluginManager.cs:293-295
Timestamp: 2025-07-01T05:46:13.251Z
Learning: In Flow.Launcher.Core/Plugin/PluginManager.cs, when checking if a plugin is modified within the PluginManager class itself, prefer using the internal static PluginModified(string id) method directly rather than going through API.PluginModified() for better performance and architectural design.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher/App.xaml.cs:246-262
Timestamp: 2025-07-21T09:19:49.684Z
Learning: In Flow Launcher's App.xaml.cs, the asynchronous plugin initialization task (containing AbstractPluginEnvironment.PreStartPluginExecutablePathUpdate, PluginManager.LoadPlugins, PluginManager.InitializePluginsAsync, and AutoPluginUpdates) does not require additional try-catch error handling according to maintainer Jack251970, as these operations are designed to handle exceptions internally.
Learnt from: Jack251970
PR: #3854
File: Flow.Launcher.Core/Plugin/PluginManager.cs:280-292
Timestamp: 2025-07-21T09:19:19.012Z
Learning: In Flow Launcher's PluginManager.cs, the post-initialization operations (RegisterResultsUpdatedEvent, UpdatePluginMetadataTranslation, RegisterPluginActionKeywords, DialogJump.InitializeDialogJumpPlugin, and AddPluginToLists) are designed to be exception-safe and do not require additional try-catch error handling according to the maintainer Jack251970.
Learnt from: Yusyuriv
PR: #3118
File: Flow.Launcher/ViewModel/MainViewModel.cs:1404-1413
Timestamp: 2024-12-08T21:12:12.060Z
Learning: In the MainViewModel class, the _lastQuery field is initialized in the constructor and is never null.
Learnt from: Yusyuriv
PR: #3057
File: Flow.Launcher.Core/Plugin/JsonRPCPluginSettings.cs:0-0
Timestamp: 2024-11-03T07:40:11.014Z
Learning: In Flow Launcher, when using Windows Forms dialogs (e.g., in JsonRPCPluginSettings.cs), path validation is enabled by default in OpenFileDialog and FolderBrowserDialog, preventing users from selecting invalid paths, but it's possible to opt out of this validation on individual dialogs.
🪛 GitHub Check: Check Spelling
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs
[warning] 27-27:
isinfty is not a recognized word. (unrecognized-spelling)
[warning] 27-27:
isprime is not a recognized word. (unrecognized-spelling)
[warning] 27-27:
eig is not a recognized word. (unrecognized-spelling)
[warning] 27-27:
eigvec is not a recognized word. (unrecognized-spelling)
[warning] 27-27:
eigval is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (9)
Plugins/Flow.Launcher.Plugin.Calculator/Main.cs (9)
14-31: Excellent regex optimization with GeneratedRegex!The migration to
[GeneratedRegex]attributes addresses the previous performance concerns about creating regex instances on every query. This compile-time approach generates optimized code and the static readonly fields ensure single initialization.Note: The static analysis spelling warnings for "isinfty", "isprime", "eig", "eigvec", "eigval" are false positives - these are valid mathematical function names supported by the Mages engine.
42-50: Excellent solution for thread safety concerns.The
ParsingContextclass perfectly addresses the previous thread safety issues by localizing query state instead of using instance fields. This ensures safe concurrent query processing.
74-91: Clean implementation with proper state management.The query processing now correctly uses per-query context for thread safety and leverages the optimized static regex. The flow maintains good error handling while improving performance.
225-247: Well-implemented formatting that respects input conventions.The
FormatResultmethod properly uses the parsing context for thread-safe formatting and intelligently applies group separators only when the input used them and for numbers > 3 digits. This addresses the formatting concerns from previous reviews.
249-254: Simple and predictable separator logic.The simplified approach of using the opposite separator (dot↔comma) provides consistent, predictable behavior that's beneficial for testing and user experience. The clear documentation explains the design decision.
256-274: Appropriate simplification that aligns with flexible parsing.The simplified validation logic correctly removes culture-specific checks since the new parsing approach handles various separator formats dynamically. Focusing on structural validation (length, characters, brackets) is the right approach.
4-4: Good addition of required using directive.The
using System.Linq;directive is correctly added to support theCount()method used in the newNormalizeNumberimplementation.
290-292: Nice correction of variable name.The rename from
matchstomatchesfixes the grammatical error and improves code readability.
309-314: Localization key corrections look good.The corrected localization keys from "caculator" to "calculator" fix important typos that would affect the user interface.
VictoriousRaptor
force-pushed
the
Calculator-accepts-flexible-separator
branch
from
276817a to
443d4f7
Compare
|
@dcog989 is it ready to go? |
Core Logic
1,234.56and1.234,56). We distinguish between separator types based on their position and surrounding digits.'1,222.33*2 = 2,444.66'or'1222,33*2 = 2444,66').Code Cleanup
NumberTranslator.cswas unused and therefore removed.CalculatorSettings_Loadedevent handler inCalculatorSettings.xaml.cs(and its XAML registration) was removed. The functionality was already handled automatically by data binding.Maintainability
NormalizeNumbermethod inMain.csto clarify its purpose.So, the plugin is now much more flexible, and will accept whatever format the user gives it regardless of Windows region settings.