fix: a quoted prefix after a range operator loses its quotes, changing meaning - #64
Open
gthb wants to merge 4 commits into
Open
fix: a quoted prefix after a range operator loses its quotes, changing meaning#64gthb wants to merge 4 commits into
gthb wants to merge 4 commits into
Conversation
fixFormulaRanges rewrote =SUM(Q1:'Sales'!A1) to =SUM(Q1:Sales!A1), which is the 3-D reference over the sheets Q1 to Sales rather than the name Q1 joined to 'Sales'!A1. Excel stores a range operation with its right prefix quoted, so the normalization changed which sheets a formula refers to. Quoting was derived from the sheet name alone. fixRanges now keeps the quotes a source wrote on a prefix right of a range operator, where dropping them would leave a sheet range; a bracketed prefix (Jan:'[1]Nope'!A1) still loses quotes it does not need. translateToA1 already added those quotes to match Excel, but only when the colon was the immediate neighbour, so a prefix behind whitespace, which Excel allows, was missed. Both now ask followsRangeOperator in stringifyPrefix.ts. That pass also lengthened tokens without moving the loc of later ones; it now carries the skew forward. docs/Prefixes.md says how the two functions differ.
borgar#63 rewrites the body of the digit-leading test and appends a new block right after it. Keeping master's title on that test, and putting this branch's new test next to the quoted-RHS test it belongs with, lets the two branches merge without conflict. No change to what is asserted.
The test comment and the prefix docs both said that dropping the quotes in `Q1:'Sales'!A1` would leave the sheet range `Q1:Sales`. It would not: a left endpoint that reads as a cell address is that cell, and wins over the sheet-range reading, so `Q1:Sales!A1` is the cell `Q1` joined to `Sales!A1` and means what the quoted form means. Both examples were picked from the one class where the rewrite is harmless. Use `foo:'Gamma'!A1`, where the left endpoint can be no cell address and the quotes really do decide: unquoted it is the sheet range `foo:Gamma`, quoted it is the name joined to `'Gamma'!A1`. Added as a test case; the cell-address cases stay, described as what they are. No behaviour change — the fix already covered both.
gthb
changed the title
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
1 participant
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.
fixFormulaRangesrewrites=SUM(foo:'Gamma'!A1)to=SUM(foo:Gamma!A1), which is the 3-D reference over the sheetsfootoGammarather than the namefoojoined to'Gamma'!A1. The quote around the right endpoint is what makes the colon a range operator, so dropping it changes which sheets a formula refers to. Excel stores that position quoted whatever the name.Quoting was derived from the sheet name alone.
fixRangesnow keeps the quotes a source wrote on a prefix after a range operator, where dropping them would leave a sheet range. A bracketed prefix (Jan:'[1]Nope'!A1) still loses quotes it does not need.translateToA1already adds those quotes to match Excel. Two fixes to that pass:followsRangeOperatorinstringifyPrefix.ts.locof later tokens. Those now shift by the added characters.docs/Prefixes.mdsays how the two functions differ.