Repository navigation
Keep leading and trailing blanks of template values visible in Excel - #1030
michelebastione merged 1 commit into
Conversation
…ini-software#295) FillTemplate wrote inline strings without xml:space="preserve", so Excel dropped leading and trailing blanks from filled values. Mark a <t> that starts or ends with white space, or that still holds a list placeholder whose value is filled in later, the way the exporter already does (mini-software#294).
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 3 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly describes the main change: preserving leading and trailing spaces in template values when displayed in Excel. Linked Issues check Issue #295requires leading spaces in XLSX template output to remain visible in Excel. Both template-generation modes callPreserveInlineStringSpacesafter single-value updates. The pass sets `xml:s…Out of Scope Changes check The changes stay within issue #295. The implementation adds whitespace preservation for template inline strings in both existing generation paths.TestIssue295verifies the behavior, and the `TestIs…Full details: Linked Issues check
-
Explanation
Issue
#295requires leading spaces in XLSX template output to remain visible in Excel. Both template-generation modes callPreserveInlineStringSpacesafter single-value updates. The pass setsxml:space="preserve"on inline text that starts or ends with whitespace and on list placeholders that are filled later.TestIssue295verifies the XML attribute and preserved values for scalar and list templates. The changes satisfy the active coding requirement.
Full details: Out of Scope Changes check
-
Explanation
The changes stay within issue
#295. The implementation adds whitespace preservation for template inline strings in both existing generation paths.TestIssue295verifies the behavior, and theTestIssueI4DQUNexpectation update matches the new generated XML. No unrelated production behavior or unrelated test scope is introduced.
-
- Fix all pre-merge checks with AI
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
gdols
left a comment
gdols
left a comment
There was a problem hiding this comment.
I went through this against the rest of the template code and it holds up.
What I checked:
- The order works: number, bool and date list cells are switched to
<v>bySetCellTypeinsideUpdateDimensionAndGetRowsInfo, before the new pass runs, so only string cells get the attribute. I tried a list with an int column and those cells come out as plain<v>12</v>, with noxml:space, and read back fine. - Formula cells (
$=...) are safe too:ProcessFormulasparses the row and replaces the whole<is>with<f>, so the attribute goes away with it. - A leading tab, a value made only of spaces and a trailing newline all get the attribute and keep the exact text.
- Full
MiniExcel.OpenXml.Testson Windows: 524/524 on net10.0 and net8.0, so that includes the 20 SQLite tests you could not run on macOS. Without the change toOpenXmlTemplate.Impl.cs,TestIssue295andTestIssueI4DQUNfail as you describe.
One thing outside this PR: OpenXmlWriter only checks for a plain space (StartsWith(" ")), while this pass uses char.IsWhiteSpace, so Export and FillTemplate now treat a leading tab or newline differently. Might be worth lining them up in a follow-up.
|
Thanks for checking those paths, especially the number, bool and formula cells, and for the full Windows run with the SQLite tests. On the follow-up: there are two places. |
Fixes #295
Problem
FillTemplateturns template cells into inline strings (<is><t>…</t></is>) but never writesxml:space="preserve"on the<t>. Excel drops leading and trailing blanks from a<t>without that attribute, so a value such as" Jack"shows asJack, even though MiniExcel reads the blanks back.Exportalready handles this (#294,OpenXmlWritersetspreserveSpace); the template path did not.As noted in the issue, the template keeps the original
celement, so the attribute has to be added there.Change
A new
PreserveInlineStringSpacespass inOpenXmlTemplate.Impl.csruns afterUpdateDimensionAndGetRowsInfoin bothGenerateSheetByUpdateModeAsyncandGenerateSheetByCreateModeAsync. It setsxml:space="preserve"on an inline<t>when:{{Name}}) are already filled in, so this covers them and also static text;{{…}}placeholder. List rows ({{employees.name}}) are filled in later by string replacement on the row XML, so the value is not known yet. Marking the template cell lets every generated row inherit the attribute without touching the per-row code.The attribute only changes how Excel treats white space at the edges of the text. Cells without such white space look the same in Excel.
Output change
List cells that hold string placeholders now always come out as
<t xml:space="preserve">…</t>, even when the value has no edge blanks. Single values only get the attribute when they need it. The text MiniExcel reads back is the same.TestIssueI4DQUNchecked the exact<t>markup of a list cell, so its expected string was updated to include the attribute.Tests
TestIssue295inMiniExcelGithubIssuesTests(next toTestIssue294):TestTemplateEasyFill.xlsx):A2(" Jack") andE2(" Jack has 123 points") carryxml:space="preserve"and keep their text;TestTemplateBasicIEmumerableFill.xlsx):A2,B2,A3andB3carry the attribute, the blanks in" Jack","HR "and" IT "are kept, andQueryreads them back unchanged.OpenXmlTemplate.Impl.cs,TestIssue295fails (Expected: "preserve",Actual: nullonA2) and so does the updatedTestIssueI4DQUN. With it, both pass. Dropping either condition of the new check makesTestIssue295fail again, once on the single-value part and once on the list part.What was run (macOS arm64, .NET SDK 10.0.401; .NET 11 is not installed, so
-p:TargetFrameworks=…limited the build to installed frameworks):dotnet test tests/MiniExcel.OpenXml.Tests -p:TargetFrameworks=net10.0: 504 passed, 20 failed. The same 20 tests fail without this change, all withDllNotFoundException: SQLite.Interop.dll(System.Data.SQLite has no native library for osx-arm64 here).net8.0: 504 passed, the same 20 SQLite failures.dotnet test tests/MiniExcel.Csv.Tests -p:TargetFrameworks=net10.0: 82 passed, 4 failed, all SQLite native-library errors as well.dotnet build src/MiniExcel.OpenXmlfornetstandard2.0,net8.0andnet10.0: no new warnings.Not run:
net9.0andnet11.0, the SQLite-backed tests, and opening the output in Excel itself. The tests check the XML that Excel reads, not what Excel displays.Summary by CodeRabbit