Quote sheet names per the file format, not the formula bar - #35
Open
jafin wants to merge 1 commit into
Open
Conversation
The quotation tables were collected from Excel's formula bar, which is more permissive than the file format. For a sheet named with a trailing U+FF5E the formula bar shows ABC~!A1 while the saved workbook stores 'ABC~'!A1, so a formula written from the displayed form can make Excel refuse to open the file. Both tables are re-collected from saved workbooks across the whole BMP, by driving Excel over COM and reading the stored formula back out of the XML. 41 codepoints in the first position and 37 in a later one were marked as needing no quotes when Excel quotes them. For a subset Excel rejects the workbook outright, and which position is fatal differs per codepoint: U+2028, U+2029, U+202A-U+202E, U+303D and U+303E as the first character, U+2065-U+2069 and U+303D anywhere in the name. The measurement is merged with the existing data as a union rather than replacing it. A newer Excel accepts 107 codepoints bare that the build behind the original tables quotes, because they were unassigned when it shipped. Quoting is accepted by every version, so the union keeps output loadable on both. Also quote a sheet named TRUE or FALSE in any casing. Every character is unremarkable on its own, so only the whole name gives it away; unquoted, Excel reads TRUE!A1 as a logical literal and rejects the file. Names shaped like a reference (A1, R1C1) are left alone: Excel quotes them but reads them bare either way, and the reader decides an identifier is an unquoted sheet name by calling ShouldQuote. Fix NameUtils.EscapeName, which doubled apostrophes from a hardcoded index 1 instead of the builder's length. Every caller happens to pass an empty builder, so no output was wrong, but the helper mangled the formula for any that did not. Add tools/sheet-quotation with the collector and the traps it has to work around, and a test asserting the compiled bitmasks still match the data files they are generated from. Fixes ClosedXML#29
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
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.
Fixes #29, though not for the reason the issue gives.
The reported character is not the bug
The issue points at U+FF5E and suspects the
QuoteFirst/QuoteNextbitmasks are missing bits. Checked against Excel (Office 16) — neither holds:ABC~!A1unquoted, in first and non-first position, and a workbook storing that form opens cleanly. Control: the same file with ASCII~(U+007E) unquoted is rejected, so the check discriminates.ident-sheet-first.txt/ident-sheet-next.txtbit-for-bit across all 65,536 codepoints.The real bug is in how the data was collected
The tables were collected from Excel's formula bar. The file format is stricter. For a sheet named
ABC~, Excel displaysbut stores
So a table built from the UI under-quotes, and for some characters the unquoted form makes Excel refuse to open the workbook — the reported symptom. Which position is fatal differs per codepoint:
U+303D (〽 PART ALTERNATION MARK) is an ordinary visible Japanese character, and a likely candidate for what the reporter actually hit.
What this changes
Both tables re-collected from saved workbooks across the whole BMP, driving Excel over COM and reading the stored
<f>back out of the XML. Excel's writer quotes a strict superset of the old tables: 41 divergences in first position, 37 in a later one, zero in the other direction.Merged as a union, not a replacement. The Excel build used here accepts 107 codepoints bare that the build behind the original tables quotes — they were unassigned when it shipped. Taking the new measurement outright would produce files that break on older Excel. Quoting is accepted by every version, so the union is the only safe merge. This is also why the change is additive: every one of the 78 data edits is
NO→YES.TRUE/FALSEsheet names are now quoted, in any casing. Every character is unremarkable alone, so only the whole name gives it away; unquoted, Excel readsTRUE!A1as a logical literal and rejects the file. Reference-shaped names (A1,R1C1,XFD1048576) are deliberately left alone — Excel quotes them but reads them bare either way, andTryGetUnquotedSheetdecides an identifier is an unquoted sheet name by callingShouldQuote, so quoting them would cost reading.NameUtils.EscapeNamedoubled apostrophes from a hardcoded index1instead of the builder's length. Every current caller passes an empty builder so no output was wrong, but the helper mangled the formula for any that did not:It now uses
AppendEscapedSheetName, which already had this right.Verification
End-to-end against Excel, with 1,095 sheet names per position rewritten using the library's output:
Reading is unaffected. All 78 changed names still parse from their old unquoted form and round-trip cleanly, so existing files stay readable — they are just re-emitted quoted.
The collector only ever measures the character in the last position for the non-first table, so a full mid-name pass over the BMP was run as a cross-check. It found zero codepoints needing quotes that the shipped table does not already cover, so both non-first positions agree.
746 tests pass (5 pre-existing skips).
Also included
tools/sheet-quotation/— the collector and a README of the traps, so the data can be regenerated rather than trusted: U+0003 wedges Excel on a modal dialogDisplayAlerts = $falsedoes not suppress; sheet names must not look like a cell reference or Excel quotes them for that reason instead of the character under test;SaveAsresolves relative paths against Excel's working directory rather than the caller's.Happy to split the tooling out or drop it if you would rather not carry it.