Skip to content

Commit d3546ed

Browse files
committed
[REF] ranges: remove sheetId argument of adaptRanges
The `adaptRanges` method of core plugins took a `sheetId` argument. But: 1) It wasn't easy to understand what it did and when to use it. It was indicating the sheet with the ranges to adapt, but needed to be ignored when adapting formulas. 2) In the future addition of named ranges, some adaptations (renaming a named range) won't have a sheetId, and will need to be applied to all sheets. 3) The argument was only there as an optimization. But it was useless in the adaptation of cells formulas, as the `if` was repeated in the `applyChange` implementation. And other core plugin adaptation costs are negligible compared to the cells. This commit removes the `sheetId` argument altogether. Task: 5380498 Part-of: #7605 Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
1 parent a54737a commit d3546ed

File tree

9 files changed

+39
-38
lines changed

9 files changed

+39
-38
lines changed

packages/o-spreadsheet-engine/src/plugins/core/cell.ts

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ import {
3636
import { recomputeZones } from "../../helpers/recompute_zones";
3737
import { Format } from "../../types/format";
3838
import { DEFAULT_LOCALE } from "../../types/locale";
39-
import { AdaptSheetName, Style, UpdateCellData, Zone } from "../../types/misc";
39+
import { Style, UpdateCellData, Zone } from "../../types/misc";
4040
import { Range, RangePart } from "../../types/range";
4141
import { ExcelWorkbookData, WorkbookData } from "../../types/workbook_data";
4242
import { SquishedCell, Squisher } from "./squisher";
@@ -67,24 +67,22 @@ export class CellPlugin extends CorePlugin<CoreState> implements CoreState {
6767
readonly nextId = 1;
6868
public readonly cells: { [sheetId: string]: { [id: string]: Cell } } = {};
6969

70-
adaptRanges({ applyChange }: RangeAdapterFunctions, sheetId: UID, sheetName: AdaptSheetName) {
70+
adaptRanges({ applyChange }: RangeAdapterFunctions) {
7171
for (const sheet of Object.keys(this.cells)) {
7272
for (const cell of Object.values(this.cells[sheet] || {})) {
7373
if (cell.isFormula) {
7474
for (const range of cell.compiledFormula.rangeDependencies) {
75-
if (range.sheetId === sheetId || range.invalidSheetName === sheetName.old) {
76-
const change = applyChange(range);
77-
if (change.changeType !== "NONE") {
78-
this.history.update(
79-
"cells",
80-
sheet,
81-
cell.id,
82-
"compiledFormula" as any,
83-
"rangeDependencies",
84-
cell.compiledFormula.rangeDependencies.indexOf(range),
85-
change.range
86-
);
87-
}
75+
const change = applyChange(range);
76+
if (change.changeType !== "NONE") {
77+
this.history.update(
78+
"cells",
79+
sheet,
80+
cell.id,
81+
"compiledFormula" as any,
82+
"rangeDependencies",
83+
cell.compiledFormula.rangeDependencies.indexOf(range),
84+
change.range
85+
);
8886
}
8987
}
9088
}

packages/o-spreadsheet-engine/src/plugins/core/conditional_format.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -175,9 +175,8 @@ export class ConditionalFormatPlugin
175175
}
176176
}
177177

178-
adaptRanges(rangeAdapters: RangeAdapterFunctions, sheetId: UID) {
179-
const sheetIds = sheetId ? [sheetId] : Object.keys(this.cfRules);
180-
for (const sheetId of sheetIds) {
178+
adaptRanges(rangeAdapters: RangeAdapterFunctions) {
179+
for (const sheetId of Object.keys(this.cfRules)) {
181180
this.adaptCFRanges(sheetId, rangeAdapters);
182181
}
183182
this.adaptCFFormulas(rangeAdapters);

packages/o-spreadsheet-engine/src/plugins/core/data_validation.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,10 @@ export class DataValidationPlugin
3434

3535
readonly rules: { [sheet: string]: DataValidationRule[] } = {};
3636

37-
adaptRanges(rangeAdapters: RangeAdapterFunctions, sheetId: UID) {
38-
this.adaptDVRanges(sheetId, rangeAdapters);
37+
adaptRanges(rangeAdapters: RangeAdapterFunctions) {
38+
for (const sheetId in this.rules) {
39+
this.adaptDVRanges(sheetId, rangeAdapters);
40+
}
3941
this.adaptDVFormulas(rangeAdapters);
4042
}
4143

packages/o-spreadsheet-engine/src/plugins/core/figures.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@ export class FigurePlugin extends CorePlugin<FigureState> implements FigureState
2727
// Command Handling
2828
// ---------------------------------------------------------------------------
2929

30-
adaptRanges({ applyChange }: RangeAdapterFunctions, sheetId: UID) {
30+
adaptRanges(rangeAdapterFunctions: RangeAdapterFunctions): void {
31+
for (const sheetId in this.figures) {
32+
this.adaptRangesOnSheet(rangeAdapterFunctions, sheetId);
33+
}
34+
}
35+
36+
adaptRangesOnSheet({ applyChange }: RangeAdapterFunctions, sheetId: UID) {
3137
for (const figure of this.getFigures(sheetId)) {
3238
const change = applyChange(
3339
this.getters.getRangeFromZone(sheetId, {

packages/o-spreadsheet-engine/src/plugins/core/merge.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,8 +117,10 @@ export class MergePlugin extends CorePlugin<MergeState> implements MergeState {
117117
}
118118
}
119119

120-
adaptRanges(rangeAdapters: RangeAdapterFunctions, sheetId: UID) {
121-
this.applyRangeChangeOnSheet(sheetId, rangeAdapters);
120+
adaptRanges(rangeAdapters: RangeAdapterFunctions) {
121+
for (const sheetId in this.merges) {
122+
this.applyRangeChangeOnSheet(sheetId, rangeAdapters);
123+
}
122124
}
123125

124126
// ---------------------------------------------------------------------------

packages/o-spreadsheet-engine/src/plugins/core/range.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ export class RangeAdapterPlugin implements CommandHandler<CoreCommand> {
105105
adaptFormulaStringRanges(defaultSheetId, formula, rangeAdapter),
106106
};
107107
for (const provider of this.providers) {
108-
provider(adapterFunctions, rangeAdapter.sheetId, rangeAdapter.sheetName);
108+
provider(adapterFunctions);
109109
}
110110
this.isAdaptingRanges = false;
111111
}

packages/o-spreadsheet-engine/src/plugins/core/tables.ts

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,11 @@ export class TablePlugin extends CorePlugin<TableState> implements TableState {
5252
readonly tables: Record<UID, Record<TableId, CoreTable | undefined>> = {};
5353
readonly nextTableId: number = 1;
5454

55-
adaptRanges({ applyChange }: RangeAdapterFunctions, sheetId: UID) {
56-
for (const table of this.getCoreTables(sheetId)) {
57-
this.applyRangeChangeOnTable(sheetId, table, applyChange);
55+
adaptRanges({ applyChange }: RangeAdapterFunctions) {
56+
for (const sheetId in this.tables) {
57+
for (const table of this.getCoreTables(sheetId)) {
58+
this.applyRangeChangeOnTable(sheetId, table, applyChange);
59+
}
5860
}
5961
}
6062

packages/o-spreadsheet-engine/src/plugins/core_plugin.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { StateObserver } from "../state_observer";
22
import { CoreCommand, CoreCommandDispatcher } from "../types/commands";
33
import { CoreGetters } from "../types/core_getters";
4-
import { AdaptSheetName, RangeAdapterFunctions, RangeProvider, UID } from "../types/misc";
4+
import { RangeAdapterFunctions, RangeProvider } from "../types/misc";
55
import { ModelConfig } from "../types/model";
66
import { WorkbookData } from "../types/workbook_data";
77
import { BasePlugin } from "./base_plugin";
@@ -62,9 +62,5 @@ export class CorePlugin<State = any>
6262
* @param sheetId an sheetId to adapt either range of that sheet specifically, or ranges pointing to that sheet
6363
* @param sheetName couple of old and new sheet names to adapt ranges pointing to that sheet
6464
*/
65-
adaptRanges(
66-
rangeAdapterFunctions: RangeAdapterFunctions,
67-
sheetId: UID,
68-
sheetName: AdaptSheetName
69-
): void {}
65+
adaptRanges(rangeAdapterFunctions: RangeAdapterFunctions): void {}
7066
}

packages/o-spreadsheet-engine/src/types/misc.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -307,11 +307,7 @@ export type Dimension = "COL" | "ROW";
307307
export type ConsecutiveIndexes = HeaderIndex[];
308308

309309
export interface RangeProvider {
310-
adaptRanges: (
311-
adapterFunctions: RangeAdapterFunctions,
312-
sheetId: UID,
313-
sheetName: AdaptSheetName
314-
) => void;
310+
adaptRanges: (adapterFunctions: RangeAdapterFunctions) => void;
315311
}
316312

317313
export type Validation<T> = (toValidate: T) => CommandResult | CommandResult[];

0 commit comments

Comments
 (0)