Fix non-ASCII characters breaking unquoted sheet names and names - #52
Open
gthb wants to merge 5 commits into
Open
Fix non-ASCII characters breaking unquoted sheet names and names#52gthb wants to merge 5 commits into
gthb wants to merge 5 commits into
Conversation
Fix bug: in `lexNameFuncCntx` the continuation loop tested the token's first character `s` against the high-char range (`s > 180`) while indexing `ALLOWED` with the current character `c`. For a name that starts with an ASCII character, `s > 180` is false, so a later high character (e.g. æ, code 230) indexed `ALLOWED` out of bounds, resulting in `undefined`, treated as invalid, and the name split at that character. For example, `Forudsætninger!A1` tokenized as `Foruds` + `ætninger!A1` instead of one reference. Names starting with a high character (`Ærø`) were unaffected because `s > 180` was true. Test the current character `c` against the high-char range, mirroring the start-character test above.
A formula whose sheet name starts above the character table lexed as one name
token, losing the "!" and the reference behind it: "Ærið!A1" came back whole.
The previous commit fixed that by reading the mask off the character in hand;
this covers the character the range boundary left out.
ALLOWED holds one entry per code point from U+0020 up to and including U+00B3,
and past it every character is allowed wherever a high one is, which is what
OK_HIGHCHAR says. The guard admitted only what is beyond U+00B4, so that one
character was in neither: it read as undefined out of the table, which is no
mask at all. It refused to start a name ("´!A1" lexed as an unknown token) and
ended one it appeared inside ("a´!A1" likewise). By the grammar the table is
generated from it is a plain name character, and its neighbours U+00B3 and
U+00B5 are both taken, so the guard now takes it too.
These pass without the mask fix as well: the bug needs the token's first character to be high, and a bracketed prefix starts at "[", so the faulty `s > 180` test reads false and the loop happens to behave. They pin that the bracketed path stays unaffected, and carry over the coverage from the branch closed in favour of this one.
It explains why the cut-off moved rather than what the code does, which is the PR's job, not the file's. The bound itself needs no gloss: `ALLOWED` is declared `new Uint8Array(180 - OFFS)` a few lines up.
The bracketed-prefix cases do not reach the allow-mask at all: a token opening with "[" is handed straight to lexContextUnquoted, which matches high characters with its own character class, so the name lexer's continuation loop never runs. Say that, rather than the reverse, and say what the cases are for. In the U+00B4 case, "it was in neither" had nothing to refer back to outside the lexer source.
gthb
marked this pull request as ready for review
July 31, 2026 17:18
gthb
changed the title
gthb
added a commit
to gthb/fx
that referenced
this pull request
Jul 31, 2026
"Ærið:Ärger!A1" is one sheet range here and two endpoints in borgar#52, which carries the same mask fix without sheet ranges. Both assertions meet when borgar#52 lands and master merges in, and git reports no conflict because they sit in different parts of the file, so the failure would otherwise read as broken sheet-range lexing. The comment says which one survives.
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.
What
Fix three ways an unquoted name or sheet name containing a non-ASCII character lexes wrongly.
=Ærið!A1came back as onerange_namedtoken spanning the whole string,!and range included, instead of a context, an operator and a range.=Ærið:Ärger!A1went the same way.=Forudsætninger!A1came back asForudsfollowed by a separateætninger!A1.´) is refused outright: it cannot start a name (=´!A1lexed as an unknown token) and it ends one it appears inside (=a´!A1likewise), while its neighbours U+00B3 and U+00B5 are both accepted.Downstream,
parsethrewInvalid syntaxon=SUM(Ærið!A1:B2)andUnexpected unknown tokenon=´!A1, and returnedkind: "name"wherekind: "range"was meant. A writer that emits sheet names with non-ASCII characters got back a formula referencing nothing.Cause
lexNameFuncCntxreads a per-character allow-mask out ofALLOWED, falling back toOK_HIGHCHARfor characters past the table's end. Two separate faults are in that one expression.The continuation loop's
s > 180 ? OK_HIGHCHAR : ALLOWED[c - OFFS]tests the token's first charactersbut indexes with the current characterc. Once a name started with a high character, every character after it took theOK_HIGHCHARbranch —!included — so the loop never stopped. When the name starts with an ASCII character the same expression fails the other way: the lookup runs off the end of the table for a high character, readsundefined, and ends the token there.ALLOWEDhas one entry per code point from U+0020 through U+00B3, and the guard admitted only what is beyond U+00B4, leaving that one character in neither. Every code unit from U+00B4 up is allowed exactly where a high character is — which is whatOK_HIGHCHARencodes — so the boundary belongs at U+00B4, not past it.Fix
Read the mask off
c, and move the cut-off to>= 180in both the loop and the first-character test above it. Each of the three edits has a test that fails without it.Compatibility
Lexing changes only for input containing a code unit at or above U+00B4, and only from a wrong result to a right one: a sweep of every UTF-16 code unit across a dozen syntactic positions found nothing below U+00B4 that changes, and no input that gains an
unknowntoken. Consumers that switch on token type will seerange_namedbecomerange(orcontext+!+range) for such sheet references, and a name ending in a high character arrive as one token rather than two.Relation to the other branches
Supersedes #50 (closed); #51 contains the same two lines among unrelated sheet-range work and drops them once this lands.