fix: range 'a':'b'!A1 is mistaken for sheet range a:b!A1 - #66
Open
gthb wants to merge 6 commits into
Open
Conversation
`'Alpha':[Book.xlsx]Gamma!A1` and `'Alpha:Beta':Gamma!A1` fail to parse: "Unexpected context_quote token". Their first name is a quoted scope with no `!` after it, so it is not a scope but an operand, and `parse` has no production for one there. The lexer types it as a scope on the chance that a second sheet name follows. Where nothing merges it, `mergeRefTokens` now corrects it to `ref_named`, which is what Excel reads there and what the unquoted `a` of `a:'b'!A1` already gets, so the two ways of writing the operation agree. A scope that does have its `!` keeps the scope type whether or not it merged, since a scope containing a `:` never merges with the name or table after it, as in `'Sheet1:Sheet2'!name`. Untouched: an unquoted scope in the same position, `1` in `1:'dec'!B11`, which the lexer leaves as `context` and which still does not parse. A number is not a name, so it wants a different answer.
The fix is a token type, and only the lexer specs covered it. These assert the outcome that motivated it: both formulas reach an AST, with the quoted scope as the name operand of the range operator.
The test asserted `'Alpha:Beta':Gamma!A1` as the name `Alpha:Beta` joined to `Gamma!A1`, under a name that read as a claim about Excel. Excel's reading of that form is unmeasured, and for the nearest measured neighbour, a quoted first name followed by an unquoted second, Excel reads the sheet range instead. What the test is really for is that the formula reaches an AST at all, where the quoted first name used to stop the parser. Renamed to say it parses that way, with the scope of the claim in a comment.
The test name and its comment left the reading as fx's own. Excel takes both formulas the same way and stores them with the second name quoted, `'Alpha:Beta':'Gamma'!A1` and `Alpha:'[1]Gamma'!A1`. Neither resolves: a defined name cannot contain a colon, and `Alpha` is not a name in that workbook, so both are #NAME?. In the second form the workbook bracket is what rules out a sheet range; the quote on the first name is redundant and Excel drops it.
Two clauses went: the lexer's speculative typing, which the description already gives as the cause, and a justification for the `!` exclusion that was wrong. It said a scope containing a `:` never merges with the name after it, which holds under `xlsx` only; in the default mode `'Sheet1:Sheet2'!name` merges into one reference. What is left is the rule the guard implements.
`'a':'b'!A1` was lexed and parsed as the sheet range `a:b`. A quote on the second name makes the colon a range operator, whatever stands to its left, so it is the name `a` joined to `'b'!A1` -- a different formula over different sheets. Excel leaves the form as written on the next save, so nothing downstream corrects it. fx already had the rule for an unquoted first name, `a:'b'!A1`. It was the quoted-first case that kept the sheet-range reading, in both lexing paths: - `mergeRefTokens` had a pattern group for `'a':b!A1` and `'a':'b'!A1` together. Narrowing its second name to unquoted makes it a subset of the group below it, which already covers `'a':b!A1`, so the group goes and the surviving one carries the rule. - `pExtendedContext` accepted a quoted second name when the first was quoted too, and now accepts an unquoted one only. Four spec files asserted the old reading and now assert the new one. Two were already at odds with themselves: `fixRanges` asserted the sheet range under the title "a quote around the second end is the range operator, not a 3D reference", and a lexer comment said Excel corrects `'[Book.xlsx]Alpha':'Gamma'!A1` into a span, which is the reading this removes. That comment is gone, along with an `XXX` asking that fixRanges deal with the form, which it now does. One grouping changes with it. `addTokenMeta` gives references that resolve to the same cells one group id, so the `'dec'!B11` of `'jan':'dec'!B11` now shares a group with a lone `Dec!B11` instead of with the sheet ranges.
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.
'a':'b'!A1is lexed and parsed as the sheet rangea:b. A quote on the second name makes the colon a range operator, whatever stands to its left, so the formula is the nameajoined to'b'!A1: a different formula over different sheets. Excel leaves it as written on the next save, so nothing downstream corrects it.Cause
fx already had the rule for an unquoted first name,
a:'b'!A1, but not for a quoted one, in either lexing path: amergeRefTokenspattern group covered'a':b!A1and'a':'b'!A1together, andpExtendedContextaccepted a quoted second name whenever the first was quoted too.Fix
Both now require the second name to be unquoted. That makes the merge group a subset of the one below it, which already covers
'a':b!A1, so it goes.addTokenMetagroups references that resolve to the same cells, so the'dec'!B11of'jan':'dec'!B11now shares a group with a loneDec!B11rather than with the sheet ranges.Note
Includes #65's commits, which this needs. A cross-fork PR cannot target a branch on the fork, so they show until #65 lands. The real diff here is the last commit alone.