Quote a sheet name that reads as a boolean literal - #53
Draft
gthb wants to merge 3 commits into
Draft
Conversation
Normalizing a formula dropped the quotes from 'TRUE'!A1 and 'FALSE'!A1, emitting a spelling the library's own lexer reads back as a boolean joined to a reference rather than as a sheet prefix, and one Excel will not open. needQuotes now treats TRUE and FALSE like the digit-leading and range-like names it already quotes.
The new quoting rule was covered only through stringifyA1Ref. The symptom that prompted it is a whole-formula normalization, so pin it where the other prefix-quoting rules are pinned, alongside the range-like and digit-leading cases.
Four of the assertions in the two new blocks are unmoved by the rule and document neighbouring behaviour instead; group them under one comment that says so, in place of comments that named only some of them. Name the XLSX block for the workbook case it also covers.
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.
What
Fix a formula normalization that turns a safe formula into one Excel will not open. A sheet name spelled as a logical literal loses the quotes it needs:
That is a round-trip loss inside the library:
tokenizereads=SUM(TRUE!A1)asbool:TRUE,operator:!,range:A1, never as a sheet prefix, so the normalized formula no longer refers to the sheet the input did. Measured in Excel it is worse than misread — a workbook containing the unquoted spelling does not open at all, with no repair prompt and no repair log, exactly as one containing an unquoted$in a sheet name does not.Every serializer that qualifies a reference with a sheet name is affected, since all reach the same
needQuotescall:fixFormulaRangesin both default andxlsxmode, the round trip back from R1C1,stringifyA1Reffor a range and for a defined-name qualifier, andstringifyStructRef.Cause
needQuotesquotes a scope that contains a banned character, reads as an A1 or R1C1 range, or starts with a digit — each a spelling the lexer resolves ahead of a name. A logical literal is resolved ahead of a name in the same leading position, and no rule covered it. Only that position is at risk:=SUM([TRUE]Sheet1!A1)lexes back as a singlerangetoken, andTrueris one name.Fix
Add
reIsBooleanbeside the existingreIsRangelike, and quote a scope matching it. It mirrorslexBoolean, which recognisesTRUEandFALSEcase-insensitively and nothing else — fx models no localized formula literals, so those two words are the whole set.Deliberate over-quoting
needQuotessees every scope in every position, so the clause also quotes one that is not leading and therefore not at risk: a workbook or path scope of that name, or a sheet scope behind a workbook scope. The digit-leading and range-like rules already do the same, quoting'[1040.xlsx]Sheet1'!A1on a workbook name alone and'[Foo]Ab12'!B2on a sheet name that is not leading either. Matching them keeps one rule per hazardous spelling rather than one per spelling and position, at the cost of quotes that are redundant rather than wrong. The XLSX test pins'[TRUE]'!A1, so the choice is recorded rather than inferred.Relation to the other branches
#51 contains this fix among unrelated sheet-range work and drops it once this lands — the same arrangement as #52.