Skip to content

fix: keep CSV rich text and plain text values consistent - #1300

Open
qiaolezi001 wants to merge 2 commits into
apache:mainfrom
qiaolezi001:fix/fesod-1297-csv-rich-text
Open

qiaolezi001 wants to merge 2 commits into
apache:mainfrom
qiaolezi001:fix/fesod-1297-csv-rich-text

Conversation

@qiaolezi001

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Fixes #1297.

CSV rich-text writes can export an empty value, or the previous plain string when a write handler replaces it with rich text. Keep both CsvCell text representations consistent so CsvSheet serializes the latest value.

What's changed?

  • Synchronize plain text when setting a RichTextString, and refresh rich text when setting a String. Retain the supplied rich-text instance; CSV exports its text without formatting.
  • Add 19 regressions: public writer exports across XLS/XLSX/CSV and both memory modes, independent file reopen, plain/rich replacement in both directions, null/blank and numeric replacement, empty text, Unicode, comma/quote/newline escaping, and CsvRichTextString instances.
  • Limit production changes to two lines in CsvCell; the existing CSV serialization and non-string getter behavior are unchanged.

Validation

JDK25, project Java8 source/target:

  • Baseline: all seven CSV regression cases fail or error; the twelve XLS/XLSX controls pass. All nineteen pass after the fix.
  • CSV-focused suite: 57 tests pass.
  • ./mvnw clean package -pl fesod-common,fesod-shaded,fesod-sheet: 1,868 tests, zero failures/errors/skips, no test groups excluded, normal fuzz seed regressions included.
  • Repository-wide validate spotless:check, Hawkeye7.0.0 and git diff --check pass.

One earlier full run failed the untouched LargeDataTest.write timing comparison while builds were running together (27,377ms vs 13,364ms). The exact full command passed in an isolated build window (10,493ms vs 9,015ms). No test or assertion was changed; both runs are retained in the validation records.

Forced Javadoc generation (-Dmaven.javadoc.skip=false) was also attempted. Common/shaded succeed; sheet reports two existing unresolved links to Lombok-generated methods in unchanged ReadSheetHolder.java:111 and SimpleReadCacheSelector.java:90. This PR does not claim forced Javadoc succeeds or include unrelated doclint changes. No local five-JDK matrix or extended fuzzing campaign was run.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment (the change follows the existing setter contract; regressions document replacements).
  • I have added the necessary unit tests and all cases have passed.

Generated-by: AI

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are reported for the CSV text-synchronization change.

No open issues 🎉

What changed in this PR

Fixes #1297 by keeping CSV cells’ plain-text and rich-text values synchronized, preventing empty or stale text from being exported.

Changes:

  • Update both string setters in CsvCell while retaining supplied rich-text instances.
  • Add 19 regression cases covering XLS/XLSX/CSV, both memory modes, value replacement, blank values, Unicode, escaping, and reopened output files.
File Description
fesod-sheet/​src/​test/​java/​org/​apache/​fesod/​sheet/​format/​CsvRichTextTest.java Adds cross-format regressions for text consistency and serialized output.
fesod-sheet/​src/​main/​java/​org/​apache/​fesod/​sheet/​metadata/​csv/​CsvCell.java Synchronizes plain-text and rich-text state in both string setters.

Review effort: Balanced

This branch has not been deployed

No deployments
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.

[Bug] CSV rich-text cells silently lose text or export stale values

3 participants