From 616ed86f1f2367da8c26ff1fc653e2df8dd5ad57 Mon Sep 17 00:00:00 2001 From: Gunnlaugur Thor Briem Date: Wed, 2 Sep 2026 17:36:37 +0000 Subject: [PATCH 1/4] fix: a quoted prefix right of a range operator loses its quotes 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. --- docs/Prefixes.md | 6 ++++++ lib/fixRanges.spec.ts | 11 +++++++++-- lib/fixRanges.ts | 17 +++++++++++++++++ lib/stringifyPrefix.ts | 18 +++++++++++++++++- lib/translateToA1.spec.ts | 21 +++++++++++++++++++++ lib/translateToA1.ts | 35 +++++++++++++++++++++-------------- 6 files changed, 91 insertions(+), 17 deletions(-) diff --git a/docs/Prefixes.md b/docs/Prefixes.md index 596b894..55797bc 100644 --- a/docs/Prefixes.md +++ b/docs/Prefixes.md @@ -86,3 +86,9 @@ parseA1Ref('[1]!A1', { xlsx: true }); Inversely, when serializing a reference object, _Fx_ expects the `workbookName` and `sheetName` properties to dictate how to compose the prefix. When the sheet scope is a "3D" sheet range (`Sheet1:Sheet2!A1`) the scope will not be split but emitted as it was found (`{ sheetName: 'Sheet1:Sheet2' }`). + +## The right side of a colon + +The quotes after a colon decide what the colon means. `Q1:'Sales'!A1` is the name `Q1` joined to the reference `'Sales'!A1`; `Sales` is quoted even though the name itself needs no quotes, to set it apart from `Q1:Sales!A1`, the 3-D reference over the sheets `Q1` to `Sales`. Excel stores that position quoted whatever the name, `Sheet1!A1:Sheet2!B2` as `Sheet1!A1:'Sheet2'!B2`, even where the left side could not begin a sheet range. + +The two _Fx_ functions treat it differently. `fixRanges` keeps the quotes a prefix has in that position and adds none, so it never rewrites a reference into one naming different sheets. `translateToA1` quotes every such prefix, as Excel's serialization does. diff --git a/lib/fixRanges.spec.ts b/lib/fixRanges.spec.ts index e5c01c6..f8301b2 100644 --- a/lib/fixRanges.spec.ts +++ b/lib/fixRanges.spec.ts @@ -117,7 +117,7 @@ describe('fixRanges prefixes', () => { isFixed("='foo':bar!A1", '=foo:bar!A1', { xlsx: true }); }); - test('a quote around the second end is the range operator, not a 3D reference', () => { + test('a quote around the RHS makes the colon a range operator, not a sheet span marker', () => { isFixed("=foo:'bar baz'!A1", "=foo:'bar baz'!A1"); // Excel fixes this to `=foo:'bar baz'!A1` isFixed("='foo':'bar baz'!A1", "='foo:bar baz'!A1"); @@ -125,7 +125,7 @@ describe('fixRanges prefixes', () => { isFixed("=Jan:'[1]Nope'!A1", '=Jan:[1]Nope!A1'); }); - test('a digit-leading second sheet name goes to the range operator', () => { + test('a digit-leading RHS makes the colon a range operator, not a sheet span marker', () => { isFixed('=SUM(Sheet1:1!A1)', "=SUM('Sheet1:1'!A1)"); isFixed('=SUM(X:1!A1)', "=SUM('X:1'!A1)"); isFixed("=SUM(Sheet1:'1'!A1)", "=SUM(Sheet1:'1'!A1)"); @@ -134,6 +134,13 @@ describe('fixRanges prefixes', () => { isFixed('=SUM([Book.xlsx]1:3!A1)', "=SUM('[Book.xlsx]1:3'!A1)"); }); + test('a quoted RHS keeps quotes its own name would not call for', () => { + // Sales needs no quotes of its own, but dropping them here would emit Q1:Sales!A1, which + // reads back as the sheet range Q1:Sales rather than as the name joined to 'Sales'!A1. + isFixed("=SUM(Q1:'Sales'!A1)", "=SUM(Q1:'Sales'!A1)"); + isFixed("=SUM(A1:'Sales'!B2)", "=SUM(A1:'Sales'!B2)"); + }); + test('a left side that is also a cell address wins over a 3D reference', () => { isFixed('=SUM(A1:B2!C3)', "=SUM(A1:'B2'!C3)"); isFixed("=SUM('A1:B2'!C3)", "=SUM('A1:B2'!C3)"); diff --git a/lib/fixRanges.ts b/lib/fixRanges.ts index d010af9..467d70b 100644 --- a/lib/fixRanges.ts +++ b/lib/fixRanges.ts @@ -9,6 +9,7 @@ import { REF_STRUCT } from './constants.ts'; import type { ReferenceA1, ReferenceA1Xlsx, Token } from './types.ts'; import { cloneToken } from './cloneToken.ts'; import { stringifyTokens } from './stringifyTokens.ts'; +import { followsRangeOperator } from './stringifyPrefix.ts'; // There is no R1C1 counterpart to this. This is because without an anchor cell // it is impossible to determine if a relative+absolute range (R[1]C[1]:R5C5) @@ -31,6 +32,15 @@ export type OptsFixRanges = { thisRow?: boolean, }; +// Would dropping this operand's quotes let it read as a sheet range? Only for a quoted prefix +// with no workbook bracket: a bracket can only lead a whole prefix, so Jan:'[1]Nope'!A1 is not a +// sheet range either way and its quotes are redundant. +function keepsSourceQuotes (value: string): boolean { + if (!value.startsWith("'")) { return false; } + const bang = value.indexOf('!'); + return bang !== -1 && !value.slice(0, bang).includes('['); +} + /** * Normalizes A1 style ranges and structured references in a list of tokens. * @@ -88,6 +98,13 @@ export function fixTokenRanges ( token.value = newValue; } else if (isRange(token)) { + // After a range operator, the quotes on a prefix keep it from reading as part of a sheet + // range, and Excel stores range operations quoted there. Quotes in that position must be + // kept, whether or not the sheet name itself needs them. + if (followsRangeOperator(output, output.length) && keepsSourceQuotes(token.value)) { + output.push(token); + continue; + } const ref = parseA1Ref(token.value, { allowTernary: true }) as ReferenceA1; const range = ref.range; // fill missing dimensions? diff --git a/lib/stringifyPrefix.ts b/lib/stringifyPrefix.ts index 06e8d15..cf60366 100644 --- a/lib/stringifyPrefix.ts +++ b/lib/stringifyPrefix.ts @@ -6,8 +6,10 @@ import type { ReferenceStructXlsx, ReferenceR1C1Xlsx, ReferenceName, - ReferenceNameXlsx + ReferenceNameXlsx, + Token } from './types.ts'; +import { OPERATOR, WHITESPACE, NEWLINE } from './constants.ts'; const CHAR_PERIOD = 46; const CHAR_0 = 48; @@ -19,6 +21,20 @@ const reIsRangelike = /^(R\d{0,7}|C\d{0,5}|R\d{0,7}C\d{0,5}|[A-Z]{1,3}\d{1,7})$/ const reIsBoolean = /^(TRUE|FALSE)$/i; const reIsLinkIndex = /^\d+$/; +const rangeOperators = new Set([ ':', '.:', ':.', '.:.' ]); + +// Is the token at `index` the right operand of a range operator? Looks back past any +// whitespace/newline, since Excel allows spacing around the range colon +// (`A1 : Jan:Mar!B2`). +export function followsRangeOperator (tokens: Token[], index: number): boolean { + for (let i = index - 1; i >= 0; i--) { + const t = tokens[i]; + if (t.type === WHITESPACE || t.type === NEWLINE) { continue; } + return t.type === OPERATOR && rangeOperators.has(t.value); + } + return false; +} + export function needQuotes (scope: string, blockSheetRanges: boolean, bracketed: boolean, checkColon = false): number { if (scope) { if (checkColon && scope.includes(':')) { diff --git a/lib/translateToA1.spec.ts b/lib/translateToA1.spec.ts index e75b5bd..a813c28 100644 --- a/lib/translateToA1.spec.ts +++ b/lib/translateToA1.spec.ts @@ -262,6 +262,27 @@ describe('quote sheet prefix on RHS of range operator', () => { test('already-quoted RHS prefix is not double-quoted', () => { isR2A("='Sheet 1'!R1C1:'Sheet 1'!R2C2", 'A1', "='Sheet 1'!$A$1:'Sheet 1'!$B$2"); }); + + test('a prefix behind whitespace is still the RHS of the range operator', () => { + // Excel allows spacing around the range colon. + isR2A('=B!R1C1 : B!R2C2', 'A1', "=B!$A$1 : 'B'!$B$2"); + isR2A('=B!R1C1 :B!R2C2', 'A1', "=B!$A$1 :'B'!$B$2"); + }); + + test('quoting a prefix carries a location skew forward to later tokens', () => { + const tokens = tokenizeXlsx('=SUM(R1C1:Sheet1!R2C2,R9C1)', { r1c1: true, withLocation: true }); + expect(translateTokensToA1(tokens, 'A1')).toEqual([ + { type: FX_PREFIX, value: '=', loc: [ 0, 1 ] }, + { type: FUNCTION, value: 'SUM', loc: [ 1, 4 ] }, + { type: OPERATOR, value: '(', loc: [ 4, 5 ] }, + { type: REF_RANGE, value: '$A$1', loc: [ 5, 9 ] }, + { type: OPERATOR, value: ':', loc: [ 9, 10 ] }, + { type: REF_RANGE, value: "'Sheet1'!$B$2", loc: [ 10, 23 ] }, + { type: OPERATOR, value: ',', loc: [ 23, 24 ] }, + { type: REF_RANGE, value: '$A$9', loc: [ 24, 28 ] }, + { type: OPERATOR, value: ')', loc: [ 28, 29 ] } + ]); + }); }); describe('translate 3D references', () => { diff --git a/lib/translateToA1.ts b/lib/translateToA1.ts index 870151f..b87b0d1 100644 --- a/lib/translateToA1.ts +++ b/lib/translateToA1.ts @@ -7,8 +7,7 @@ import { parseA1Range } from './parseA1Range.ts'; import type { RangeA1, ReferenceR1C1Xlsx, Token } from './types.ts'; import { stringifyTokens } from './stringifyTokens.ts'; import { cloneToken } from './cloneToken.ts'; -import { OPERATOR } from './constants.ts'; -import { quotePrefix } from './stringifyPrefix.ts'; +import { quotePrefix, followsRangeOperator } from './stringifyPrefix.ts'; // Turn on the most permissive setting when parsing ranges so we don't have to think about // this option. We already know that range tokens are legal, so we're not going to encounter @@ -168,24 +167,32 @@ export function translateTokensToA1 ( // Excel unconditionally quotes the sheet prefix on the RHS of a range // operator in XLSX files: Sheet1!A1:Sheet1!B2 → Sheet1!A1:'Sheet1'!B2. - // Apply the same quoting to match Excel's serialization. - for (let i = 2; i < outTokens.length; i++) { - const tok = outTokens[i]; - if (!isRange(tok)) { - continue; + // Apply the same quoting to match Excel's serialization. Quoting lengthens + // the token, so this carries its own skew forward, same as the loop above. + let quoteSkew = 0; + for (let i = 0; i < outTokens.length; i++) { + let tok = outTokens[i]; + if (quoteSkew && tok.loc) { + tok = cloneToken(tok); + tok.loc[0] += quoteSkew; + tok.loc[1] += quoteSkew; + outTokens[i] = tok; } - const prev = outTokens[i - 1]; - if (prev?.type !== OPERATOR || prev.value !== ':') { + if (!isRange(tok) || !followsRangeOperator(outTokens, i)) { continue; } const bangIdx = tok.value.indexOf('!'); - if (bangIdx > 0) { + if (bangIdx > 0 && tok.value[0] !== "'") { const prefix = tok.value.slice(0, bangIdx); - // Only quote if not already quoted - if (prefix[0] !== "'") { - outTokens[i] = cloneToken(tok); - outTokens[i].value = quotePrefix(prefix) + tok.value.slice(bangIdx); + const oldLength = tok.value.length; + tok = cloneToken(tok); + tok.value = quotePrefix(prefix) + tok.value.slice(bangIdx); + const delta = tok.value.length - oldLength; + if (tok.loc) { + tok.loc[1] += delta; } + quoteSkew += delta; + outTokens[i] = tok; } } From 89e3179896fae09df19098b5d9269b531663f93e Mon Sep 17 00:00:00 2001 From: Gunnlaugur Thor Briem Date: Thu, 3 Sep 2026 09:41:14 +0000 Subject: [PATCH 2/4] test: place the new spec test so it does not collide with #63 #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. --- lib/fixRanges.spec.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/fixRanges.spec.ts b/lib/fixRanges.spec.ts index f8301b2..ea38fbb 100644 --- a/lib/fixRanges.spec.ts +++ b/lib/fixRanges.spec.ts @@ -125,7 +125,14 @@ describe('fixRanges prefixes', () => { isFixed("=Jan:'[1]Nope'!A1", '=Jan:[1]Nope!A1'); }); - test('a digit-leading RHS makes the colon a range operator, not a sheet span marker', () => { + test('a quoted RHS keeps quotes its own name would not call for', () => { + // Sales needs no quotes of its own, but dropping them here would emit Q1:Sales!A1, which + // reads back as the sheet range Q1:Sales rather than as the name joined to 'Sales'!A1. + isFixed("=SUM(Q1:'Sales'!A1)", "=SUM(Q1:'Sales'!A1)"); + isFixed("=SUM(A1:'Sales'!B2)", "=SUM(A1:'Sales'!B2)"); + }); + + test('a digit-leading second sheet name goes to the range operator', () => { isFixed('=SUM(Sheet1:1!A1)', "=SUM('Sheet1:1'!A1)"); isFixed('=SUM(X:1!A1)', "=SUM('X:1'!A1)"); isFixed("=SUM(Sheet1:'1'!A1)", "=SUM(Sheet1:'1'!A1)"); @@ -134,13 +141,6 @@ describe('fixRanges prefixes', () => { isFixed('=SUM([Book.xlsx]1:3!A1)', "=SUM('[Book.xlsx]1:3'!A1)"); }); - test('a quoted RHS keeps quotes its own name would not call for', () => { - // Sales needs no quotes of its own, but dropping them here would emit Q1:Sales!A1, which - // reads back as the sheet range Q1:Sales rather than as the name joined to 'Sales'!A1. - isFixed("=SUM(Q1:'Sales'!A1)", "=SUM(Q1:'Sales'!A1)"); - isFixed("=SUM(A1:'Sales'!B2)", "=SUM(A1:'Sales'!B2)"); - }); - test('a left side that is also a cell address wins over a 3D reference', () => { isFixed('=SUM(A1:B2!C3)', "=SUM(A1:'B2'!C3)"); isFixed("=SUM('A1:B2'!C3)", "=SUM('A1:B2'!C3)"); From 9955052c1430526169a426444fd528955a2bf12a Mon Sep 17 00:00:00 2001 From: Gunnlaugur Thor Briem Date: Thu, 3 Sep 2026 12:54:41 +0000 Subject: [PATCH 3/4] Use an example that shows what the quotes actually decide MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- docs/Prefixes.md | 2 +- lib/fixRanges.spec.ts | 9 +++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/Prefixes.md b/docs/Prefixes.md index 55797bc..d9dcbf0 100644 --- a/docs/Prefixes.md +++ b/docs/Prefixes.md @@ -89,6 +89,6 @@ When the sheet scope is a "3D" sheet range (`Sheet1:Sheet2!A1`) the scope will n ## The right side of a colon -The quotes after a colon decide what the colon means. `Q1:'Sales'!A1` is the name `Q1` joined to the reference `'Sales'!A1`; `Sales` is quoted even though the name itself needs no quotes, to set it apart from `Q1:Sales!A1`, the 3-D reference over the sheets `Q1` to `Sales`. Excel stores that position quoted whatever the name, `Sheet1!A1:Sheet2!B2` as `Sheet1!A1:'Sheet2'!B2`, even where the left side could not begin a sheet range. +The quotes after a colon decide what the colon means. `foo:'Gamma'!A1` is the name `foo` joined to the reference `'Gamma'!A1`; `Gamma` is quoted even though the name itself needs no quotes, to set it apart from `foo:Gamma!A1`, the 3-D reference over the sheets `foo` to `Gamma`. Excel stores that position quoted whatever the name, `Sheet1!A1:Sheet2!B2` as `Sheet1!A1:'Sheet2'!B2`, even where the left side could not begin a sheet range — a left side that reads as a cell address, `Q1` or `A1`, is that cell rather than a sheet, so there the quotes change no reading and Excel writes them anyway. The two _Fx_ functions treat it differently. `fixRanges` keeps the quotes a prefix has in that position and adds none, so it never rewrites a reference into one naming different sheets. `translateToA1` quotes every such prefix, as Excel's serialization does. diff --git a/lib/fixRanges.spec.ts b/lib/fixRanges.spec.ts index ea38fbb..111dc7b 100644 --- a/lib/fixRanges.spec.ts +++ b/lib/fixRanges.spec.ts @@ -126,8 +126,13 @@ describe('fixRanges prefixes', () => { }); test('a quoted RHS keeps quotes its own name would not call for', () => { - // Sales needs no quotes of its own, but dropping them here would emit Q1:Sales!A1, which - // reads back as the sheet range Q1:Sales rather than as the name joined to 'Sales'!A1. + // The quote around the right endpoint is what makes the colon a range operator: Excel reads + // `foo:'Gamma'!A1` as the name `foo` joined to `'Gamma'!A1`, and `foo:Gamma!A1` as the sheet + // range `foo:Gamma`. So dropping a quote the right name does not need for its own sake + // changes which sheets the formula reads. + isFixed("=SUM(foo:'Gamma'!A1)", "=SUM(foo:'Gamma'!A1)"); + // With a cell address on the left the reading is the same either way, since the cell wins + // over the sheet range; Excel still stores the quote, and takes it off nothing here. isFixed("=SUM(Q1:'Sales'!A1)", "=SUM(Q1:'Sales'!A1)"); isFixed("=SUM(A1:'Sales'!B2)", "=SUM(A1:'Sales'!B2)"); }); From 717789dfdcc203dc33c459443b2104bfd8010c0d Mon Sep 17 00:00:00 2001 From: Gunnlaugur Thor Briem Date: Thu, 3 Sep 2026 13:05:39 +0000 Subject: [PATCH 4/4] Shorter, clearer comments --- lib/fixRanges.spec.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/fixRanges.spec.ts b/lib/fixRanges.spec.ts index 111dc7b..fd5af23 100644 --- a/lib/fixRanges.spec.ts +++ b/lib/fixRanges.spec.ts @@ -126,13 +126,10 @@ describe('fixRanges prefixes', () => { }); test('a quoted RHS keeps quotes its own name would not call for', () => { - // The quote around the right endpoint is what makes the colon a range operator: Excel reads - // `foo:'Gamma'!A1` as the name `foo` joined to `'Gamma'!A1`, and `foo:Gamma!A1` as the sheet - // range `foo:Gamma`. So dropping a quote the right name does not need for its own sake - // changes which sheets the formula reads. + // The quote around the right endpoint makes the colon a range operator. So even if the right + // name doesn't need quoting, the quotes are kept, else the meaning would change. isFixed("=SUM(foo:'Gamma'!A1)", "=SUM(foo:'Gamma'!A1)"); - // With a cell address on the left the reading is the same either way, since the cell wins - // over the sheet range; Excel still stores the quote, and takes it off nothing here. + // A cell address on the left side rules out the sheet range interpretation isFixed("=SUM(Q1:'Sales'!A1)", "=SUM(Q1:'Sales'!A1)"); isFixed("=SUM(A1:'Sales'!B2)", "=SUM(A1:'Sales'!B2)"); });