Repository navigation
fix: DataFormatter trailing-comma scaling and literals between digits - #1232
Merged
Merged
Conversation
Port of apache/poi#1396. - Trailing commas scaled by an extra power of ten for each character between the decimal point and the comma (the issue #2319 workaround), e.g. "0," showed 1234567 as "124" instead of "1235". Remove that and divide exactly, so formats such as "#,##0.0," keep their decimal places. - Excel treats '!' as an escape like '\'; rewrite it as '\' outside quotes, brackets and '_'/'*' operands. - Number formats with quoted or escaped text between digit placeholders, e.g. "0\.0," (issue #2319: shows 9234 as "0.9"), cannot be expressed with DecimalFormat, so they are now formatted by CellFormat. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Test Results1 178 tests +25 1 178 ✅ +25 2m 22s ⏱️ -6s Results for commit a1b6175. ± Comparison against base commit 56e8327. This pull request removes 8 and adds 33 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
3 tasks done
Contributor
|
Great PR! Thank you very much! |
skytin1004
approved these changes
Oct 16, 2026
Contributor
There was a problem hiding this comment.
LGTM @pjfanning. I verified the changes against the issue and ran the tests locally. All tests passed, and I did not find a blocker.
Formats with quoted or escaped text after trailing scaling commas, such as 0.0,"万" or #,##0,,"M", fell back to the raw value because the stripped pattern is not valid for DecimalFormat, and a quoted comma in #,##0.0,"," was taken as an extra scaling comma. Route these to CellFormat too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pjfanning
added a commit
to apache/poi
that referenced
this pull request
Oct 17, 2026
…mat (#1426) * DataFormatter: format literal text after trailing commas with CellFormat Formats with quoted or escaped text after trailing scaling commas, such as 0.0,"万" or #,##0,,"M", fell back to the raw value because the stripped pattern is not valid for DecimalFormat, and a quoted comma in #,##0.0,"," was treated as another scaling comma. Route these sections to CellFormat, like formats with literal text between digit placeholders. Reported in apache/fesod#1232, whose DataFormatter was ported from POI. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Note that the trailing-comma literal cases were checked with Excel Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
pjfanning
added a commit
that referenced
this pull request
Oct 17, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bengbengbalabalabeng
added a commit
that referenced
this pull request
Oct 19, 2026
* test: check DataFormatter against text computed by Excel Adds DataFormatterAgainstExcelTest, a port of POI's TestDataFormatterAgainstExcel (apache/poi#1398). The TEXT(value, format) results cached in Excel-saved sample workbooks from POI's test data are compared with what Fesod's DataFormatter produces. Chinese date/time and [DBNum] cases from POI's TestDataFormatter (apache/poi#1400) are also included. Cases that do not match are listed, grouped by cause, and asserted to still differ, so the lists document current behaviour and give regression coverage for changes to the formatter, such as switching to POI's DataFormatter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: drop cases fixed by #1232 from the known failures Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Bengbengbalabalabeng <70380092+bengbengbalabalabeng@users.noreply.github.com>
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
4 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.
Ports apache/poi#1396 to Fesod's
DataFormatter. Fixes #1120.Warning
This PR and #1231 (the "check DataFormatter against Excel" test) will conflict. Whichever is merged second needs a rebase.
#1231's test asserts that each known failure still differs from Excel. This PR fixes 167 of those cases, so after both are in, #1231's
UNSUPPORTED_FORMATS/KNOWN_FAILURESlists need those entries removed:0,,#,,,0.#,and#,.#,|#\:#=/=#|and|0\:0=/=?|I checked by running #1231's test on this branch: the only failures were "now matches Excel" cases, and there were no regressions. Matches go from 151 to 318 of the 569 Excel cases.
Note
Overlap with #1121. #1121 (@nkuprins) fixes the same issue (#1120) in the same code, so only one of the two should be merged. All 14 test cases from #1121's
scalesByTrailingCommasalso pass on this branch. The two take different approaches:DecimalFormatand adds an extra power of ten for digits after an escaped or quoted dot.CellFormat, which also handles cases the extra-shift approach can't, e.g.0".0"0,→123.05,0\.00,;-0\.0,(a different literal per section), and fractions with literal text.Keeping the same logic as POI also makes a later switch to POI's
DataFormattereasier.Changes
1. Trailing commas no longer over-scale.
The issue #2319 workaround also divided by ten for each character between the
.and the,. It usedindexOf, so even formats without a dot were affected. That has been removed.0,1241235#,,010.#,123.51234.6The division is now exact (
BigDecimal.divide(divider), no rounding), so the decimal places survive until formatting.#,##0.0,gives1,234.6.2.
!is an escape character, like\.0!.0,→0.9and0!!→5!. A!inside quotes, inside brackets, or after_or*is left alone.3. Number formats with literal text between digit placeholders go to
CellFormat.DecimalFormatcan't express these. For #2319's format0\.0,, 9234 →0.9and 123456 →12.3. The format is cached as aCellFormatWrapper.Tests
DataFormatterTest: newscalesByTrailingCommas,keepsLiteralTextBetweenDigitsandtreatsExclamationMarkAsEscapetables. These are POI's #1396 cases, plus the0,/#,,/0.#,regressions and #2319's values.TrailingCommaFormatReadTest: reads the #2319 workbooks (temp/issue2319/test1.xlsxandtest2.xlsx, still in the test resources) end to end and asserts the formatted values. The originalIssue2319Testwas removed in refactor(examples): Fesod Examples Module Refactoring #770, and it only printed results without asserting anything../mvnw -pl fesod-sheet test -Dmaven.test.skip=false: 1112 tests, 0 failures.spotless:checkis clean.🤖 Generated with Claude Code