HF-24: add stringifyCurrency config callback for TEXT#1665
HF-24: add stringifyCurrency config callback for TEXT#1665marcin-kordas-hoc wants to merge 24 commits into
Conversation
✅ Deploy Preview for hyperformula-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Performance comparison of head (9afc8bd) vs base (456addd) |
marcin-kordas-hoc
force-pushed
the
feature/hf-24-stringify-currency
branch
from
0246ce0 to
09babfd
Compare
Per code review — TypeScript signature already declares parameter
and return types, so {type} brackets in JSDoc are redundant noise
and inconsistent with the sibling exported functions in this file
(defaultStringifyDuration, defaultStringifyDateTime have no JSDoc
type tags).
…ngify callbacks
…claim, mention U+202F NBSP)
… entry, embedded-quote nuance, adapter guard)
- Add typed contract signature block at top - Add Minimal example subsection (3-line callback for fresh-user contract) - Add Default behavior subsection (explains defaultStringifyCurrency) - Add Error behavior subsection (callback exception propagation) - Add MS-LCID specification link in adapter intro - Drop trailing-quote rule from CURRENCY_RULES (not callable from TEXT) - Move NBSP tip below console.log output (was between config and output)
…vent letter-format hijack The previous order (DateTime -> Duration -> Currency) let parseForDateTimeFormat greedily match characters D, M, S, Y, H inside currency format strings. Formats like '[$USD-409] #,##0.00' or 'USD #,##0.00' were converted to '[$US9-409] #,##0.00' before the user-supplied stringifyCurrency callback could intercept them. Currency dispatch now runs first. The default callback returns undefined for every input, so the existing date/time/duration/number-format chain is preserved bit-for-bit when stringifyCurrency is not set. Found by Codex review (codex-cli 0.130.0, base develop, max effort).
defaultStringifyDateTime now returns undefined when formatArg contains Excel's LCID-tagged currency notation [$SYMBOL-LCID]. Without this guard, parseForDateTimeFormat greedily consumed D/M/S/Y/H letters inside the currency code, mangling output even when a user-supplied stringifyCurrency callback returned undefined for opt-out. Before: TEXT(100, '[$USD-409] #,##0.00') with partial callback -> '[$US9-409] #,##0.00' (D->9 mangle) After: TEXT(100, '[$USD-409] #,##0.00') with partial callback -> '[$USD-41009] #,##0.00' (USD preserved, falls through to numberFormat) Excel never uses [$...] for date formats, so the guard is unambiguous. Found by Codex re-review (after first dispatch reorder fix d119b4c).
… date locale) Codex re-review identified that the prior LCID guard (introduced in d496e30) over-matched Excel's locale-only modifier syntax `[$-LCID]` used in date and time formats (e.g. `[$-409]dd/mm/yyyy`), incorrectly skipping date dispatch and falling through to numberFormat. The guard regex now requires a non-empty SYMBOL portion between `[$` and the dash. Currency tags (`[$USD-409]`, `[$€-2]`, `[$zł-415]`) continue to skip date dispatch as intended; locale-only modifiers (`[$-409]`, `[$-F800]`) flow through to parseForDateTimeFormat as before. Also softens the 'bit-for-bit preserved' doc claim: for LCID-tagged currency formats without a callback, output now goes through numberFormat (best-effort) instead of the pre-existing date-parser hijack. Setting stringifyCurrency remains the recommended path.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…tency) Bugbot identified that the LCID-tagged currency guard added to defaultStringifyDateTime (b7c61a5) was missing from its sibling defaultStringifyDuration. Currency symbols containing duration-token letters (H in CHF/HUF, M in AMD/HMD) were interpreted as time tokens when a stringifyCurrency callback returned undefined. Applies the same regex `/\[\$[^\-\]]+-/` guard in identical position to preserve sibling parity with defaultStringifyDateTime.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 80fd34e. Configure here.
Bugbot Low: previous comment 'preserving the existing dispatch path bit-for-bit when stringifyCurrency is not set' was inaccurate after the LCID guards landed in defaultStringifyDateTime/Duration. For non-currency formats bit-for-bit holds; for LCID-tagged currency formats output now falls through to numberFormat instead of being mangled by the date parser — a deliberate improvement, not a preservation. Comment reworded to acknowledge this.
Public branch merged upstream/develop in d77d5a6 (bringing HF-85 D-function code). Tests-repo branch merged origin/develop in 354b872 (bringing HF-85 D-function tests). CI clones tests-repo by matching branch name, so this empty commit re-runs the full matrix with the updated tests checkout. Should resolve the codecov/project drop (was -1.40% because D-function code shipped without matching tests in the same branch namespace).
|
Task linked: HF-85 Implement function DCOUNT |
The previous wording suggested that the built-in number formatter handles `$#,##0.00` via the default dispatch path. Sandbox audit showed the built-in numberFormat actually fails on any format that includes the comma thousands separator: TEXT(1234.5, "$#,##0.00") -> "$1235,##0.00" (not "$1,234.50") The intro paragraph now lists only the formats that genuinely work without a callback (`$0.00`, `$0`, `$#.00`) and explicitly calls out the broken cases (`$#,##0.00`, LCID-tagged, accounting two-section). The Default behavior subsection gains a side-by-side comparison table (without callback / with adapter / Excel) and a recommendation to set `stringifyCurrency` for any application showing currency to users. Docs-only change. No source or test impact.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1665 +/- ##
========================================
Coverage 97.16% 97.16%
========================================
Files 175 175
Lines 15319 15329 +10
Branches 3287 3290 +3
========================================
+ Hits 14884 14894 +10
Misses 435 435
🚀 New features to boost your workflow:
|
| * The INDEX function doesn't support returning whole rows or columns of the source range – it always returns the contents of a single cell. | ||
| * The FILTER function accepts either single rows of equal width or single columns of equal height. In other words, all arrays passed to the FILTER function must have equal dimensions, and at least one of those dimensions must be 1. | ||
| * Array-producing functions (e.g., SEQUENCE, FILTER) require their output dimensions to be determinable at parse time. Passing cell references or formulas as dimension arguments (e.g., `=SEQUENCE(A1)`) results in a `#VALUE!` error, because the output size cannot be resolved before evaluation. | ||
| * The TEXT function does not accept embedded double-quote literals in the format string (e.g., `=TEXT(A1, "#,##0.00 ""zł""")` fails at parse time). Use the LCID-tagged form (`[$zł-415] #,##0.00`) or supply a custom [`stringifyCurrency`](configuration-options.md#stringifycurrency) callback that handles such formats outside the parser. |
There was a problem hiding this comment.
What is the LCID-tag? If I want to display polish currencies in a format "1234,56 zł", can I do it without stringifyCurrency?
There was a problem hiding this comment.
Good catch — clarified inline in known-limitations.md: defined the LCID tag as a Microsoft Locale ID with a link to the MS-LCID spec, and explicitly noted that the Polish "1234,56 zł" pattern (decimal-comma) requires the stringifyCurrency callback because the built-in number formatter always emits . as the decimal separator. Fixed in f1eb4ef.
| | Coercion of explicit arguments | VARP(2, 3, 4, TRUE(), FALSE(), "1",) | 1.9592, based on the behavior of Microsoft Excel. | GoogleSheets implementation is not consistent with the standard (see also `VAR.S`, `STDEV.P`, and `STDEV.S` function.) | 1.9592 | | ||
| | Ranges created with `:` | A1:A2<br><br>A$1:$A$2<br><br>A:C<br><br>1:2<br><br>Sheet1!A1:A2 | Allowed ranges consist of two addresses (A1:B5), columns (A:C) or rows (3:5).<br>They cannot be mixed or contain named expressions. | Everything allowed. | Same as Google Sheets. | | ||
| | Formatting inside the TEXT function | TEXT(A1,"dd-mm-yy")<br><br>TEXT(A1,"###.###”) | Not all formatting options are supported,<br>e.g., only some date formatting options: (`hh`, `mm`, `ss`, `am`, `pm`, `a`, `p`, `dd`, `yy`, and `yyyy`).<br><br>No currency formatting inside the TEXT function. | A wide variety of options for string formatting is supported. | Same as Google Sheets. | | ||
| | Formatting inside the TEXT function | TEXT(A1,"dd-mm-yy")<br><br>TEXT(A1,"###.###”) | Not all formatting options are supported,<br>e.g., only some date formatting options: (`hh`, `mm`, `ss`, `am`, `pm`, `a`, `p`, `dd`, `yy`, and `yyyy`).<br><br>Currency formatting is opt-in via the [`stringifyCurrency`](date-and-time-handling.md#currency-integration) callback; without it, currency format strings fall through to the built-in number formatter.<br><br>Embedded double-quote literals (e.g. `#,##0.00 "zł"`) are not accepted by the parser; use the LCID-tagged form (`[$zł-415] #,##0.00`) instead. | A wide variety of options for string formatting is supported. | Same as Google Sheets. | |
There was a problem hiding this comment.
Since we added stringifyCurrency, I think we can remove this line from List of Runtime Differences. Instead in the Compatibility with Ms Excel and Compatibility with Google Sheets guides, we should mention that the user need to provide both stringifyDateTime and stringifyCurrency to support all formats in the TEXT function.
There was a problem hiding this comment.
Done. The TEXT-formatting row in list-of-differences.md is now a one-liner that points at both stringifyDateTime and stringifyCurrency. Added a new "TEXT function formats" subsection to both compatibility-with-microsoft-excel.md and compatibility-with-google-sheets.md noting that both callbacks together cover the full TEXT format range. Fixed in f1eb4ef.
|
|
||
| And now, HyperFormula recognizes these values as valid dates and can operate on them. | ||
|
|
||
| ## Currency integration |
There was a problem hiding this comment.
This section should be made into a separate guide currency-handling
There was a problem hiding this comment.
Done. The whole "Currency integration" section moved into a new top-level guide docs/guide/currency-handling.md, wired into the Internationalization sidebar section. date-and-time-handling.md keeps a single-paragraph pointer to the new guide. Fixed in f1eb4ef.
| By default, the `TEXT` function renders only the simplest currency-looking formats — `"$0.00"`, `"$0"`, or `"$#.00"` (no thousands separator). Common Excel patterns such as `"$#,##0.00"` (with comma grouping), `"[$€-2] #,##0.00"` (EUR with German grouping), `"[$zł-415] #,##0.00"` (PLN), or accounting two-section formats like `"$#,##0.00;($#,##0.00)"` are **not** rendered correctly by the built-in number formatter; provide a [`stringifyCurrency`](../api/interfaces/configparams.md#stringifycurrency) callback to handle them. | ||
|
|
||
| HyperFormula itself ships with **no currency data** and **no currency library dependency**. You choose how to format: native `Intl.NumberFormat`, a third-party library, or a hand-rolled lookup table. |
There was a problem hiding this comment.
IMHO these paragraph sound to negative. They say a lot about things that HyperFormula does not support. I'd rather say it in more positive tone like Out of the box HF supports all currency symbols through the currencySymbolconfig option and simple currency formats e.g.: "$0.00", "$0", or "$#.00". If your app needs more formats, you can define them using thestringifyCurrency configuration option. (do not use my exact wording; it's just an example of the more positive-sounding tone).
There was a problem hiding this comment.
Reframed. The new currency-handling.md opens by stating what HF handles out of the box (simple $-prefixed formats — "$0.00", "$0", "$#.00") and frames stringifyCurrency as the additive extension point for locale-aware grouping, non-$ symbols, decimal-comma patterns, and accounting two-section formats. The reference table is preserved further down for users who want the side-by-side comparison, but no longer leads the page. Fixed in f1eb4ef.
|
|
||
| HyperFormula itself ships with **no currency data** and **no currency library dependency**. You choose how to format: native `Intl.NumberFormat`, a third-party library, or a hand-rolled lookup table. | ||
|
|
||
| The callback contract: |
There was a problem hiding this comment.
Before discussing the callback, give a very simple example of the behavior with just currencySymbol provided and no custom stringifyCurrency.
There was a problem hiding this comment.
Done. The new currency-handling.md now opens with a "Default behavior" subsection that demonstrates simple $-prefixed formats ("$0.00", "$#.00") producing "$1234.50" out of the box, before introducing the callback contract. The stringifyCurrency discussion follows for users who need richer formats. Fixed in f1eb4ef.
Addresses review feedback on PR #1665: - Extract Currency integration section from date-and-time-handling.md into a new top-level guide currency-handling.md so the topic stands on its own and is reachable from the sidebar. - Open the guide with a positive framing: out-of-the-box support for simple $-prefixed formats, with stringifyCurrency as the additive extension point for richer patterns. Lead with a default-behavior example before introducing the callback. - known-limitations: explain what an LCID tag is inline and clarify that the Polish '1234,56 zł' (decimal-comma) pattern requires the stringifyCurrency callback because the built-in number formatter always emits '.' as the decimal separator. - list-of-differences: drop the TEXT-formatting paragraphs that were duplicating the new guide; replace with a one-liner pointing at stringifyDateTime + stringifyCurrency for full TEXT coverage. - compatibility-with-microsoft-excel and compatibility-with-google-sheets: add a 'TEXT function formats' subsection noting that both callbacks together cover the full TEXT format range. - Update ConfigParams.ts JSDoc cross-reference to point at the new currency-handling guide. Regenerated docs/api artifacts (gitignored) follow automatically via typedoc on docs:build.
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
The previous push uploaded commit f1eb4ef to upstream but the pull_request:synchronize event did not fire workflow runs — only the lightweight bot checks (Cursor Bugbot, Netlify rules) attached. This empty commit forces a fresh synchronize so the full matrix (Lint, Test, Build on various envs, Performance, Security, CodeQL, Build docs) runs against the new docs reorganization.
3 participants
Summary
Adds a
stringifyCurrencyconfig option mirroring the existingstringifyDateTime/stringifyDurationcallbacks. When set, theTEXTfunction consults the callback before falling through to the built-in number formatter, so users can plug in locale-aware currency formatting (for example viaIntl.NumberFormator a third-party library) without bringing currency data into the HyperFormula core.The default implementation returns
undefinedso existing TEXT behavior is preserved bit-for-bit.Linked
agents/hyperformula/docs/specs/2026-04-21-hf-24-currency-in-text.mdagents/hyperformula/docs/specs/2026-04-24-hf-24-tech-rationale.mdagents/hyperformula/docs/specs/2026-04-27-hf-24-stringify-currency-plan.mdTests
Tests added in the matching
feature/hf-24-stringify-currencybranch ofhandsontable/hyperformula-tests. Coverage:undefinedundefined) → fall-through tonumberFormatstringifyCurrencyIntl.NumberFormatadapter (USD shorthand, EUR via LCID, JPY via LCID, PLN via LCID, accounting two-section), plus a fall-through case demonstrating opt-outNotes
#,##0.00 "zł"(trailing quoted symbol). HF's formula parser does not accept embedded quotes inside TEXT format strings, so the docs example and the corresponding test were swapped to use[$zł-415] #,##0.00(LCID-tagged symbol). The adapter still recognizes the trailing-quote pattern for users invoking the callback outside HyperFormula.'1.234,50 €'(symbol-trailing) for[$€-2]and'￥1,235'(full-width yen sign, no space) for[$¥-411]because that is whatIntl.NumberFormat('de-DE'/'ja-JP', ...)actually produces on modern Node ICU. NBSP normalization in tests covers both\u00A0and\u202Fvariants for ICU build robustness.[$SYMBOL]boundary: the example regex requires the-LCIDsegment. A bare[$USD]pattern is not handled by the adapter and falls through to the built-innumberFormat, whose handling of[$...]in HyperFormula is implementation-defined. Testdocs adapter does not handle [$SYMBOL] without LCID segmentdocuments the boundary.456adddff= develop with HF-85 DatabasePlugin). HF-24's runtime impact is one extra dispatcher call informat()perTEXTinvocation, which the Sheet A/B/T benchmarks don't exercise. The variance is most likely benchmark noise or HF-85 import overhead carried in via the develop merge, not HF-24-specific.Test plan
handsontable/hyperformulaPRhandsontable/hyperformula-testsPR (matching branch)Intl.NumberFormaton Node 14+Private tests PR: handsontable/hyperformula-tests#10
Note
Medium Risk
Touches the core
TEXTformatting dispatch path by adding a new currency hook and new LCID-tag guards, which could subtly change formatting behavior for some format strings. Default behavior is intended to remain unchanged, but regressions are possible around date/time vs currency format parsing.Overview
Adds a new
stringifyCurrencyconfiguration callback (defaulting to a no-op) that lets consumers override currency rendering in theTEXTfunction.Updates the formatter dispatch so currency formatting is attempted before date/time parsing, and adds LCID-tag guards in
defaultStringifyDateTime/defaultStringifyDurationto avoid mis-parsing Excel-style currency tags.Extends docs and changelog with a new Currency handling guide, links from compatibility pages, and updates
TEXTdocs/limitations to reference the new option.Reviewed by Cursor Bugbot for commit a9474ca. Bugbot is set up for automated code reviews on this repo. Configure here.