HF-24: add stringifyCurrency config callback for TEXT#1665
Open
marcin-kordas-hoc wants to merge 22 commits into
Open
HF-24: add stringifyCurrency config callback for TEXT#1665marcin-kordas-hoc wants to merge 22 commits into
marcin-kordas-hoc wants to merge 22 commits into
Conversation
✅ Deploy Preview for hyperformula-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
3 tasks
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:
|
sequba
requested changes
May 19, 2026
| * 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. |
Contributor
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?
| | 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. | |
Contributor
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.
|
|
||
| And now, HyperFormula recognizes these values as valid dates and can operate on them. | ||
|
|
||
| ## Currency integration |
Contributor
There was a problem hiding this comment.
This section should be made into a separate guide currency-handling
Comment on lines
+101
to
+103
| 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. |
Contributor
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).
|
|
||
| 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: |
Contributor
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.
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
3 participants
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.
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
Changes
TEXTformatting dispatch by adding a new currency hook and by skipping date/duration parsing for LCID-tagged currency formats, which could affect output for some existing format strings.Overview
Adds a new
stringifyCurrencyconfiguration callback (defaulting toundefined) thatTEXTconsults before date/time, duration, and numeric formatting, enabling custom currency rendering without adding currency data to core.Adjusts the formatter pipeline to avoid mis-parsing Excel LCID-tagged currency formats (
[$SYMBOL-LCID] ...) as date/time or duration tokens, and documents the new opt-in currency behavior and related limitations across the guides and changelog.Reviewed by Cursor Bugbot for commit 9afc8bd. Bugbot is set up for automated code reviews on this repo. Configure here.