Fix non-ASCII characters breaking unquoted names and sheet names - #52
Open
gthb wants to merge 5 commits into
Open
Fix non-ASCII characters breaking unquoted names and sheet 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
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.
gthb
added a commit
to gthb/fx
that referenced
this pull request
Aug 5, 2026
- "silently" said what the surrounding sentence already said, and
dramatized it: a lookup that "matches no sheet at all" is quiet by
construction, and "no `undefined` to signal it" is the whole point of
the raw-quoted-prefix warning. Dropped, or replaced with what actually
happens ("does not fail at all").
- "carries" for plain possession, in the borgar#52 cross-reference.
- "spelling" where it was not doing the work it does elsewhere in this
branch, which is to distinguish two ways of writing one reference. In
the generated-input module the subject is cases and forms, not
spellings.
Also unpicked one garden-path sentence in docs/Prefixes.md: "a name Excel
quotes standing alone quotes the whole range" now reads "a name that Excel
quotes when it stands alone forces quotes on the whole range".
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 three ways an unquoted name or sheet name lexes wrongly when it contains a character at or above U+00B4 (
´), where the lexer's table of per-character allow-masks ends. The few non-ASCII characters below that, U+00A1 through U+00B3, already lexed correctly.=Ærið!A1lexed as a singlerange_namedtoken holdingÆrið!A1, a defined name, rather than as a sheet prefix, a!and a range.=Ærið:Ärger!A1went the same way.=Forudsætninger!A1came back asForudsfollowed by a separateætninger!A1.´) was refused outright, though U+00B3 just below it and U+00B5 just above it are both legal in a name: it could not start one (=´!A1lexed as an unknown token) and it ended one that it stood inside (=a´!A1likewise came back as an unknown token).Downstream,
parsethrewInvalid syntaxon=SUM(Ærið!A1:B2)andUnexpected unknown tokenon=´!A1, and gave=Ærið!A1aReferenceIdentifierwithkind: 'name'wherekind: 'range'was meant, leaving a consumer to look up a defined name calledÆrið!A1.Cause
lexNameFuncCntxreads a per-character allow-mask out ofALLOWED, falling back toOK_HIGHCHARfor characters past the table's end. That one expression has two separate faults.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. Where the first character was not a high one, the same expression failed the other way: the lookup ran off the end of the table for a high character, readundefined, and ended the token there.ALLOWEDhas one entry per code point from U+0020 through U+00B3, and the guard admitted only code units above U+00B4, leaving U+00B4 itself in neither. It is allowed in exactly the positionsOK_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: three changes over two lines, each pinned by a test that fails without it.Compatibility
Lexing changes only for input containing a code unit at or above U+00B4, which is now treated uniformly as a name and context character, as the character classes commented beside the code call for. 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 with a high character in it arrive as one token rather than two.