fix(format): support thousands grouping and format sections in numberFormat/TEXT (HF-287)#1716
Draft
marcin-kordas-hoc wants to merge 1 commit into
Draft
fix(format): support thousands grouping and format sections in numberFormat/TEXT (HF-287)#1716marcin-kordas-hoc wants to merge 1 commit into
marcin-kordas-hoc wants to merge 1 commit into
Conversation
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
Collaborator
Author
|
bugbot run |
marcin-kordas-hoc
force-pushed
the
feat/hf-287-numberformat-sections
branch
from
July 24, 2026 12:32
98bb963 to
8a64987
Compare
Performance comparison of head (fa890f6) vs base (23a7437) |
marcin-kordas-hoc
force-pushed
the
feat/hf-287-numberformat-sections
branch
from
July 24, 2026 12:54
8a64987 to
b92d6c0
Compare
…Format/TEXT (HF-287)
The TEXT number formatter understood only a single simple mask
(`[#0]+(\.[#0]*)?`), so complex masks leaked their unparsed tail into the
output (e.g. `TEXT(1234.5,"#,##0.00")` -> `1235,##0.00`) and it ignored
the instance's configured separators.
Extend the existing formatter in place (Option A):
- parser.ts: widen the number-format regex to a FLAT class `[#0,]+(\.[#0]*)?`
that admits the grouping comma (no nested quantifier — DEV-2120 ReDoS
discipline). Export its source for a white-box shape test.
- format.ts: strip presentational color tags (`[Red]`, ...) after the currency
callback and before date/time dispatch; split the mask into sign-selected
sections (positive;negative;zero) honoring quotes/escapes; thread Config so
the decimal glyph uses `decimalSeparator` and grouping uses `thousandSeparator`
(empty on default config -> no visible glyph). Sign is extracted on `abs`,
fixing the pre-existing `padLeft('-5',3)` bug (`TEXT(-5,"000.00")` -> `-005.00`).
Trailing scaler commas degrade to a visible literal rather than silently
mis-scaling. Parse failures fall back to the cleaned format string.
No public API change, no i18n, no grammar rewrite. Percent scaling, scaler
arithmetic, `?` placeholders, scientific notation and `[condition]` comparators
remain out of scope.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
marcin-kordas-hoc
force-pushed
the
feat/hf-287-numberformat-sections
branch
from
July 24, 2026 18:11
b92d6c0 to
fa890f6
Compare
Collaborator
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit fa890f6. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1716 +/- ##
===========================================
+ Coverage 97.21% 97.23% +0.01%
===========================================
Files 178 178
Lines 15611 15676 +65
Branches 3460 3440 -20
===========================================
+ Hits 15177 15242 +65
+ Misses 434 426 -8
- Partials 0 8 +8
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
TEXT/numberFormatonly understood a single simple mask ([#0]+(\.[#0]*)?); complex masks emitted garbage — e.g.TEXT(1234.5,"#,##0.00")→"1235,##0.00". It ignored thousands grouping (#,##0), format sections (;), and HF's configureddecimalSeparator/thousandSeparator.How (Option A — extend the formatter in place; no grammar rewrite, no public API, no i18n)
src/format/parser.ts: widennumberFormatRegexto admit,into the flat character class ([#0,]+(\.[#0]*)?) — no nested quantifier (keeps the DEV-2120 ReDoS discipline); white-box shape test via exportedNUMBER_FORMAT_REGEX_SOURCE.src/format/format.ts: section split (positive;negative;zero, quote/escape-aware), grouping withconfig.thousandSeparator, decimal viaconfig.decimalSeparator, sign onabs, color-tag strip applied AFTER thestringifyCurrencycallback (doesn't disturb that extension point), parse-failure falls back to the cleaned format string.compatibility-with-microsoft-excel.md) reworded as nuances; CHANGELOG under Fixed.Verified against Excel (customer repro table)
#,##0.001,234.50,)#,##0.00;-#,##0.00-1,234.50#,##0.00 "zł"1 234.50 zł)$#,##0.00;-$#,##0.00-$1,234.50000.00-005.00New spec: 36 tests. Format/TEXT regression: 203 green, 0 regressions.
One existing assertion flips (correct-behavior change):
function-text.spec.ts"works for number format" —TEXT(12.45,"$###,##0.00")was'$12,##0.00'(garbage), now'$12.45'. That test lives in the privatehyperformula-testsrepo and needs the paired update, or unit-tests CI will fail on the old expectation.Scoped OUT (per ADR — not regressions)
percent
0.00%, scaler arithmetic (degrades visibly),?placeholders, scientificE+,@/4th text section,[condition]comparators, and placeholder chars inside quoted literals. Also:=TEXT(x,"… ""zł""")as a literal formula still returns#ERROR!— HF's formula parser rejects embedded""(unrelated to the formatter, which does handle the quoted literal when the format arrives via config/programmatically).Note
Medium Risk
Changes core
format()dispatch and number rendering used byTEXT, with intentional output changes for previously broken masks; scope is localized to formatting with broad test coverage.Overview
Fixes
TEXT/ built-in number formatting so complex Excel-style masks no longer leak unparsed fragments (e.g.#,##0.00→1235,##0.00).The number path now splits format strings on
;(quote/escape-aware) and picks positive/negative/zero sections, groups thousands when#,##0is used andthousandSeparatoris set, usesdecimalSeparator/thousandSeparatorfrom config, formats negatives on magnitude with correct sign handling, and strips[Red]-style color tags afterstringifyCurrencybut before date/time dispatch. Placeholder-less sections render as literals; the parser regex admits grouping commas with a flat pattern and skips non-placeholder runs.Docs and CHANGELOG document Excel parity nuances and limits (percent, scaler commas, etc.). New
test/hf-287-numberformat.spec.tscovers oracles,TEXTintegration, and ReDoS regex shape.Reviewed by Cursor Bugbot for commit fa890f6. Bugbot is set up for automated code reviews on this repo. Configure here.