Skip to content

Apply percent immediately for all pending operators - #1

Open
sheep-farm wants to merge 2 commits into
omacom:masterfrom
sheep-farm:percent-apply-immediately
Open

Apply percent immediately for all pending operators#1
sheep-farm wants to merge 2 commits into
omacom:masterfrom
sheep-farm:percent-apply-immediately

Conversation

@sheep-farm

@sheep-farm sheep-farm commented Aug 15, 2026

Copy link
Copy Markdown

Summary

  • Unifies the percent button behavior so the result is shown immediately for all four operators (+, , ×, ÷).

Test plan

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

@sheep-farm
sheep-farm force-pushed the percent-apply-immediately branch from 6c654ed to 906a504 Compare August 15, 2026 00:52
@sheep-farm sheep-farm changed the title Apply percent immediately on pending addition or subtraction Apply percent immediately for all pending operators Aug 15, 2026
When % follows any pending operator, show the running total with the
percent applied as soon as % is pressed:

- 200 + 10% displays 220
- 200 − 10% displays 180
- 200 × 10% displays 20
- 200 ÷ 10% displays 2000

The evaluated expression is preserved as "200 + 10%" instead of the
intermediate "200 + 20", making the calculation easier to read. On its
own, x% still divides by 100.
@sheep-farm
sheep-farm force-pushed the percent-apply-immediately branch from 70ee295 to 6528d54 Compare August 15, 2026 01:07
@sheep-farm sheep-farm changed the title Apply percent immediately for all pending operators Apply percent immediately and add display formatting options Aug 15, 2026
@sheep-farm
sheep-farm force-pushed the percent-apply-immediately branch from 016e888 to 6528d54 Compare August 15, 2026 14:09
@sheep-farm sheep-farm changed the title Apply percent immediately and add display formatting options Apply percent immediately for all pending operators Aug 15, 2026
Applying the percent by collapsing everything left of the pending operator into a single base loses precedence for × and ÷: `2 + 3 × 10 %` evaluated (2 + 3) × 0.1 and showed 0.5 where every calculator, and this one before the change, shows 2.3. `10 + 2 × 3 %` showed 0.36 instead of 10.06. Resolving the percent into the operand the pending operator is waiting for and then evaluating the whole token list keeps the answer right while still showing it immediately, which is what the change was after.

The same rewrite gives the failure paths an answer. A non-finite total used to return with nothing changed, so `200 ÷ 0 %` left the percent key silently dead; it now errors the way `=` does on the same expression.

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

Copy link
Copy Markdown
Collaborator

Reviewed on a disposable VM; ./bin/test and ./bin/build both pass on the branch as it now stands.

The unification lost operator precedence for × and ÷. Taking evaluateTokens over everything left of the pending operator makes the base the whole left-hand side, so 2 + 3 × 10 % computed (2 + 3) × 0.1 = 0.5 where it used to give 2 + (3 × 0.1) = 2.3. Same shape for 2 + 3 ÷ 10 % (50, was 32) and 10 + 2 × 3 % (0.36, was 10.06). The expression line made it worse rather than better: it read 2 + 3 × 10% beside a display of 0.5, so the calculator contradicted itself. Every single-operand case in the README was right — 200 + 10 % → 220, 200 − 10 % → 180, 200 × 10 % → 20, 200 ÷ 10 % → 2000, 50 % → 0.5 — which is exactly why the new tests went green.

A smaller one alongside it: a non-finite total returned with nothing changed, so 200 ÷ 0 % left the percent key silently dead, and a left side that fails to evaluate (1 ÷ 0 + 5 %) did the same. Under "shows the result immediately" that should report the error the way = already does.

Pushed 18e23c1 for both. It keeps your immediate-result semantics and the shape of the code you wrote, and changes only how the total is reached: resolve the percent into the operand the pending operator is waiting for — a percentage of the running total for + and , a plain fraction for × and ÷, exactly as before — then evaluate the whole token list, which lets precedence stand. Both of your percent tests pass unchanged. percentKeepsPrecedence and percentErrorsOnDivisionByZero are new; both fail against 6528d54 and pass after. The README sentence goes back to naming the +/ versus ×/÷ distinction, because the code still makes it.

One thing left for the maintainer rather than for you: 200 + 10 % × 2 = changes from 240 to 440 under this PR, since % now completes the operation instead of filling in its operand. That is the point of the change and your test pins it, but it is a user-visible break from what iOS and macOS Calculator do, so it wants a decision rather than a quiet merge.

Worth knowing: #7 carries 6528d54 as-is, so it still has the precedence bug until this branch is merged or rebased into it.

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