Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions lib/fixRanges.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,6 @@ describe('fixRanges prefixes', () => {
isFixed('=SUM(A:C!A1)', "=SUM('A:C'!A1)");
isFixed('=SUM(B:C!A1)', "=SUM('B:C'!A1)");
isFixed('=SUM(C:D!A1)', "=SUM('C:D'!A1)");
isFixed('=SUM(A:8!A1)', "=SUM('A:8'!A1)");
isFixed('=SUM(B:8!A1)', "=SUM('B:8'!A1)");
isFixed('=SUM(8:D!A1)', "=SUM('8:D'!A1)");
isFixed('=SUM(10:23!A1)', "=SUM('10:23'!A1)");
isFixed('=A:B!A1', '=A:B!A1');
Expand Down Expand Up @@ -126,12 +124,27 @@ describe('fixRanges prefixes', () => {
});

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)");
// Excel fixes Sheet1:1!A1 by quoting the RHS, choosing the latter of these interpretations:
// - 'Sheet1:1'!A1 is a 3-D reference spanning the sheets Sheet1 to 1
// - Sheet1:'1'!A1 is a range from (misleadingly named) _defined name_ Sheet1 to cell '1'!A1
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)");
isFixed("=SUM(Jan:'2020plan'!A1)", "=SUM(Jan:'2020plan'!A1)");
isFixed('=SUM([Book.xlsx]Alpha:3!A1)', "=SUM([Book.xlsx]Alpha:'3'!A1)");
// A number cannot be the left operand of the range operator, so these are sheet ranges.
isFixed('=SUM(1:5!A1)', "=SUM('1:5'!A1)");
isFixed("=SUM('1:5'!A1)", "=SUM('1:5'!A1)");
isFixed('=SUM([Book.xlsx]1:3!A1)', "=SUM('[Book.xlsx]1:3'!A1)");
isFixed('=SUM(Sheet1:1!A1)', "=SUM(Sheet1:'1'!A1)", { xlsx: true });
isFixed('=SUM(1:5!A1)', "=SUM('1:5'!A1)", { xlsx: true });
// Quoting the whole prefix makes a sheet range, regardless of what the right name looks like.
isFixed("=SUM('Sheet1:1'!A1)", "=SUM('Sheet1:1'!A1)");
});

test.fails('a digit-leading second sheet name goes to the range operator even when not all digits', () => {
// Fails today because 2020plan is lexed as the number 2020 followed by the name plan.
Comment on lines +145 to +146

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Flips to passing when #62 is merged.

Suggested change
test.fails('a digit-leading second sheet name goes to the range operator even when not all digits', () => {
// Fails today because 2020plan is lexed as the number 2020 followed by the name plan.
test('a digit-leading second sheet name goes to the range operator even when not all digits', () => {

isFixed('=SUM(Jan:2020plan!A1)', "=SUM(Jan:'2020plan'!A1)");
});

test('a left side that is also a cell address wins over a 3D reference', () => {
Expand Down
31 changes: 31 additions & 0 deletions lib/mergeRefTokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ const BR_OPEN = 91; // [
const BR_CLOSE = 93; // ]
const COLON = 58;

// In an unquoted pair of sheet names, a digit-led right name makes the colon a range operator
// rather than marking a sheet range, unless the left name is a number, which cannot be the range
// operator's left operand.
const reDigitLed = /^\d/;
const reAllDigits = /^\d+$/;

// The sheet name in an unquoted prefix: what follows its [workbook] and any path before that.
function sheetNameOf (unquotedPrefix: string): string {
const close = unquotedPrefix.lastIndexOf(']');
return close === -1 ? unquotedPrefix : unquotedPrefix.slice(close + 1);
}

const validRunsMerge = [
// A1 | A1:B2 | A:B | 1:2 | A1:B
[ REF_CELL, ':', REF_CELL ],
Expand Down Expand Up @@ -93,6 +105,7 @@ const matcher = (
let cols = 0;
let brackets = 0;
let inPrefix = false;
let rightSheetName: string | undefined;
// the longest run so far that ended on a terminal: a longer run may turn out
// to be invalid, in which case we fall back to the best valid subset run
let best = 0;
Expand All @@ -109,6 +122,24 @@ const matcher = (
const value = token.value;
if (inPrefix) {
const firstChar = value.charCodeAt(0);
if (
firstChar !== COLON &&
(token.type === CONTEXT || token.type === CONTEXT_QUOTE || token.type === REF_NAMED)
) {
const name = sheetNameOf(token.type === CONTEXT_QUOTE ? unquotePrefix(value) : value);
if (cols === 0) {
rightSheetName = name;
}
// This is entered only if an earlier token set cols to 1, so never for a quoted sheet span
else if (
cols === 1 &&
rightSheetName != null &&
reDigitLed.test(rightSheetName) &&
!reAllDigits.test(name)
) {
break runloop;
}
}
if (firstChar === COLON) {
cols++;
if (cols >= 2 || brackets) { break runloop; }
Expand Down
23 changes: 17 additions & 6 deletions lib/parseA1Ref.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,17 +169,28 @@ describe('parse A1 references', () => {
});

test('a period/digit-leading sheet names', () => {
// These aren't strictly legal, as sheets that begin with digits or periods must
// be quoted. But Excel will parse and convert them to quoted, so we do to.
// A sheet name that begins with a digit must be quoted. At the right end of an unquoted pair
// Excel quotes that name alone, which leaves the colon to the range operator.
const range = { top: 0, left: 0, bottom: 0, right: 0 };
isA1Equal('Sheet1:1!A1', { context: [ 'Sheet1:1' ], range });
isA1Equal('Jan:2020plan!A1', { context: [ 'Jan:2020plan' ], range });
isA1Equal('X:1!A1', { context: [ 'X:1' ], range });
isA1Equal('[Book.xlsx]Alpha:3!A1', { context: [ 'Book.xlsx', 'Alpha:3' ], range });
// Unquoted, a digit-led second name makes the colon a range operator rather than a sheet range
// marker, so these are not a single reference.
expect(parseA1Ref('Sheet1:1!A1')).toBe(undefined);
expect(parseA1Ref('Jan:2020plan!A1')).toBe(undefined);
expect(parseA1Ref('X:1!A1')).toBe(undefined);
expect(parseA1Ref('[Book.xlsx]Alpha:3!A1')).toBe(undefined);
// A number cannot be the left operand of the range operator, so these are sheet ranges.
isA1Equal('1:5!A1', { context: [ '1:5' ], range });
isA1Equal('12:15!A1', { context: [ '12:15' ], range });
isA1Equal('1:2020plan!A1', { context: [ '1:2020plan' ], range });
isA1Equal('[Book.xlsx]1:3!A1', { context: [ 'Book.xlsx', '1:3' ], range });
// the same rule in the xlsx variant
isA1Equal('Sheet1:1!A1', undefined, { xlsx: true });
isA1Equal('Jan:2020plan!A1', undefined, { xlsx: true });
isA1Equal('[1]Alpha:3!A1', undefined, { xlsx: true });
isA1Equal('1:5!A1', { sheetName: '1:5', range }, { xlsx: true });
isA1Equal('[1]1:3!A1', { workbookName: '1', sheetName: '1:3', range }, { xlsx: true });
// Quoting the whole prefix makes a sheet range whatever the right name is.
isA1Equal("'Sheet1:1'!A1", { context: [ 'Sheet1:1' ], range });
});

test('a left side that is also a cell address wins over a sheet range', () => {
Expand Down
7 changes: 7 additions & 0 deletions lib/parseRef.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,13 @@ const pExtendedContext: RefParserPart = (t, data, xlsx, r1c1, tokens) => {
const d: Partial<RefParseDataCtx & RefParseDataXls> = {};
const value = type === CONTEXT_QUOTE ? unquotePrefix(t.value) : t.value;
splitContext(value, d, xlsx);
// A digit-led second sheet name makes the colon a range operator, not a sheet range marker,
// unless the first name is an integer, which cannot be the range operator's left operand.
const second = xlsx ? d.sheetName : d.context?.[0];
const first = xlsx ? data.sheetName : data.context?.at(-1);
if (second && /^\d/.test(second) && !/^\d+$/.test(first ?? '')) {
return;
}
if (xlsx && d.sheetName && !d.workbookName) {
data.sheetName += ':' + d.sheetName;
return 1;
Expand Down