Skip to content

Commit

Permalink
fix(sheet): sheet name quote (#4198)
Browse files Browse the repository at this point in the history 
Co-authored-by: GitHub Actions <[email protected]>
  • Loading branch information
@Dushusir @actions-user
Dushusir and actions-user authored Jan 7, 2025
1 parent b413d68 commit f42c01a
Show file tree
Hide file tree
Showing 17 changed files with 457 additions and 29 deletions.
134 changes: 134 additions & 0 deletions e2e/visual-comparison/sheets/sheets-visual-comparison.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -156,3 +156,137 @@ test('diff facade sheet hooks', async () => {
const screenshot = await page.locator(SHEET_MAIN_CANVAS_ID).screenshot();
await expect(screenshot).toMatchSnapshot(filename, { maxDiffPixels: 5 });
});

test('diff set force string cell', async () => {
const browser = await chromium.launch({
headless: !!isCI, // Set to false to see the browser window
});
const context = await browser.newContext({
viewport: { width: 1280, height: 1280 },
deviceScaleFactor: 2, // Set your desired DPR
});
const page = await context.newPage();
await page.goto('http://localhost:3000/sheets/');
await page.waitForTimeout(2000);

await page.evaluate(() => window.E2EControllerAPI.loadDefaultStyleSheet());
await page.waitForTimeout(2000);

await page.evaluate(async () => {
const activeWorkbook = window.univerAPI.getActiveWorkbook();
const activeSheet = activeWorkbook.getActiveSheet();

const sheetId = activeSheet.getSheetId();
const unitId = activeWorkbook.getId();

await window.univerAPI.executeCommand('sheet.operation.set-selections', {
selections: [
{
range: {
startRow: 0,
startColumn: 7,
endRow: 0,
endColumn: 7,
rangeType: 0,
unitId,
sheetId,
},
primary: {
actualRow: 0,
actualColumn: 7,
isMerged: false,
isMergedMainCell: false,
startRow: 0,
startColumn: 7,
endRow: 0,
endColumn: 7,
},
style: {
strokeWidth: 1,
stroke: '#274fee',
fill: 'rgba(39,79,238,0.07)',
widgets: {},
widgetSize: 6,
widgetStrokeWidth: 1,
widgetStroke: '#ffffff',
autofillSize: 6,
autofillStrokeWidth: 1,
autofillStroke: '#ffffff',
rowHeaderFill: 'rgba(39,79,238,0.07)',
rowHeaderStroke: '#274fee',
rowHeaderStrokeWidth: 1,
columnHeaderFill: 'rgba(39,79,238,0.07)',
columnHeaderStroke: '#274fee',
columnHeaderStrokeWidth: 1,
expandCornerSize: 40,
},
},
],
unitId,
subUnitId: sheetId,
type: 2,
});

activeWorkbook.startEditing();
await window.univerAPI.getActiveDocument().appendText("'1");
activeWorkbook.endEditing(true);
});

const filename = generateSnapshotName('set-force-string-cell');
const screenshot = await page.locator(SHEET_MAIN_CANVAS_ID).screenshot();
await expect(screenshot).toMatchSnapshot(filename, { maxDiffPixels: 5 });

await page.waitForTimeout(2000);
await browser.close();
});

test('diff set text format number cell', async () => {
const browser = await chromium.launch({
headless: !!isCI, // Set to false to see the browser window
});
const context = await browser.newContext({
viewport: { width: 1280, height: 1280 },
deviceScaleFactor: 2, // Set your desired DPR
});
const page = await context.newPage();
await page.goto('http://localhost:3000/sheets/');
await page.waitForTimeout(2000);

await page.evaluate(() => window.E2EControllerAPI.loadDefaultStyleSheet());
await page.waitForTimeout(2000);

await page.evaluate(async () => {
await window.univerAPI.executeCommand('sheet.command.numfmt.set.numfmt', {
values: [
{
row: 0,
col: 7,
pattern: '@@@',
type: 'text',
},
],
});

await window.univerAPI.getActiveWorkbook().getActiveSheet().getRange('H1').setValue(2);

await window.univerAPI.getActiveWorkbook().getActiveSheet().getRange('I1').setValue(3);

await window.univerAPI.executeCommand('sheet.command.numfmt.set.numfmt', {
values: [
{
row: 0,
col: 8,
pattern: '@@@',
type: 'text',
},
],
});
});

const filename = generateSnapshotName('set-text-format-number-cell');
const screenshot = await page.locator(SHEET_MAIN_CANVAS_ID).screenshot();
await expect(screenshot).toMatchSnapshot(filename, { maxDiffPixels: 5 });

await page.waitForTimeout(2000);
await browser.close();
});
Binary file added ...visual-comparison.spec.ts-snapshots/set-force-string-cell-ci-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...-comparison.spec.ts-snapshots/set-text-format-number-cell-ci-chromium-linux.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
11 changes: 11 additions & 0 deletions packages/engine-formula/src/engine/analysis/lexer-tree-builder.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1456,7 +1456,18 @@ export class LexerTreeBuilder extends Disposable {
} else {
const nextCurrentString = formulaStringArray[cur + 1];
if (nextCurrentString && nextCurrentString === matchToken.SINGLE_QUOTATION) {
// handle 'Sheet'1'!A1

// Add the first single quotation
this._pushSegment(currentString);
this._addSequenceArray(sequenceArray, currentString, cur);
cur++;

// Add the second single quotation
this._pushSegment(nextCurrentString);
this._addSequenceArray(sequenceArray, nextCurrentString, cur);
cur++;
continue;
} else {
this._closeSingleQuotation();
}
Expand Down
83 changes: 82 additions & 1 deletion packages/engine-formula/src/engine/utils/__tests__/reference.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,19 @@ describe('Test Reference', () => {
});

it('serializeRangeToRefString', () => {
expect(
serializeRangeToRefString({
range: {
startColumn: 0,
endColumn: 10,
startRow: 5,
endRow: 10,
rangeType: RANGE_TYPE.COLUMN,
},
sheetName: '',
unitId: '',
})
).toEqual('A:K');
expect(
serializeRangeToRefString({
range: {
Expand Down Expand Up @@ -140,6 +153,45 @@ describe('Test Reference', () => {
unitId: 'workbook-1',
})
).toEqual("'[workbook-1]sheet1'!16:16");
expect(
serializeRangeToRefString({
range: {
startColumn: Number.NaN,
endColumn: Number.NaN,
startRow: 15,
endRow: 15,
rangeType: RANGE_TYPE.ROW,
},
sheetName: "sheet'1",
unitId: '',
})
).toEqual("'sheet''1'!16:16");
expect(
serializeRangeToRefString({
range: {
startColumn: Number.NaN,
endColumn: Number.NaN,
startRow: 15,
endRow: 15,
rangeType: RANGE_TYPE.ROW,
},
sheetName: "sheet''1",
unitId: '',
})
).toEqual("'sheet''''1'!16:16");
expect(
serializeRangeToRefString({
range: {
startColumn: Number.NaN,
endColumn: Number.NaN,
startRow: 15,
endRow: 15,
rangeType: RANGE_TYPE.ROW,
},
sheetName: "sheet'1",
unitId: 'workbook-1',
})
).toEqual("'[workbook-1]sheet''1'!16:16");
});

it('deserializeRangeWithSheet', () => {
Expand Down Expand Up @@ -259,7 +311,7 @@ describe('Test Reference', () => {
'12a',
'💩a',
'❤️b',
"Sheet'",
"Sheet'1",
'!Sheet',
'！Sheet',
'Sheet1（副本）',
Expand Down Expand Up @@ -305,11 +357,40 @@ describe('Test Reference', () => {
unitId: '',
});

// with single quote
expect(handleRefStringInfo("'sheet''1'!A1")).toStrictEqual({
refBody: 'A1',
sheetName: "sheet'1",
unitId: '',
});

// with double quote
expect(handleRefStringInfo("'sheet''''1'!A1")).toStrictEqual({
refBody: 'A1',
sheetName: "sheet''1",
unitId: '',
});

expect(handleRefStringInfo("'[Book-1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: 'Book-1.xlsx',
});

// with single quote
expect(handleRefStringInfo("'[Book''1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: "Book'1.xlsx",
});

// with double quote
expect(handleRefStringInfo("'[Book''''1.xlsx]Sheet1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'Sheet1',
unitId: "Book''1.xlsx",
});

expect(handleRefStringInfo("'[Book-1.xlsx]sheet-1'!$A$4")).toStrictEqual({
refBody: '$A$4',
sheetName: 'sheet-1',
Expand Down
36 changes: 30 additions & 6 deletions packages/engine-formula/src/engine/utils/reference.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,7 @@ export function serializeRange(range: IRange): string {
* @param range
*/
export function serializeRangeWithSheet(sheetName: string, range: IRange): string {
if (needsQuoting(sheetName)) {
return `'${sheetName}'!${serializeRange(range)}`;
}
return `${sheetName}!${serializeRange(range)}`;
return `${addQuotesBothSides(sheetName)}!${serializeRange(range)}`;
}

/**
Expand All @@ -167,7 +164,7 @@ export function serializeRangeWithSheet(sheetName: string, range: IRange): strin
*/
export function serializeRangeWithSpreadsheet(unit: string, sheetName: string, range: IRange): string {
if (needsQuoting(unit) || needsQuoting(sheetName)) {
return `'[${unit}]${sheetName}'!${serializeRange(range)}`;
return `'[${quoteSheetName(unit)}]${quoteSheetName(sheetName)}'!${serializeRange(range)}`;
}

return `[${unit}]${sheetName}!${serializeRange(range)}`;
Expand Down Expand Up @@ -206,7 +203,7 @@ export function handleRefStringInfo(refString: string) {

if (unitIdMatch != null) {
unitId = unitIdMatch[0].trim();
unitId = unitId.slice(1, unitId.length - 1);
unitId = unquoteSheetName(unitId.slice(1, unitId.length - 1));
refString = refString.replace(UNIT_NAME_REGEX_PRECOMPILING, '');
}

Expand All @@ -218,6 +215,8 @@ export function handleRefStringInfo(refString: string) {
if (sheetName[0] === "'" && sheetName[sheetName.length - 1] === "'") {
sheetName = sheetName.substring(1, sheetName.length - 1);
}

sheetName = unquoteSheetName(sheetName);
refBody = refString.substring(sheetNameIndex + 1);
} else {
refBody = refString;
Expand Down Expand Up @@ -426,6 +425,31 @@ export function needsQuoting(name: string) {
return false;
}

/**
* Add quotes to the sheet name
*/
export function addQuotesBothSides(name: string) {
return needsQuoting(name) ? `'${quoteSheetName(name)}'` : name;
}

/**
* Add a single quote before the single quote
* @param name
* @returns Quoted name
*/
function quoteSheetName(name: string) {
return name.replace(/'/g, "''");
}

/**
* Replace double single quotes with single quotes
* @param name
* @returns Unquoted name
*/
function unquoteSheetName(name: string) {
return name.replace(/''/g, "'");
}

function isA1Notation(name: string) {
const match = name.match(/[1-9][0-9]{0,6}/);
// Excel has a limit on the number of rows and columns: targetRow > 1048576 || targetColumn > 16384, Univer has no limit
Expand Down
8 changes: 6 additions & 2 deletions packages/engine-formula/src/functions/lookup/address/__tests__/index.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
import { describe, expect, it } from 'vitest';

import { ErrorType } from '../../../../basics/error-type';
import { ArrayValueObject, transformToValue, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { ErrorValueObject } from '../../../../engine/value-object/base-value-object';
import {
BooleanValueObject,
NumberValueObject,
StringValueObject,
} from '../../../../engine/value-object/primitive-object';
import { getObjectValue } from '../../../__tests__/create-function-test-bed';
import { FUNCTION_NAMES_LOOKUP } from '../../function-names';
import { Address } from '../index';
import { ArrayValueObject, transformToValue, transformToValueObject } from '../../../../engine/value-object/array-value-object';
import { getObjectValue } from '../../../__tests__/create-function-test-bed';

describe('Test address', () => {
const testFunction = new Address(FUNCTION_NAMES_LOOKUP.ADDRESS);
Expand Down Expand Up @@ -65,6 +65,10 @@ describe('Test address', () => {
expect(calculate(2, 3, 1, false, '[Book1]Sheet1')).toBe("'[Book1]Sheet1'!R2C3");
});

it('Absolute reference to sheet name with single quote', async () => {
expect(calculate(1, 1, 1, true, "Sheet'1")).toBe("'Sheet''1'!$A$1");
});

it('Absolute reference to another worksheet', async () => {
expect(calculate(2, 3, 1, false, 'EXCEL SHEET')).toBe("'EXCEL SHEET'!R2C3");
});
Expand Down
Loading

0 comments on commit f42c01a

Please sign in to comment.