Skip to content

feat(Calculator): format large numbers with locale-aware thousands separators#47001

Closed
SAY-5 wants to merge 1 commit intomicrosoft:mainfrom
SAY-5:feat/calc-thousands-separators
Closed

feat(Calculator): format large numbers with locale-aware thousands separators#47001
SAY-5 wants to merge 1 commit intomicrosoft:mainfrom
SAY-5:feat/calc-thousands-separators

Conversation

@SAY-5
Copy link
Copy Markdown

@SAY-5 SAY-5 commented Apr 13, 2026

Summary of the Pull Request

Calculator results for large numbers (e.g. 1000000) now display with thousands separators (e.g. 1,000,000) using the user's locale. Small numbers and decimals keep their existing formatting.

The approach: detect how many decimal places the raw result has, then apply the N format specifier with that count. This preserves the original precision while adding separators to the integer part.

Before: 1000000 / 3.14159
After: 1,000,000 / 3.14159

PR Checklist

  • Closes: Large numbers separators in Command palette #40203
  • Communication: Issue is labeled Help Wanted
  • Tests: Manually verified with various numbers and locale settings
  • Localization: Uses CultureInfo from the existing output locale setting
  • Dev docs: N/A
  • New binaries: N/A

Calculator results like 1000000 now display as 1,000,000 (or the
locale equivalent). Uses the N format specifier with the same number
of decimal places as the raw result, so 3.14 stays 3.14 but
1000000 becomes 1,000,000.

Fixes microsoft#40203
@daverayment daverayment added the CmdPal - Calculator Issues related to the Calculator built-in extension label Apr 14, 2026
@daverayment
Copy link
Copy Markdown
Collaborator

This is seriously flawed, so I'm closing it.

After an initial review, here are some findings:

  1. TextToSuggest has a serious regression (line 135). Setting decimalResult to the result of the new FormatWithSeparators() method means that the outputCulture-encoded value - "1,234,567.89" for example - will be placed in TextToSuggest. This 'poisons' the suggestions with outputCulture formatted results instead of retaining the original values which lack thousands separators.
  2. There's a CopyTextCommand regression, too, because decimalResult no longer contains the plain value:
        return new ListItem(new CopyTextCommand(decimalResult))

(Line 130)

This results in the formatted value, e.g. "1,234,567.89" being stored for copying, which will likely break every programmatic use case, such as pasting into spreadsheet formulas, code, other calculators etc.

  1. SaveCommand also suffers a regression, with the formatted value being saved instead of the plain value. This means saved items will break calculator parsing if resubmitted as part of an expression:
        var result = FormatWithSeparators(roundedResult.Value, outputCulture);
        ...
        var saveCommand = new SaveCommand(result);

The core issue with the PR is that you're conflating the display formatting with the data value itself. The two need to be strictly separated, and the formatted value should never pollute the rest of the system. I don't believe the work has been tested sufficiently and no proof of testing was submitted.

For reference, PowerToys Run correctly separates Title from QueryTextDisplay in its ResultHelper code. I believe this is all that is required from the fix.

Finally, please note that an issue with the "Help Wanted" label doesn't mean that you don't need to follow the contributor guidelines. PR work should be pre-agreed.

@SAY-5
Copy link
Copy Markdown
Author

SAY-5 commented Apr 14, 2026

Fair criticism. I was conflating the display value with the data value. The formatted string leaked into TextToSuggest, CopyCommand, and SaveCommand, breaking copy-paste and saved expression resubmission. The fix needed to only change the Title while leaving the underlying data untouched, like how PowerToys Run separates Title from QueryTextDisplay. Thanks for the detailed breakdown.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CmdPal - Calculator Issues related to the Calculator built-in extension

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Large numbers separators in Command palette

2 participants