From 37f2328a82b7245eb0a47e129f55e273fe41051f Mon Sep 17 00:00:00 2001 From: Pierre Rousseau Date: Tue, 31 Dec 2024 08:42:45 +0100 Subject: [PATCH] [REF] helpers: introduce getUniqueText MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit adds the helper `getUniqueText`. This helper is used to generate a unique text based on the given text and the existing texts. closes odoo/o-spreadsheet#5392 Task: 4440233 Signed-off-by: Rémi Rahir (rar) --- src/helpers/misc.ts | 19 +++++++++++++++ .../spreadsheet_pivot/spreadsheet_pivot.ts | 12 ++++------ src/index.ts | 2 ++ src/migrations/migration_steps.ts | 11 ++++----- src/plugins/core/sheet.ts | 20 +++++----------- src/plugins/core/table_style.ts | 13 +++-------- src/plugins/ui_core_views/pivot_ui.ts | 11 ++++----- src/plugins/ui_feature/insert_pivot.ts | 10 ++------ src/plugins/ui_stateful/filter_evaluation.ts | 12 ++++------ src/xlsx/xlsx_writer.ts | 13 +++++------ tests/helpers/misc_helpers.test.ts | 23 +++++++++++++++++++ 11 files changed, 79 insertions(+), 67 deletions(-) diff --git a/src/helpers/misc.ts b/src/helpers/misc.ts index 61f6535e69..f36ec9fa97 100644 --- a/src/helpers/misc.ts +++ b/src/helpers/misc.ts @@ -653,3 +653,22 @@ export function transpose2dPOJO( } return result; } + +export function getUniqueText( + text: string, + texts: string[], + options: { + compute?: (text: string, increment: number) => string; + start?: number; + computeFirstOne?: boolean; + } = {} +): string { + const compute = options.compute ?? ((text, i) => `${text} (${i})`); + const computeFirstOne = options.computeFirstOne ?? false; + let i = options.start ?? 1; + let newText = computeFirstOne ? compute(text, i) : text; + while (texts.includes(newText)) { + newText = compute(text, i++); + } + return newText; +} diff --git a/src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts b/src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts index 5194c84084..f4278376dc 100644 --- a/src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts +++ b/src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts @@ -26,7 +26,7 @@ import { import { InitPivotParams, Pivot } from "../../../types/pivot_runtime"; import { toXC } from "../../coordinates"; import { formatValue, isDateTimeFormat } from "../../format/format"; -import { deepEquals, isDefined } from "../../misc"; +import { deepEquals, getUniqueText, isDefined } from "../../misc"; import { AGGREGATORS_FN, areDomainArgsFieldsValid, @@ -457,12 +457,10 @@ export class SpreadsheetPivot implements Pivot `${name}${i}`, + start: 2, + }); } private extractDataEntriesFromRange(range: Range): DataEntries { diff --git a/src/index.ts b/src/index.ts index eaf69ec08a..1e0caf2c54 100644 --- a/src/index.ts +++ b/src/index.ts @@ -74,6 +74,7 @@ import { deepEquals, expandZoneOnInsertion, formatValue, + getUniqueText, isDefined, isInside, isMarkdownLink, @@ -340,6 +341,7 @@ export const helpers = { areDomainArgsFieldsValid, splitReference, sanitizeSheetName, + getUniqueText, }; export const links = { diff --git a/src/migrations/migration_steps.ts b/src/migrations/migration_steps.ts index f3a6a6ce52..0418ca18a5 100644 --- a/src/migrations/migration_steps.ts +++ b/src/migrations/migration_steps.ts @@ -1,5 +1,5 @@ import { BACKGROUND_CHART_COLOR, FORMULA_REF_IDENTIFIER } from "../constants"; -import { getItemId, sanitizeSheetName } from "../helpers"; +import { getItemId, getUniqueText, sanitizeSheetName } from "../helpers"; import { toXC } from "../helpers/coordinates"; import { getMaxObjectId } from "../helpers/pivot/pivot_helpers"; import { DEFAULT_TABLE_CONFIG } from "../helpers/table_presets"; @@ -112,12 +112,9 @@ migrationStepRegistry const oldName = sheet.name; sanitizeSheetName; const escapedName: string = sanitizeSheetName(oldName, "_"); - let i = 1; - let newName = escapedName; - while (namesTaken.includes(newName)) { - newName = `${escapedName}${i}`; - i++; - } + const newName = getUniqueText(escapedName, namesTaken, { + compute: (name, i) => `${name}${i}`, + }); sheet.name = newName; namesTaken.push(newName); diff --git a/src/plugins/core/sheet.ts b/src/plugins/core/sheet.ts index 2bda019542..6f876132a0 100644 --- a/src/plugins/core/sheet.ts +++ b/src/plugins/core/sheet.ts @@ -2,6 +2,7 @@ import { FORBIDDEN_SHEETNAME_CHARS_IN_EXCEL_REGEX } from "../../constants"; import { createDefaultRows, deepCopy, + getUniqueText, getUnquotedSheetName, groupConsecutive, includesAll, @@ -429,14 +430,11 @@ export class SheetPlugin extends CorePlugin implements SheetState { } getNextSheetName(baseName = "Sheet"): string { - let i = 1; const names = this.orderedSheetIds.map(this.getSheetName.bind(this)); - let name = `${baseName}${i}`; - while (names.includes(name)) { - name = `${baseName}${i}`; - i++; - } - return name; + return getUniqueText(baseName, names, { + compute: (name, i) => `${name}${i}`, + computeFirstOne: true, + }); } getSheetSize(sheetId: UID): ZoneDimension { @@ -760,15 +758,9 @@ export class SheetPlugin extends CorePlugin implements SheetState { } private getDuplicateSheetName(sheetName: string) { - let i = 1; const names = this.orderedSheetIds.map(this.getSheetName.bind(this)); const baseName = _t("Copy of %s", sheetName); - let name = baseName.toString(); - while (names.includes(name)) { - name = `${baseName} (${i})`; - i++; - } - return name; + return getUniqueText(baseName.toString(), names); } private deleteSheet(sheet: Sheet) { diff --git a/src/plugins/core/table_style.ts b/src/plugins/core/table_style.ts index e86996530e..3405a96d10 100644 --- a/src/plugins/core/table_style.ts +++ b/src/plugins/core/table_style.ts @@ -1,4 +1,4 @@ -import { toHex } from "../../helpers"; +import { getUniqueText, toHex } from "../../helpers"; import { DEFAULT_TABLE_CONFIG, TABLE_PRESETS, @@ -88,15 +88,8 @@ export class TableStylePlugin extends CorePlugin implements Ta getNewCustomTableStyleName(): string { let name = _t("Custom Table Style"); - const styleNames = new Set(Object.values(this.styles).map((style) => style.displayName)); - if (!styleNames.has(name)) { - return name; - } - let i = 2; - while (styleNames.has(`${name} ${i}`)) { - i++; - } - return `${name} ${i}`; + const styleNames = Object.values(this.styles).map((style) => style.displayName); + return getUniqueText(name, styleNames, { compute: (name, i) => `${name} ${i}`, start: 2 }); } isTableStyleEditable(styleId: string): boolean { diff --git a/src/plugins/ui_core_views/pivot_ui.ts b/src/plugins/ui_core_views/pivot_ui.ts index bb33798940..dbca6caf5c 100644 --- a/src/plugins/ui_core_views/pivot_ui.ts +++ b/src/plugins/ui_core_views/pivot_ui.ts @@ -2,6 +2,7 @@ import { Token } from "../../formulas"; import { astToFormula } from "../../formulas/parser"; import { toScalar } from "../../functions/helper_matrices"; import { toBoolean } from "../../functions/helpers"; +import { getUniqueText } from "../../helpers"; import { getFirstPivotFunction, getNumberOfPivotFunctions, @@ -265,13 +266,9 @@ export class PivotUIPlugin extends UIPlugin { generateNewCalculatedMeasureName(measures: PivotCoreMeasure[]) { const existingMeasures = measures.map((m) => m.fieldName); - let i = 1; - let name = _t("Calculated measure %s", i); - while (existingMeasures.includes(name)) { - i++; - name = _t("Calculated measure %s", i); - } - return name; + return getUniqueText(_t("Calculated measure 1"), existingMeasures, { + compute: (name, i) => _t("Calculated measure %s", i), + }); } getPivot(pivotId: UID) { diff --git a/src/plugins/ui_feature/insert_pivot.ts b/src/plugins/ui_feature/insert_pivot.ts index be6a213c6c..5f7bcb469f 100644 --- a/src/plugins/ui_feature/insert_pivot.ts +++ b/src/plugins/ui_feature/insert_pivot.ts @@ -1,5 +1,5 @@ import { PIVOT_TABLE_CONFIG } from "../../constants"; -import { sanitizeSheetName } from "../../helpers"; +import { getUniqueText, sanitizeSheetName } from "../../helpers"; import { createPivotFormula } from "../../helpers/pivot/pivot_helpers"; import { SpreadsheetPivotTable } from "../../helpers/pivot/table_spreadsheet_pivot"; import { getZoneArea, positionToZone } from "../../helpers/zones"; @@ -112,15 +112,9 @@ 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 = sanitizeSheetName(pivotName); - let name = sanitizedName; - while (names.includes(name)) { - name = `${sanitizedName} (${i})`; - i++; - } - return name; + return getUniqueText(sanitizedName, names); } insertPivotWithTable( diff --git a/src/plugins/ui_stateful/filter_evaluation.ts b/src/plugins/ui_stateful/filter_evaluation.ts index d97df0fc59..18c6548e9b 100644 --- a/src/plugins/ui_stateful/filter_evaluation.ts +++ b/src/plugins/ui_stateful/filter_evaluation.ts @@ -1,5 +1,6 @@ import { deepCopy, + getUniqueText, positions, range, toLowerCase, @@ -221,12 +222,9 @@ export class FilterEvaluationPlugin extends UIPlugin { if (!colName) { colName = `Column${colIndex}`; } - let currentColName = colName; - let i = 2; - while (usedColNames.includes(currentColName)) { - currentColName = colName + String(i); - i++; - } - return currentColName; + return getUniqueText(colName, usedColNames, { + compute: (name, i) => colName + String(i), + start: 2, + }); } } diff --git a/src/xlsx/xlsx_writer.ts b/src/xlsx/xlsx_writer.ts index 76d34cbfd1..d0fe72b29c 100644 --- a/src/xlsx/xlsx_writer.ts +++ b/src/xlsx/xlsx_writer.ts @@ -1,5 +1,5 @@ import { DEFAULT_CELL_HEIGHT, DEFAULT_CELL_WIDTH } from "../constants"; -import { escapeRegExp, toZone, zoneToDimension } from "../helpers"; +import { escapeRegExp, getUniqueText, toZone, zoneToDimension } from "../helpers"; import { ExcelSheetData, ExcelWorkbookData } from "../types"; import { XLSXExport, @@ -376,14 +376,13 @@ function createRelRoot(): XLSXExportFile { */ export function fixLengthySheetNames(data: ExcelWorkbookData): ExcelWorkbookData { const nameMapping: Record = {}; - const newNames = new Set(); + const newNames: string[] = []; for (const sheet of data.sheets) { let newName = sheet.name.slice(0, 31); - let i = 1; - while (newNames.has(newName)) { - newName = newName.slice(0, 31 - String(i).length) + i++; - } - newNames.add(newName); + newName = getUniqueText(newName, newNames, { + compute: (name, i) => name.slice(0, 31 - String(i).length) + i, + }); + newNames.push(newName); if (newName !== sheet.name) { nameMapping[sheet.name] = newName; sheet.name = newName; diff --git a/tests/helpers/misc_helpers.test.ts b/tests/helpers/misc_helpers.test.ts index 5f879584ee..6f682617a3 100644 --- a/tests/helpers/misc_helpers.test.ts +++ b/tests/helpers/misc_helpers.test.ts @@ -2,6 +2,7 @@ import { DateTime, deepCopy, deepEquals, + getUniqueText, groupConsecutive, isConsecutive, lazy, @@ -275,3 +276,25 @@ describe("Memoize", () => { expect(memoizedFn.name).toEqual("smile (memoized)"); }); }); + +describe("getUniqueText", () => { + test("with no existing text", () => { + expect(getUniqueText("a", [])).toEqual("a"); + }); + + test("with existing text", () => { + expect(getUniqueText("a", ["a", "a (1)"])).toEqual("a (2)"); + }); + + test("with custom compute", () => { + expect(getUniqueText("a", ["a", "a 1"], { compute: (t, i) => `${t} ${i}` })).toEqual("a 2"); + }); + + test("with computeFirstOne", () => { + expect(getUniqueText("a", [], { computeFirstOne: true })).toEqual("a (1)"); + }); + + test("with start", () => { + expect(getUniqueText("a", ["a"], { start: 2 })).toEqual("a (2)"); + }); +});