Skip to content

Commit

Permalink
[IMP] array formula: getSpreadPositionsOf now returns root position
Browse files Browse the repository at this point in the history
Before this the getter `getSpreadPositionsOf` only returned the
position of the cells an array formula was spread to, minus the
cell the array formula was in. This made it impossible to differentiate
between a cell with a normal formula, and a cell with an array formula
returning a 1x1 matrix.

Now the getter returns the root position of the array formula as well.

The getter `getArrayFormulaSpreadingOn` was also modified to work when
calling it on the root position of an array formula.

There was also a bug where the spread relation weren't properly cleaned.

Task: 3707037
Part-of: #3624
  • Loading branch information
hokolomopo committed Mar 25, 2024
1 parent 21cb7a8 commit 53ac031
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 6 deletions.
4 changes: 2 additions & 2 deletions src/clipboard_handlers/cell_clipboard.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { CommandResult } from "..";
import { canonicalizeNumberValue } from "../formulas/formula_locale";
import { formatValue } from "../helpers";
import { deepEquals, formatValue } from "../helpers";
import { getPasteZones } from "../helpers/clipboard/clipboard_helpers";
import {
CellPosition,
Expand Down Expand Up @@ -55,7 +55,7 @@ export class CellClipboardHandler extends AbstractCellClipboardHandler<
const spreader = this.getters.getArrayFormulaSpreadingOn(position);
let cell = this.getters.getCell(position);
const evaluatedCell = this.getters.getEvaluatedCell(position);
if (spreader) {
if (spreader && !deepEquals(spreader, position)) {
const isSpreaderCopied =
rowsIndexes.includes(spreader.row) && columnsIndexes.includes(spreader.col);
const content = isSpreaderCopied
Expand Down
10 changes: 7 additions & 3 deletions src/plugins/ui_core_views/cell_evaluation/evaluator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,11 @@ export class Evaluator {
if (!this.spreadingRelations.isArrayFormula(position)) {
return [];
}
return Array.from(this.spreadingRelations.getArrayResultPositions(position));
if (this.evaluatedCells.get(position)?.type === CellValueType.error) {
return [position];
}
const spreadPositions = Array.from(this.spreadingRelations.getArrayResultPositions(position));
return [position, ...spreadPositions];
}

getEvaluatedPositions(): CellPosition[] {
Expand All @@ -68,7 +72,7 @@ export class Evaluator {

getArrayFormulaSpreadingOn(position: CellPosition): CellPosition | undefined {
if (!this.spreadingRelations.hasArrayFormulaResult(position)) {
return undefined;
return this.spreadingRelations.isArrayFormula(position) ? position : undefined;
}
const arrayFormulas = this.spreadingRelations.getFormulaPositionsSpreadingOn(position);
return Array.from(arrayFormulas).find((position) => !this.blockedArrayFormulas.has(position));
Expand Down Expand Up @@ -229,6 +233,7 @@ export class Evaluator {
if (!this.blockedArrayFormulas.has(position)) {
this.invalidateSpreading(position);
}
this.spreadingRelations.removeNode(position);

const cell = this.getters.getCell(position);
if (cell === undefined) {
Expand Down Expand Up @@ -401,7 +406,6 @@ export class Evaluator {
this.nextPositionsToUpdate.addMany(this.getCellsDependingOn([child]));
this.nextPositionsToUpdate.addMany(this.getArrayFormulasBlockedByOrSpreadingOn(child));
}
this.spreadingRelations.removeNode(position);
}

// ----------------------------------------------------------
Expand Down
48 changes: 47 additions & 1 deletion tests/evaluation/evaluation_formula_array.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { arg, functionRegistry } from "../../src/functions";
import { toScalar } from "../../src/functions/helper_matrices";
import { toMatrix, toNumber } from "../../src/functions/helpers";
import { Model } from "../../src/model";
import { DEFAULT_LOCALE } from "../../src/types";
import { DEFAULT_LOCALE, UID } from "../../src/types";
import {
addColumns,
addRows,
Expand All @@ -18,9 +18,12 @@ import { getCellContent, getCellError, getEvaluatedCell } from "../test_helpers/
import { restoreDefaultFunctions } from "../test_helpers/helpers";

let model: Model;
let sheetId: UID;

describe("evaluate formulas that return an array", () => {
beforeEach(() => {
model = new Model();
sheetId = model.getters.getActiveSheetId();
functionRegistry.add("MFILL", {
description: "Return an n*n matrix filled with n.",
args: [
Expand Down Expand Up @@ -758,4 +761,47 @@ describe("evaluate formulas that return an array", () => {
});
});
});

describe("Spread formula getters", () => {
test("getArrayFormulaSpreadingOn works on the root of an array formula", () => {
setCellContent(model, "A1", "=MFILL(2,2,42)");
expect(model.getters.getArrayFormulaSpreadingOn({ sheetId, col: 0, row: 0 })).toEqual({
sheetId,
col: 0,
row: 0,
});
});

test("getSpreadPositionsOf getter returns the cells the formula spread on, as well as the cell the formula is on", () => {
setCellContent(model, "A1", "=MFILL(2,2,42)");
const sheetId = model.getters.getActiveSheetId();
expect(model.getters.getSpreadPositionsOf({ sheetId, col: 0, row: 0 })).toEqual([
{ sheetId, col: 0, row: 0 },
{ sheetId, col: 0, row: 1 },
{ sheetId, col: 1, row: 0 },
{ sheetId, col: 1, row: 1 },
]);
});

test("getSpreadPositionsOf does only return self if the formula could not spread", () => {
setCellContent(model, "A1", "=MFILL(2,2,42)");
setCellContent(model, "A2", "(ツ)_/¯");
expect(model.getters.getSpreadPositionsOf({ sheetId, col: 0, row: 0 })).toEqual([
{ sheetId, col: 0, row: 0 },
]);
});

test("getSpreadPositionsOf is correct after the evaluation changed so the formula can spread again", () => {
setCellContent(model, "H1", "5");
setCellContent(model, "A1", "=MFILL(H1,H1,42)");

expect(model.getters.getSpreadPositionsOf({ sheetId, col: 0, row: 0 })).toHaveLength(25);

setCellContent(model, "A4", "Block spread");
expect(model.getters.getSpreadPositionsOf({ sheetId, col: 0, row: 0 })).toHaveLength(1);

setCellContent(model, "H1", "2");
expect(model.getters.getSpreadPositionsOf({ sheetId, col: 0, row: 0 })).toHaveLength(4);
});
});
});

0 comments on commit 53ac031

Please sign in to comment.