Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(formula): use ref range formula #1694

Merged
merged 13 commits into from
Apr 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion packages/core/src/shared/__test__/common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,34 @@
*/

import { describe, expect, it } from 'vitest';
import { cellToRange } from '../common';
import { cellToRange, isFormulaId, isFormulaString } from '../common';

describe('Test common', () => {
it('Test cellToRange', () => {
expect(cellToRange(0, 1)).toStrictEqual({ startRow: 0, startColumn: 1, endRow: 0, endColumn: 1 });
});

it('Test isFormulaString', () => {
expect(isFormulaString('=SUM(1)')).toBe(true);
expect(isFormulaString('SUM(1)')).toBe(false);
expect(isFormulaString('=')).toBe(false);
expect(isFormulaString('')).toBe(false);
expect(isFormulaString(1)).toBe(false);
expect(isFormulaString(null)).toBe(false);
expect(isFormulaString(undefined)).toBe(false);
expect(isFormulaString(true)).toBe(false);
expect(isFormulaString({})).toBe(false);
expect(isFormulaString({ f: '' })).toBe(false);
});

it('Test isFormulaId', () => {
expect(isFormulaId('id1')).toBe(true);
expect(isFormulaId('')).toBe(false);
expect(isFormulaId(1)).toBe(false);
expect(isFormulaId(null)).toBe(false);
expect(isFormulaId(undefined)).toBe(false);
expect(isFormulaId(true)).toBe(false);
expect(isFormulaId({})).toBe(false);
expect(isFormulaId({ f: '' })).toBe(false);
});
});
12 changes: 11 additions & 1 deletion packages/core/src/shared/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,12 +146,22 @@ export function getColorStyle(color: Nullable<IColorStyle>): Nullable<string> {
return null;
}

/**
* A string starting with an equal sign is a formula
* @param value
* @returns
*/
export function isFormulaString(value: any): boolean {
return Tools.isString(value) && value.substring(0, 1) === '=' && value.length > 1;
}

/**
* any string
* @param value
* @returns
*/
export function isFormulaId(value: any): boolean {
return Tools.isString(value) && value.indexOf('=') === -1 && value.length === 6;
return Tools.isString(value) && value.length > 0;
}

/**
Expand Down
3 changes: 0 additions & 3 deletions packages/core/src/types/interfaces/i-range.ts
Original file line number Diff line number Diff line change
Expand Up @@ -213,9 +213,6 @@ export interface IOptionData {
*
*/
contentsOnly?: boolean;
/**
* Whether to clear only the comments.
*/
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,34 +15,23 @@
*/

import type { IMutation } from '@univerjs/core';
import { CommandType, Tools } from '@univerjs/core';
import { CommandType } from '@univerjs/core';
import type { IAccessor } from '@wendellhu/redi';

import type { IArrayFormulaRangeType, IArrayFormulaUnitCellType } from '../../basics/common';
import { FormulaDataModel } from '../../models/formula-data.model';

export interface ISetArrayFormulaDataMutationParams {
arrayFormulaRange: IArrayFormulaRangeType;
arrayFormulaCellData: IArrayFormulaUnitCellType;
}

export const SetArrayFormulaDataUndoMutationFactory = (accessor: IAccessor): ISetArrayFormulaDataMutationParams => {
const formulaDataModel = accessor.get(FormulaDataModel);
const arrayFormulaRange = Tools.deepClone(formulaDataModel.getArrayFormulaRange());
const arrayFormulaCellData = Tools.deepClone(formulaDataModel.getArrayFormulaCellData());
return {
arrayFormulaRange,
arrayFormulaCellData,
};
};

/**
* There is no need to process data here, it is used as the main thread to send data to the worker. The main thread has already updated the data in advance, and there is no need to update it again here.
*/
export const SetArrayFormulaDataMutation: IMutation<ISetArrayFormulaDataMutationParams> = {
id: 'formula.mutation.set-array-formula-data',
type: CommandType.MUTATION,
handler: (accessor: IAccessor, params: ISetArrayFormulaDataMutationParams) => {
const formulaDataModel = accessor.get(FormulaDataModel);
formulaDataModel.setArrayFormulaRange(params.arrayFormulaRange);
formulaDataModel.setArrayFormulaCellData(params.arrayFormulaCellData);
return true;
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,18 @@ import { CommandType } from '@univerjs/core';
import type { IAccessor } from '@wendellhu/redi';

import type { IFormulaData } from '../../basics/common';
import { FormulaDataModel } from '../../models/formula-data.model';

export interface ISetFormulaDataMutationParams {
formulaData: IFormulaData;
}

/**
* There is no need to process data here, it is used as the main thread to send data to the worker. The main thread has already updated the data in advance, and there is no need to update it again here.
*/
export const SetFormulaDataMutation: IMutation<ISetFormulaDataMutationParams> = {
id: 'formula.mutation.set-formula-data',
type: CommandType.MUTATION,
handler: (accessor: IAccessor, params: ISetFormulaDataMutationParams) => {
const formulaDataModel = accessor.get(FormulaDataModel);
formulaDataModel.setFormulaData(params.formulaData);
return true;
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ export class CalculateController extends Disposable {
this._calculateFormulaService.stopFormulaExecution();
} else if (command.id === SetFormulaDataMutation.id) {
const formulaData = (command.params as ISetFormulaDataMutationParams).formulaData as IFormulaData;
this._formulaDataModel.setFormulaData(formulaData);

// formulaData is the incremental data sent from the main thread and needs to be merged into formulaDataModel
this._formulaDataModel.mergeFormulaData(formulaData);
} else if (command.id === SetFormulaCalculationStartMutation.id) {
const params = command.params as ISetFormulaCalculationStartMutation;

Expand All @@ -83,6 +85,7 @@ export class CalculateController extends Disposable {
}

const { arrayFormulaRange, arrayFormulaCellData } = params;
// TODO@Dushusir: Merge the array formula data into the formulaDataModel
this._formulaDataModel.setArrayFormulaRange(arrayFormulaRange);
this._formulaDataModel.setArrayFormulaCellData(arrayFormulaCellData);
}
Expand Down Expand Up @@ -114,8 +117,6 @@ export class CalculateController extends Disposable {

const arrayFormulaCellData = this._formulaDataModel.getArrayFormulaCellData();

// Synchronous to the main thread
// this._commandService.executeCommand(SetFormulaDataMutation.id, { formulaData });
this._calculateFormulaService.execute({
formulaData,
arrayFormulaCellData,
Expand Down
3 changes: 2 additions & 1 deletion packages/engine-formula/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ export { RegisterFunctionMutation } from './commands/mutations/register-function
export {
type ISetArrayFormulaDataMutationParams,
SetArrayFormulaDataMutation,
SetArrayFormulaDataUndoMutationFactory,
} from './commands/mutations/set-array-formula-data.mutation';

export { RemoveDefinedNameMutation, SetDefinedNameMutation, type ISetDefinedNameMutationSearchParam, type ISetDefinedNameMutationParam } from './commands/mutations/set-defined-name.mutation';
Expand Down Expand Up @@ -147,3 +146,5 @@ export { IFormulaRuntimeService, FormulaRuntimeService } from './services/runtim
export { IFormulaCurrentConfigService, FormulaCurrentConfigService } from './services/current-data.service';

export { IActiveDirtyManagerService } from './services/active-dirty-manager.service';

export type { IRangeChange } from './models/formula-data.model';
Loading
Loading