diff --git a/src/constants.ts b/src/constants.ts index 729d6e2bef..9a350886b9 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -245,8 +245,8 @@ export const DEBOUNCE_TIME = 200; export const MESSAGE_VERSION = 1; // Sheets -export const FORBIDDEN_SHEET_CHARS = ["'", "*", "?", "/", "\\", "[", "]"] as const; -export const FORBIDDEN_IN_EXCEL_REGEX = /'|\*|\?|\/|\\|\[|\]/; +export const FORBIDDEN_SHEETNAME_CHARS = ["'", "*", "?", "/", "\\", "[", "]"] as const; +export const FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX = /'|\*|\?|\/|\\|\[|\]/; // Cells export const FORMULA_REF_IDENTIFIER = "|"; diff --git a/src/helpers/misc.ts b/src/helpers/misc.ts index 94de609b80..396c04d75b 100644 --- a/src/helpers/misc.ts +++ b/src/helpers/misc.ts @@ -1,11 +1,13 @@ //------------------------------------------------------------------------------ // Miscellaneous //------------------------------------------------------------------------------ -import { NEWLINE } from "../constants"; +import { FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX, NEWLINE } from "../constants"; import { ConsecutiveIndexes, Lazy, UID } from "../types"; import { SearchOptions } from "../types/find_and_replace"; import { Cloneable, DebouncedFunction } from "./../types/misc"; +const sanitizeSheetNameRegex = new RegExp(FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX, "g"); + /** * Remove quotes from a quoted string * ```js @@ -109,6 +111,11 @@ export function getCanonicalSymbolName(symbolName: string): string { return symbolName; } +/** Replace the excel-excluded characters of a sheetName */ +export function sanitizeSheetName(sheetName: string, replacementChar: string = " "): string { + return sheetName.replace(sanitizeSheetNameRegex, replacementChar); +} + export function clip(val: number, min: number, max: number): number { return val < min ? min : val > max ? max : val; } diff --git a/src/helpers/ui/sheet_interactive.ts b/src/helpers/ui/sheet_interactive.ts index 7f288ab133..6c2a856b45 100644 --- a/src/helpers/ui/sheet_interactive.ts +++ b/src/helpers/ui/sheet_interactive.ts @@ -1,4 +1,4 @@ -import { FORBIDDEN_SHEET_CHARS } from "../../constants"; +import { FORBIDDEN_SHEETNAME_CHARS } from "../../constants"; import { _t } from "../../translation"; import { CommandResult, SpreadsheetChildEnv, UID } from "../../types"; @@ -20,7 +20,7 @@ export function interactiveRenameSheet( env.raiseError( _t( "Some used characters are not allowed in a sheet name (Forbidden characters are %s).", - FORBIDDEN_SHEET_CHARS.join(" ") + FORBIDDEN_SHEETNAME_CHARS.join(" ") ), errorCallback ); diff --git a/src/index.ts b/src/index.ts index d0244a651c..8f10c00977 100644 --- a/src/index.ts +++ b/src/index.ts @@ -88,6 +88,7 @@ import { positionToZone, reduceZoneOnDeletion, rgbaToHex, + sanitizeSheetName, splitReference, toCartesian, toUnboundedZone, @@ -340,6 +341,7 @@ export const helpers = { createPivotFormula, areDomainArgsFieldsValid, splitReference, + sanitizeSheetName, }; export const links = { diff --git a/src/migrations/migration_steps.ts b/src/migrations/migration_steps.ts index 4114243683..45f2922dda 100644 --- a/src/migrations/migration_steps.ts +++ b/src/migrations/migration_steps.ts @@ -1,9 +1,5 @@ -import { - BACKGROUND_CHART_COLOR, - FORBIDDEN_IN_EXCEL_REGEX, - FORMULA_REF_IDENTIFIER, -} from "../constants"; -import { getItemId } from "../helpers"; +import { BACKGROUND_CHART_COLOR, FORMULA_REF_IDENTIFIER } from "../constants"; +import { getItemId, sanitizeSheetName } from "../helpers"; import { toXC } from "../helpers/coordinates"; import { getMaxObjectId } from "../helpers/pivot/pivot_helpers"; import { DEFAULT_TABLE_CONFIG } from "../helpers/table_presets"; @@ -109,13 +105,13 @@ migrationStepRegistry versionFrom: "7", migrate(data: any): any { const namesTaken: string[] = []; - const globalForbiddenInExcel = new RegExp(FORBIDDEN_IN_EXCEL_REGEX, "g"); for (let sheet of data.sheets || []) { if (!sheet.name) { continue; } const oldName = sheet.name; - const escapedName: string = oldName.replace(globalForbiddenInExcel, "_"); + sanitizeSheetName; + const escapedName: string = sanitizeSheetName(oldName, "_"); let i = 1; let newName = escapedName; while (namesTaken.includes(newName)) { diff --git a/src/plugins/core/sheet.ts b/src/plugins/core/sheet.ts index 993e1b5701..d0f1ada95e 100644 --- a/src/plugins/core/sheet.ts +++ b/src/plugins/core/sheet.ts @@ -1,4 +1,4 @@ -import { FORBIDDEN_IN_EXCEL_REGEX } from "../../constants"; +import { FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX } from "../../constants"; import { createDefaultRows, deepCopy, @@ -649,7 +649,7 @@ export class SheetPlugin extends CorePlugin implements SheetState { ) { return CommandResult.DuplicatedSheetName; } - if (FORBIDDEN_IN_EXCEL_REGEX.test(name!)) { + if (FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX.test(name!)) { return CommandResult.ForbiddenCharactersInSheetName; } return CommandResult.Success; diff --git a/src/plugins/ui_feature/insert_pivot.ts b/src/plugins/ui_feature/insert_pivot.ts index 5eb438b809..be6a213c6c 100644 --- a/src/plugins/ui_feature/insert_pivot.ts +++ b/src/plugins/ui_feature/insert_pivot.ts @@ -1,4 +1,5 @@ -import { FORBIDDEN_IN_EXCEL_REGEX, PIVOT_TABLE_CONFIG } from "../../constants"; +import { PIVOT_TABLE_CONFIG } from "../../constants"; +import { sanitizeSheetName } from "../../helpers"; import { createPivotFormula } from "../../helpers/pivot/pivot_helpers"; import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot"; import { getZoneArea, positionToZone } from "../../helpers/zones"; @@ -113,7 +114,7 @@ export class InsertPivotPlugin extends UIPlugin { private getPivotDuplicateSheetName(pivotName: string) { let i = 1; const names = this.getters.getSheetIds().map((id) => this.getters.getSheetName(id)); - const sanitizedName = pivotName.replace(new RegExp(FORBIDDEN_IN_EXCEL_REGEX, "g"), " "); + const sanitizedName = sanitizeSheetName(pivotName); let name = sanitizedName; while (names.includes(name)) { name = `${sanitizedName} (${i})`; diff --git a/tests/model/model_import_export.test.ts b/tests/model/model_import_export.test.ts index ad5fe126f6..8253b4e2ef 100644 --- a/tests/model/model_import_export.test.ts +++ b/tests/model/model_import_export.test.ts @@ -4,7 +4,7 @@ import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH, DEFAULT_REVISION_ID, - FORBIDDEN_SHEET_CHARS, + FORBIDDEN_SHEETNAME_CHARS, } from "../../src/constants"; import { toCartesian, toZone } from "../../src/helpers"; import { DEFAULT_TABLE_CONFIG } from "../../src/helpers/table_presets"; @@ -212,7 +212,7 @@ describe("Migrations", () => { stacked: false, }); }); - test.each(FORBIDDEN_SHEET_CHARS)("migrate version 7: sheet Names", (char) => { + test.each(FORBIDDEN_SHEETNAME_CHARS)("migrate version 7: sheet Names", (char) => { const model = new Model({ version: 7, sheets: [ diff --git a/tests/pivots/pivot_plugin.test.ts b/tests/pivots/pivot_plugin.test.ts index bca9433700..630f56d933 100644 --- a/tests/pivots/pivot_plugin.test.ts +++ b/tests/pivots/pivot_plugin.test.ts @@ -1,5 +1,5 @@ import { CommandResult } from "../../src"; -import { FORBIDDEN_SHEET_CHARS } from "../../src/constants"; +import { FORBIDDEN_SHEETNAME_CHARS } from "../../src/constants"; import { EMPTY_PIVOT_CELL } from "../../src/helpers/pivot/table_spreadsheet_pivot"; import { renameSheet, selectCell, setCellContent } from "../test_helpers/commands_helpers"; import { createModelFromGrid, toCellPosition } from "../test_helpers/helpers"; @@ -191,7 +191,7 @@ describe("Pivot plugin", () => { A2: "Alice", }; const model = createModelFromGrid(grid); - addPivot(model, "A1:A2", { name: `forbidden: ${FORBIDDEN_SHEET_CHARS}` }, "pivot1"); + addPivot(model, "A1:A2", { name: `forbidden: ${FORBIDDEN_SHEETNAME_CHARS}` }, "pivot1"); model.dispatch("DUPLICATE_PIVOT_IN_NEW_SHEET", { newPivotId: "pivot2", newSheetId: "Sheet2", diff --git a/tests/sheet/sheets_plugin.test.ts b/tests/sheet/sheets_plugin.test.ts index a17a77c4e7..617d7fd59c 100644 --- a/tests/sheet/sheets_plugin.test.ts +++ b/tests/sheet/sheets_plugin.test.ts @@ -1,4 +1,4 @@ -import { FORBIDDEN_SHEET_CHARS } from "../../src/constants"; +import { FORBIDDEN_SHEETNAME_CHARS } from "../../src/constants"; import { getCanonicalSymbolName, numberToLetters, toZone } from "../../src/helpers"; import { Model } from "../../src/model"; import { CommandResult } from "../../src/types"; @@ -39,7 +39,7 @@ import "../test_helpers/helpers"; import { createEqualCF, testUndoRedo, toRangesData } from "../test_helpers/helpers"; jest.mock("../../src/helpers/uuid", () => require("../__mocks__/uuid")); - +FORBIDDEN_SHEETNAME_CHARS; describe("sheets", () => { test("can create a new sheet, then undo, then redo", () => { const model = new Model(); @@ -141,7 +141,7 @@ describe("sheets", () => { ).toBeCancelledBecause(CommandResult.DuplicatedSheetName); }); - test.each(FORBIDDEN_SHEET_CHARS)("Cannot rename a sheet with a %s in the name", (char) => { + test.each(FORBIDDEN_SHEETNAME_CHARS)("Cannot rename a sheet with a %s in the name", (char) => { const model = new Model(); expect( renameSheet(model, model.getters.getActiveSheetId(), `my life ${char}`)