From 8b06b4f2b2abb13905d3e96069910ced867e447f Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Tue, 2 Apr 2024 15:16:39 +0800 Subject: [PATCH 1/3] fix(formula): copy paste range with formulas --- .../formula-clipboard.controller.spec.ts | 477 +++++++++++++++++- .../formula-clipboard.controller.ts | 14 +- 2 files changed, 470 insertions(+), 21 deletions(-) diff --git a/packages/sheets-formula/src/controllers/__tests__/formula-clipboard.controller.spec.ts b/packages/sheets-formula/src/controllers/__tests__/formula-clipboard.controller.spec.ts index 63ded74ba0..16fbd3811e 100644 --- a/packages/sheets-formula/src/controllers/__tests__/formula-clipboard.controller.spec.ts +++ b/packages/sheets-formula/src/controllers/__tests__/formula-clipboard.controller.spec.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import type { ICellData, IMutationInfo, Nullable, Univer } from '@univerjs/core'; +import type { ICellData, Nullable, Univer } from '@univerjs/core'; import { ICommandService, IUniverInstanceService, ObjectMatrix } from '@univerjs/core'; import { LexerTreeBuilder } from '@univerjs/engine-formula'; import { type ISetRangeValuesMutationParams, SetRangeValuesMutation } from '@univerjs/sheets'; @@ -144,7 +144,8 @@ describe('Test paste with formula', () => { 12: { 2: {}, 3: { - f: '=SUM(A13)', + f: null, + si: '3e4r5t', v: null, p: null, }, @@ -164,25 +165,461 @@ describe('Test paste with formula', () => { copyInfo, lexerTreeBuilder ); - removeFormulaId(redoUndoList.redos as Array>); + expect(redoUndoList).toStrictEqual(result); }); - }); -}); -function removeFormulaId(redos: Array>) { - if (redos.length > 0) { - const cellValue = redos[0].params.cellValue; - if (cellValue) { - Object.keys(cellValue).forEach((rowIndex) => { - const row = cellValue[Number(rowIndex)]; - Object.keys(row).forEach((columnIndex) => { - const cell = row[Number(columnIndex)]; - if (cell?.hasOwnProperty('si')) { - delete cell?.si; - } - }); + it('Copy range with formulas', async () => { + const unitId = 'test'; + const subUnitId = 'sheet1'; + const range = { + startRow: 5, + startColumn: 5, + endRow: 8, + endColumn: 8, + rangeType: 0, + }; + const matrix = new ObjectMatrix({ + 0: { + 0: { + p: null, + v: 1, + s: null, + f: '=SUM(A1)', + si: null, + t: 2, + }, + 1: { + p: null, + v: 2, + s: null, + f: '=SUM(B1)', + si: 'OENnXU', + t: 2, + }, + 2: { + p: null, + v: 3, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 3: { + p: null, + v: 4, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + }, + 1: { + 0: { + p: null, + v: 2, + s: null, + f: '=SUM(A2)', + si: 'jcozeE', + t: 2, + }, + 1: { + p: null, + v: 3, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 2: { + p: null, + v: 4, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 3: { + p: null, + v: 5, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + }, + 2: { + 0: { + p: null, + v: 3, + s: null, + f: null, + si: 'jcozeE', + t: 2, + }, + 1: { + p: null, + v: 4, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 2: { + p: null, + v: 5, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 3: { + p: null, + v: 6, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + }, + 3: { + 0: { + p: null, + v: 4, + s: null, + f: null, + si: 'jcozeE', + t: 2, + }, + 1: { + p: null, + v: 5, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 2: { + p: null, + v: 6, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + 3: { + p: null, + v: 7, + s: null, + f: null, + si: 'OENnXU', + t: 2, + }, + }, }); - } - } -} + + const accessor = { + get, + }; + + const copyInfo = { + copyType: COPY_TYPE.COPY, + copyRange: { + startRow: 0, + startColumn: 5, + endRow: 3, + endColumn: 8, + rangeType: 0, + }, + }; + + const result = { + undos: [ + { + id: 'sheet.mutation.set-range-values', + params: { + unitId, + subUnitId, + cellValue: { + 5: { + 5: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 6: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 7: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 8: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + }, + 6: { + 5: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 6: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 7: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 8: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + }, + 7: { + 5: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 6: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 7: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 8: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + }, + 8: { + 5: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 6: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 7: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + 8: { + s: null, + f: null, + si: null, + p: null, + v: null, + t: null, + }, + }, + }, + options: {}, + }, + }, + ], + redos: [ + { + id: 'sheet.mutation.set-range-values', + params: { + unitId, + subUnitId, + cellValue: { + 5: { + 5: { + si: 'bBSIMi', + f: '=SUM(A6)', + v: null, + p: null, + }, + 6: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 7: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 8: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + }, + 6: { + 5: { + si: 'jcozeE', + f: null, + v: null, + p: null, + }, + 6: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 7: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 8: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + }, + 7: { + 5: { + si: 'jcozeE', + f: null, + v: null, + p: null, + }, + 6: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 7: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 8: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + }, + 8: { + 5: { + si: 'jcozeE', + f: null, + v: null, + p: null, + }, + 6: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 7: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + 8: { + si: 'OENnXU', + f: null, + v: null, + p: null, + }, + }, + }, + }, + }, + ], + }; + + const redoUndoList = getSetCellFormulaMutations( + unitId, + subUnitId, + range, + matrix, + accessor, + copyInfo, + lexerTreeBuilder + ); + + // Randomly generated id, no comparison is made + const resultFormulaId = result.redos[0].params.cellValue['5'][5].si; + const originRedoParams = redoUndoList.redos[0].params as ISetRangeValuesMutationParams; + + if (!originRedoParams.cellValue || !originRedoParams.cellValue['5'] || !originRedoParams.cellValue['5'][5]) { + throw new Error('cellValue is undefined'); + } + + originRedoParams.cellValue['5'][5].si = resultFormulaId; + + expect(redoUndoList).toStrictEqual(result); + }); + }); +}); diff --git a/packages/sheets-formula/src/controllers/formula-clipboard.controller.ts b/packages/sheets-formula/src/controllers/formula-clipboard.controller.ts index b5772d033b..927e2a7452 100644 --- a/packages/sheets-formula/src/controllers/formula-clipboard.controller.ts +++ b/packages/sheets-formula/src/controllers/formula-clipboard.controller.ts @@ -17,6 +17,7 @@ import type { ICellData, IMutationInfo, IRange } from '@univerjs/core'; import { Disposable, + isFormulaId, isFormulaString, IUniverInstanceService, LifecycleStages, @@ -143,6 +144,7 @@ export function getSetCellFormulaMutations( matrix.forValue((row, col, value) => { const originalFormula = value.f || ''; + const originalFormulaId = value.si || ''; let valueObject: ICellDataWithSpanInfo = {}; // Paste the formula only, you also need to process some regular values @@ -150,7 +152,17 @@ export function getSetCellFormulaMutations( valueObject = Tools.deepClone(value); } - if (isFormulaString(originalFormula) && copyRowLength && copyColumnLength) { + if (!copyRowLength || !copyColumnLength) { + return; + } + + // Directly reuse when there is a formula id + if (isFormulaId(originalFormulaId)) { + valueObject.si = originalFormulaId; + valueObject.f = null; + valueObject.v = null; + valueObject.p = null; + } else if (isFormulaString(originalFormula)) { const rowIndex = row % copyRowLength; const colIndex = col % copyColumnLength; From 42f7f3b397db163ff55147b74479d374a5fdf572 Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Tue, 2 Apr 2024 19:43:05 +0800 Subject: [PATCH 2/3] fix(formula): cache false as blank array value --- .../__tests__/array-value-object.spec.ts | 82 ++++++++++++++++++- .../engine/value-object/array-value-object.ts | 37 +++++++-- 2 files changed, 110 insertions(+), 9 deletions(-) diff --git a/packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts b/packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts index 09c14e6861..99d48845ee 100644 --- a/packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts +++ b/packages/engine-formula/src/engine/value-object/__tests__/array-value-object.spec.ts @@ -16,8 +16,12 @@ import { describe, expect, it } from 'vitest'; +import type { Nullable } from '@univerjs/core'; import { ArrayValueObject, transformToValueObject, ValueObjectFactory } from '../array-value-object'; -import type { BooleanValueObject, NumberValueObject } from '../primitive-object'; +import { BooleanValueObject, NumberValueObject } from '../primitive-object'; +import type { BaseValueObject } from '../base-value-object'; +import { ErrorValueObject } from '../base-value-object'; +import { ErrorType } from '../../../basics/error-type'; describe('arrayValueObject test', () => { const originArrayValueObject = ArrayValueObject.create({ @@ -397,4 +401,80 @@ describe('arrayValueObject test', () => { expect(errorValueObject.isError()).toBeTruthy(); }); }); + + describe('Test DefaultValue', () => { + it('After set defaultValue, use ', () => { + const calculateValueList: Nullable[][] = [[]]; + calculateValueList[0][1] = NumberValueObject.create(2); + + const arrayValueObject = ArrayValueObject.create({ + calculateValueList, + rowCount: 2, + columnCount: 2, + unitId: '', + sheetId: '', + row: 0, + column: 0, + }); + + arrayValueObject.setDefaultValue(BooleanValueObject.create(false)); + + // getFirstCell + expect(arrayValueObject.getFirstCell().getValue()).toStrictEqual(false); + + // getLastCell + expect(arrayValueObject.getLastCell().getValue()).toStrictEqual(false); + + // iterator + arrayValueObject.iterator(iterator); + + // iteratorReverse + arrayValueObject.iteratorReverse(iterator); + + arrayValueObject.mapValue(mapValue); + + function iterator(valueObject: Nullable, row: number, column: number) { + if (!valueObject) { + return; + } + + const value = valueObject.getValue(); + + if (row === 0 && column === 0) { + expect(value).toStrictEqual(false); + } else if (row === 0 && column === 1) { + expect(value).toStrictEqual(2); + } else if (row === 1 && column === 0) { + expect(value).toStrictEqual(false); + } else if (row === 1 && column === 1) { + expect(value).toStrictEqual(false); + } + } + + function mapValue(valueObject: Nullable, row: number, column: number) { + if (!valueObject) { + return ErrorValueObject.create(ErrorType.NA); + } + + iterator(valueObject, row, column); + + return valueObject; + } + + // flatten + const flatten = arrayValueObject.flatten(); + expect(flatten.getFirstCell().getValue()).toStrictEqual(false); + + // slice + const firstColumn = arrayValueObject.slice([0, 1], [0, 1]); + if (!firstColumn) { + throw new Error('firstColumn is null'); + } + expect(firstColumn.getFirstCell().getValue()).toStrictEqual(false); + + // transpose + const transpose = arrayValueObject.transpose(); + expect(transpose.getFirstCell().getValue()).toStrictEqual(false); + }); + }); }); diff --git a/packages/engine-formula/src/engine/value-object/array-value-object.ts b/packages/engine-formula/src/engine/value-object/array-value-object.ts index 8b3364eb63..32a8d658a3 100644 --- a/packages/engine-formula/src/engine/value-object/array-value-object.ts +++ b/packages/engine-formula/src/engine/value-object/array-value-object.ts @@ -123,6 +123,11 @@ export class ArrayValueObject extends BaseValueObject { private _flattenCache: Nullable; + /** + * The default value of the array, null values in comparison results support setting to false + */ + private _defaultValue: Nullable = null; + private _flattenPosition: Nullable<{ stringArray: BaseValueObject[]; stringPosition: number[]; @@ -212,6 +217,10 @@ export class ArrayValueObject extends BaseValueObject { return true; } + setDefaultValue(value: Nullable) { + this._defaultValue = value; + } + get(row: number, column: number) { const rowValues = this._values[row]; if (rowValues == null) { @@ -271,7 +280,7 @@ export class ArrayValueObject extends BaseValueObject { for (let r = startRow; r <= endRow; r++) { for (let c = startColumn; c <= endColumn; c++) { - if (callback(valueList[r]?.[c], r, c) === false) { + if (callback(valueList[r]?.[c] || this._defaultValue, r, c) === false) { return; } } @@ -287,7 +296,7 @@ export class ArrayValueObject extends BaseValueObject { for (let r = endRow; r >= startRow; r--) { for (let c = endColumn; c >= startColumn; c--) { - if (callback(valueList[r][c], r, c) === false) { + if (callback(valueList[r]?.[c] || this._defaultValue, r, c) === false) { return; } } @@ -330,12 +339,12 @@ export class ArrayValueObject extends BaseValueObject { getFirstCell() { const { startRow, startColumn } = this.getRangePosition(); - return this.get(startRow, startColumn) || NullValueObject.create(); + return this.get(startRow, startColumn) || this._defaultValue || NullValueObject.create(); } getLastCell() { const { endRow, endColumn } = this.getRangePosition(); - return this.get(endRow, endColumn) || NullValueObject.create(); + return this.get(endRow, endColumn) || this._defaultValue || NullValueObject.create(); } /** @@ -395,6 +404,8 @@ export class ArrayValueObject extends BaseValueObject { const arrayV = this._createNewArray(newValue, 1, newValue[0].length); + arrayV.setDefaultValue(this._defaultValue); + this._flattenCache = arrayV; return arrayV; @@ -504,7 +515,7 @@ export class ArrayValueObject extends BaseValueObject { return; }; - let cell = array[r][c]; + let cell = array[r][c] || this._defaultValue; if (cell == null) { cell = NullValueObject.create(); @@ -533,6 +544,9 @@ export class ArrayValueObject extends BaseValueObject { const newResultArray = this._createNewArray(result, result.length, result[0].length, startRow, startColumn); + // Synchronize defaultValue + newResultArray.setDefaultValue(this._defaultValue); + this._sliceCache.set(cacheKey, newResultArray); return newResultArray; @@ -560,7 +574,10 @@ export class ArrayValueObject extends BaseValueObject { const rowCount = this._rowCount; const columnCount = this._columnCount; - return this._createNewArray(transposeArray, columnCount, rowCount); + const newArray = this._createNewArray(transposeArray, columnCount, rowCount); + + newArray.setDefaultValue(this._defaultValue); + return newArray; } /** @@ -934,7 +951,7 @@ export class ArrayValueObject extends BaseValueObject { if (row == null) { rowList[c] = ErrorValueObject.create(ErrorType.VALUE); } else { - const currentValue = row[c]; + const currentValue = row[c] || this._defaultValue; if (currentValue) { rowList[c] = callbackFn(currentValue, r, c); @@ -1372,7 +1389,11 @@ export class ArrayValueObject extends BaseValueObject { this._batchOperatorValue(value, c, result, batchOperatorType, operator); } - return this._createNewArray(result, rowCount, columnCount); + const newArray = this._createNewArray(result, rowCount, columnCount); + + // Mark empty values in the array as false + newArray.setDefaultValue(BooleanValueObject.create(false)); + return newArray; } private _batchOperatorValue( From 71eaca8913f2b6019179dcffa4992d502e35f8e6 Mon Sep 17 00:00:00 2001 From: Dushusir <1414556676@qq.com> Date: Mon, 8 Apr 2024 16:52:17 +0800 Subject: [PATCH 3/3] fix(formula): formulaDataItem's x and y support negative numbers --- .../src/controllers/formula-editor-show.controller.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/sheets-formula/src/controllers/formula-editor-show.controller.ts b/packages/sheets-formula/src/controllers/formula-editor-show.controller.ts index 45a0bbe130..73abb83fbf 100644 --- a/packages/sheets-formula/src/controllers/formula-editor-show.controller.ts +++ b/packages/sheets-formula/src/controllers/formula-editor-show.controller.ts @@ -95,7 +95,8 @@ export class FormulaEditorShowController extends Disposable { if (formulaDataItem != null) { const { f, si, x = 0, y = 0 } = formulaDataItem; - if (si != null && (x > 0 || y > 0)) { + // x and y support negative numbers. Negative numbers appear when the drop-down fill moves up or to the left. + if (si != null && (x !== 0 || y !== 0)) { let formulaString = ''; if (f.length > 0) { formulaString = f;