Skip to content

Add configurable decimal separator and fixed decimal places - #3

Open
sheep-farm wants to merge 2 commits into
omacom-io:masterfrom
sheep-farm:decimal-separator-config
Open

Add configurable decimal separator and fixed decimal places#3
sheep-farm wants to merge 2 commits into
omacom-io:masterfrom
sheep-farm:decimal-separator-config

Conversation

@sheep-farm

Copy link
Copy Markdown

Summary

  • Adds two optional display preferences read from ~/.config/Omacom/omacalc.conf:
    • decimalSeparator (dot or comma) lets numbers be typed and displayed with a Brazilian-style comma instead of a period. All internal parsing and math stay locale-independent; only the presented digits and the decimal key label change.
    • fixedDecimalPlaces rounds non-integer results to a fixed number of decimal places (e.g. 10 ÷ 3 becomes 3.33 with fixedDecimalPlaces=2), while whole-number results (4 + 6) keep showing as plain integers. Left unset, formatting is unchanged.
  • Chained operands and expressions reflect the same rounding as the result they came from, instead of leaking full floating-point precision (e.g. 3.33 + 1, not 3.33333333333333 + 1).

Test plan

  • ./bin/test passes
  • ./bin/build succeeds

sheep-farm and others added 2 commits August 15, 2026 12:05
Reads two optional display preferences from ~/.config/Omacom/omacalc.conf:

- decimalSeparator (dot/comma) lets numbers be typed and shown with a
  Brazilian-style comma instead of a period. Internal parsing and math
  stay locale-independent; only the presented digits and the decimal
  key label change.
- fixedDecimalPlaces rounds non-integer results to a fixed number of
  decimals (e.g. 10 / 3 -> 3.33), while whole-number results keep
  showing as plain integers. Left unset, formatting is unchanged.

Chained operands and expressions reflect the same rounding as the
result they came from, instead of leaking full floating-point
precision.
With decimalSeparator=comma the display, and so the clipboard, uses a
comma: copying 1.5 puts "1,500" on the clipboard when it was typed as
1.500. QLocale::c() accepts a comma as a thousands group when three
digits follow it, so that parsed cleanly as 1500 and the existing
comma-to-dot fallback never ran. Copying a number out of the calculator
and pasting it back multiplied it by a thousand.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@omarchybot

Copy link
Copy Markdown
Collaborator

Reviewed this against master and ran ./bin/test and ./bin/build on a disposable VM; everything passes, and I confirmed the compatibility claim holds — with neither key set, formatResult() falls straight through to formatNumber() and localizeNumber() returns its argument, so every one of the three return paths in display() and the operand formatting in prettyExpression() are byte-identical to master. The locale boundary is otherwise clean: m_entry and m_tokens only ever receive C-locale strings, and localization happens strictly on the way out.

One defect, which I have pushed as 90e8c98. copyResult() copies display(), which now carries the configured separator, and pasteNumber() parses with QLocale::c(), which accepts a comma as a thousands separator when exactly three digits follow it. So with decimalSeparator=comma, typing 1.500, pressing Ctrl+C and then Ctrl+V turned 1.5 into 1500 — the existing comma-to-dot fallback never ran because the first parse had already "succeeded". The fix de-localizes the pasted text before parsing when the separator is a comma, and respectsDecimalSeparatorConfig now covers the round-trip; removing the fix makes that assertion fail with "1500".

Three things I did not touch, because they are calls for the maintainer rather than bugs to patch under you.

prettyExpression() now applies fixedDecimalPlaces to every operand while evaluation still uses the unrounded value, so the calculation line can show an equation that does not add up. With fixedDecimalPlaces=2, 1.004 + 0.004 = renders 1.00 + 0.00 and answers 1.01, and 10 ÷ 3 = + 0.004 = renders 3.33 + 0.00 and answers 3.34. The same code also rounds operands the user typed themselves, not just chained results: type 1.234, press +, and the display drops to 1.23 even though the math keeps 1.234. Your stated goal was to stop a chained result from leaking its full precision, and separating result-derived tokens from typed ones would get that without the false equations — but that is a design change, so I left it to Basecamp.

pressPercent() sets m_entry with formatNumber() rather than formatResult(), so with fixedDecimalPlaces=2 pressing 50 % shows 0.5 and then jumps to 0.50 on =. Cosmetic, but the entry buffer is the one place the setting deliberately does not apply, so which way it should go is a product decision.

In the tests, the settings.remove() cleanup sits at the bottom of each new test function. QCOMPARE returns on failure, so a single failing assertion in respectsDecimalSeparatorConfig skips the cleanup and leaks decimalSeparator=comma into the two fixed-places tests that follow; I saw exactly that cascade while mutation-testing the fix, where one real failure reported as three. A Qt Test cleanup() slot would make that robust.

Worth knowing: PR #7 already contains this commit, so the two cannot be merged independently.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants