fix(formula): formula string results are displayed as regular strings (…

…#2206) * fix(formula): formula string results are displayed as regular strings * fix(formula): filter plain formula error text in cell * fix(formula): numfmt pattern priority in formula * fix(formula): type error
dream-num · May 13, 2024 · 1d1a45f · 1d1a45f
1 parent 48417ca
commit 1d1a45f
Show file tree

Hide file tree

Showing 11 changed files with 120 additions and 28 deletions.
diff --git a/...s/src/basics/__tests__/numfmt-kit.spec.ts → ...engine/utils/__tests__/numfmt-kit.spec.ts b/...s/src/basics/__tests__/numfmt-kit.spec.ts → ...engine/utils/__tests__/numfmt-kit.spec.ts
@@ -15,7 +15,8 @@
  */
 
 import { describe, expect, it } from 'vitest';
-import { compareNumfmtPriority } from '../numfmt-kit';
+import { compareNumfmtPriority, comparePatternPriority } from '../numfmt-kit';
+import { operatorToken } from '../../../basics/token';
 
 const numfmtMap = {
     currency: '"¥"#,##0.00_);[Red]("¥"#,##0.00)',
@@ -161,4 +162,23 @@ describe('Test numfmt kit', () => {
         expect(compareNumfmtPriority(numfmtMap.accounting, numfmtMap.unknown)).toBe(numfmtMap.unknown);
         expect(compareNumfmtPriority(numfmtMap.accounting, numfmtMap.accounting)).toBe(numfmtMap.accounting);
     });
+
+    it('Function comparePatternPriority', () => {
+        // Currency + Currency = Currency
+        expect(comparePatternPriority(numfmtMap.currency, numfmtMap.currency, operatorToken.PLUS)).toBe(numfmtMap.currency);
+        // Currency - Currency = Currency
+        expect(comparePatternPriority(numfmtMap.currency, numfmtMap.currency, operatorToken.MINUS)).toBe(numfmtMap.currency);
+        // Currency * Currency = General
+        expect(comparePatternPriority(numfmtMap.currency, numfmtMap.currency, operatorToken.MULTIPLY)).toBe('');
+        // Currency / Currency = General
+        expect(comparePatternPriority(numfmtMap.currency, numfmtMap.currency, operatorToken.DIVIDED)).toBe('');
+        // Date + Date = General
+        expect(comparePatternPriority(numfmtMap.date, numfmtMap.date, operatorToken.PLUS)).toBe('');
+        // Date - Date = General
+        expect(comparePatternPriority(numfmtMap.date, numfmtMap.date, operatorToken.MINUS)).toBe('');
+        // Date * Date = General
+        expect(comparePatternPriority(numfmtMap.date, numfmtMap.date, operatorToken.MULTIPLY)).toBe('');
+        // Date / Date = General
+        expect(comparePatternPriority(numfmtMap.date, numfmtMap.date, operatorToken.DIVIDED)).toBe('');
+    });
 });
diff --git a/packages/engine-formula/src/engine/utils/__tests__/value-object.spec.ts b/packages/engine-formula/src/engine/utils/__tests__/value-object.spec.ts
@@ -59,6 +59,7 @@ describe('Test object cover', () => {
     it('Function objectValueToCellValue', () => {
         expect(objectValueToCellValue(NumberValueObject.create(1))).toStrictEqual({ v: 1, t: CellValueType.NUMBER });
         expect(objectValueToCellValue(StringValueObject.create('Univer'))).toStrictEqual({ v: 'Univer', t: CellValueType.STRING });
+        expect(objectValueToCellValue(StringValueObject.create('0'))).toStrictEqual({ v: '0', t: CellValueType.STRING });
         expect(objectValueToCellValue(BooleanValueObject.create(true))).toStrictEqual({ v: 1, t: CellValueType.BOOLEAN });
         expect(objectValueToCellValue(BooleanValueObject.create(false))).toStrictEqual({ v: 0, t: CellValueType.BOOLEAN });
         expect(objectValueToCellValue(NullValueObject.create())).toStrictEqual({ v: null });

diff --git a/packages/sheets/src/basics/numfmt-kit.ts → ...ne-formula/src/engine/utils/numfmt-kit.ts b/packages/sheets/src/basics/numfmt-kit.ts → ...ne-formula/src/engine/utils/numfmt-kit.ts
@@ -15,7 +15,9 @@
  */
 
 import type { ICellData, Nullable, Styles } from '@univerjs/core';
-import numfmt from '@univerjs/engine-numfmt';
+// @ts-ignore
+import numfmt from 'numfmt';
+import { operatorToken } from '../../basics/token';
 
 const currencySymbols = [
     '$',
@@ -47,6 +49,20 @@ const currencySymbols = [
     '₿',
 ];
 
+type FormatType =
+    | 'currency'
+    | 'date'
+    | 'datetime'
+    | 'error'
+    | 'fraction'
+    | 'general'
+    | 'grouped'
+    | 'number'
+    | 'percent'
+    | 'scientific'
+    | 'text'
+    | 'time';
+
 enum NumberFormatType {
     General,
     Number,
@@ -188,10 +204,54 @@ function getNumberFormatType(pattern: string): NumberFormatType {
         return NumberFormatType.Accounting;
     }
 
-    const type = numfmt.getInfo(pattern).type || 'unknown';
+    const type = numfmt.getInfo(pattern).type as FormatType || 'unknown';
     return NumberFormatTypeMap[type];
 }
 
 function isAccounting(pattern: string) {
     return !!currencySymbols.find((code) => pattern.includes(code)) && pattern.startsWith('_(');
 };
+
+
+/**
+ * The number format of the formula inherits the number format of the referenced cell, usually taking the format of the cell at the front position, but some formats have higher priority, there are some special cases.
+ *
+ * e.g.
+ * Currency * Currency = General
+ * Currency / Currency = General
+ *
+ * For two cells with the same number format, the calculated result should inherit the following number format
+ * ┌─────┬─────────┬──────────┬────────────┬─────────┬─────────┬────────────┬──────────┬────────────┬──────┬─────────┬──────────┐
+ * │     │ Number  │ Currency │ Accounting │ Date    │ Time    │ Percentage │ Fraction │ Scientific │ Text │ Special │ Custom   │
+ * ├─────┼─────────┼──────────┼────────────┼─────────┼─────────┼────────────┼──────────┼────────────┼──────┼─────────┼──────────┤
+ * │  +  │ Number  │ Currency │ Accounting │ General │ Time    │ Percentage │ Fraction │ Scientific │ Text │ Special │ General  │
+ * │  -  │ Number  │ Currency │ Accounting │ General │ Time    │ Percentage │ Fraction │ Scientific │ Text │ Special │ General  │
+ * │  *  │ General │ General  │ General    │ General │ General │ Percentage │ Fraction │ Scientific │ Text │ General │ General  │
+ * │  /  │ General │ General  │ General    │ General │ General │ Percentage │ Fraction │ Scientific │ Text │ General │ General  │
+ * └─────┴─────────┴──────────┴────────────┴─────────┴─────────┴────────────┴──────────┴────────────┴──────┴─────────┴──────────┘
+ *
+ * @param previousPattern
+ * @param nextPattern
+ */
+export function comparePatternPriority(previousPattern: string, nextPattern: string, operator: operatorToken) {
+    const previousPatternType = getNumberFormatType(previousPattern);
+    const nextPatternType = getNumberFormatType(nextPattern);
+
+    if (operator === operatorToken.PLUS || operator === operatorToken.MINUS) {
+        if ((previousPatternType === NumberFormatType.Date && nextPatternType === NumberFormatType.Date) || (previousPatternType === NumberFormatType.Custom && nextPatternType === NumberFormatType.Custom)) {
+            return '';
+        }
+
+        return nextPattern;
+    }
+
+    if (operator === operatorToken.MULTIPLY || operator === operatorToken.DIVIDED) {
+        if ((previousPatternType === NumberFormatType.Percentage && nextPatternType === NumberFormatType.Percentage) || (previousPatternType === NumberFormatType.Fraction && nextPatternType === NumberFormatType.Fraction) || (previousPatternType === NumberFormatType.Scientific && nextPatternType === NumberFormatType.Scientific) || (previousPatternType === NumberFormatType.Text && nextPatternType === NumberFormatType.Text)) {
+            return nextPattern;
+        }
+
+        return '';
+    }
+
+    return previousPattern || nextPattern;
+}
diff --git a/packages/engine-formula/src/engine/utils/value-object.ts b/packages/engine-formula/src/engine/utils/value-object.ts
@@ -15,7 +15,7 @@
  */
 
 import type { ICellData, Nullable } from '@univerjs/core';
-import { CellValueType, isRealNum } from '@univerjs/core';
+import { CellValueType } from '@univerjs/core';
 import type { BaseReferenceObject, FunctionVariantType } from '../reference-object/base-reference-object';
 import type { ArrayValueObject } from '../value-object/array-value-object';
 import type { BaseValueObject, ErrorValueObject } from '../value-object/base-value-object';
@@ -106,10 +106,11 @@ export function objectValueToCellValue(objectValue: Nullable<BaseValueObject>):
             };
         }
         // String "00"
-        if (vo.isString() && isRealNum(v)) {
+        // =IF(1,"0") evaluates to "0", which should be a normal string (regardless of whether it is a number or not). Forced strings only appear when preceded by single quotes
+        if (vo.isString()) {
             return {
                 v,
-                t: CellValueType.FORCE_STRING,
+                t: CellValueType.STRING,
                 ...cellWithStyle,
             };
         }

diff --git a/packages/engine-formula/src/engine/value-object/primitive-object.ts b/packages/engine-formula/src/engine/value-object/primitive-object.ts
@@ -20,10 +20,11 @@ import { isRealNum } from '@univerjs/core';
 import { reverseCompareOperator } from '../../basics/calculate';
 import { BooleanValue, ConcatenateType } from '../../basics/common';
 import { ERROR_TYPE_SET, ErrorType } from '../../basics/error-type';
-import { compareToken } from '../../basics/token';
+import { compareToken, operatorToken } from '../../basics/token';
 import { compareWithWildcard, isWildcard } from '../utils/compare';
 import { ceil, floor, mod, pow, round } from '../utils/math-kit';
 import { FormulaAstLRU } from '../../basics/cache-lru';
+import { comparePatternPriority } from '../utils/numfmt-kit';
 import { BaseValueObject, ErrorValueObject } from './base-value-object';
 
 export type PrimitiveValueType = string | boolean | number | null;
@@ -442,7 +443,7 @@ export class NumberValueObject extends BaseValueObject {
         }
 
         // Set number format
-        object.setPattern(this.getPattern() || valueObject.getPattern());
+        object.setPattern(comparePatternPriority(this.getPattern(), valueObject.getPattern(), operatorToken.PLUS));
 
         return object;
     }
@@ -467,7 +468,7 @@ export class NumberValueObject extends BaseValueObject {
         }
 
         // Set number format
-        object.setPattern(this.getPattern() || valueObject.getPattern());
+        object.setPattern(comparePatternPriority(this.getPattern(), valueObject.getPattern(), operatorToken.MINUS));
 
         return object;
     }
@@ -484,7 +485,7 @@ export class NumberValueObject extends BaseValueObject {
         }
 
         // Set number format
-        object.setPattern(this.getPattern() || valueObject.getPattern());
+        object.setPattern(comparePatternPriority(this.getPattern(), valueObject.getPattern(), operatorToken.MULTIPLY));
 
         return object;
     }
@@ -505,7 +506,7 @@ export class NumberValueObject extends BaseValueObject {
         }
 
         // Set number format
-        object.setPattern(this.getPattern() || valueObject.getPattern());
+        object.setPattern(comparePatternPriority(this.getPattern(), valueObject.getPattern(), operatorToken.DIVIDED));
 
         return object;
     }

diff --git a/packages/engine-formula/src/index.ts b/packages/engine-formula/src/index.ts
@@ -149,3 +149,4 @@ export { IFormulaCurrentConfigService, FormulaCurrentConfigService } from './ser
 export { IActiveDirtyManagerService } from './services/active-dirty-manager.service';
 
 export type { IRangeChange } from './models/formula-data.model';
+export { handleNumfmtInCell } from './engine/utils/numfmt-kit';
diff --git a/packages/sheets-formula/src/controllers/utils/__tests__/utils.spec.ts b/packages/sheets-formula/src/controllers/utils/__tests__/utils.spec.ts
@@ -21,18 +21,22 @@ import { extractFormulaError } from '../utils';
 
 describe('Test utils', () => {
     it('Function extractFormulaError', () => {
-        expect(extractFormulaError({ v: ErrorType.DIV_BY_ZERO })).toBe(ErrorType.DIV_BY_ZERO);
-        expect(extractFormulaError({ v: ErrorType.NAME })).toBe(ErrorType.NAME);
-        expect(extractFormulaError({ v: ErrorType.VALUE })).toBe(ErrorType.VALUE);
-        expect(extractFormulaError({ v: ErrorType.NUM })).toBe(ErrorType.NUM);
-        expect(extractFormulaError({ v: ErrorType.NA })).toBe(ErrorType.NA);
-        expect(extractFormulaError({ v: ErrorType.CYCLE })).toBe(ErrorType.CYCLE);
-        expect(extractFormulaError({ v: ErrorType.REF })).toBe(ErrorType.REF);
-        expect(extractFormulaError({ v: ErrorType.SPILL })).toBe(ErrorType.SPILL);
-        expect(extractFormulaError({ v: ErrorType.CALC })).toBe(ErrorType.CALC);
-        expect(extractFormulaError({ v: ErrorType.ERROR })).toBe(ErrorType.ERROR);
-        expect(extractFormulaError({ v: ErrorType.CONNECT })).toBe(ErrorType.CONNECT);
-        expect(extractFormulaError({ v: ErrorType.NULL })).toBe(ErrorType.NULL);
+        expect(extractFormulaError({ v: ErrorType.DIV_BY_ZERO, f: '=1/0' })).toBe(ErrorType.DIV_BY_ZERO);
+        expect(extractFormulaError({ v: ErrorType.NAME, f: '=S' })).toBe(ErrorType.NAME);
+        expect(extractFormulaError({ v: ErrorType.VALUE, f: '=SUM(A1)' })).toBe(ErrorType.VALUE);
+        expect(extractFormulaError({ v: ErrorType.NUM, f: '=SUM(A1)' })).toBe(ErrorType.NUM);
+        expect(extractFormulaError({ v: ErrorType.NA, f: '=SUM(A1)' })).toBe(ErrorType.NA);
+        expect(extractFormulaError({ v: ErrorType.CYCLE, f: '=SUM(A1)' })).toBe(ErrorType.CYCLE);
+        expect(extractFormulaError({ v: ErrorType.REF, f: '=SUM(A1)' })).toBe(ErrorType.REF);
+        expect(extractFormulaError({ v: ErrorType.SPILL, f: '=SUM(A1)' })).toBe(ErrorType.SPILL);
+        expect(extractFormulaError({ v: ErrorType.CALC, f: '=SUM(A1)' })).toBe(ErrorType.CALC);
+        expect(extractFormulaError({ v: ErrorType.ERROR, f: '=SUM(A1)' })).toBe(ErrorType.ERROR);
+        expect(extractFormulaError({ v: ErrorType.CONNECT, f: '=SUM(A1)' })).toBe(ErrorType.CONNECT);
+
+        expect(extractFormulaError({ v: ErrorType.NULL, f: '=SUM(A1)' })).toBe(ErrorType.NULL);
+        expect(extractFormulaError({ v: ErrorType.NULL, f: '=SUM(A1)', si: 'id1' })).toBe(ErrorType.NULL);
+        expect(extractFormulaError({ v: ErrorType.NULL, si: 'id1' })).toBe(ErrorType.NULL);
+        expect(extractFormulaError({ v: ErrorType.NULL })).toBeNull();
 
         expect(extractFormulaError({ f: '=SUM(1)' })).toBeNull();
         expect(extractFormulaError({ si: 'id1' })).toBeNull();

diff --git a/packages/sheets-formula/src/controllers/utils/utils.ts b/packages/sheets-formula/src/controllers/utils/utils.ts
@@ -15,7 +15,7 @@
  */
 
 import type { ICellData, IContextService, Nullable } from '@univerjs/core';
-import { FOCUSING_DOC, FOCUSING_UNIVER_EDITOR, FOCUSING_UNIVER_EDITOR_STANDALONE_SINGLE_MODE } from '@univerjs/core';
+import { FOCUSING_DOC, FOCUSING_UNIVER_EDITOR, FOCUSING_UNIVER_EDITOR_STANDALONE_SINGLE_MODE, isFormulaId, isFormulaString } from '@univerjs/core';
 import type { ErrorType } from '@univerjs/engine-formula';
 import { ERROR_TYPE_SET } from '@univerjs/engine-formula';
 
@@ -33,7 +33,8 @@ export function whenEditorStandalone(contextService: IContextService) {
  * @returns
  */
 export function extractFormulaError(cell: Nullable<ICellData>) {
-    if (typeof cell?.v === 'string' && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
+    // Must contain a formula
+    if (typeof cell?.v === 'string' && (isFormulaString(cell.f) || isFormulaId(cell.si)) && ERROR_TYPE_SET.has(cell.v as ErrorType)) {
         return cell.v as ErrorType;
     }
 

diff --git a/packages/sheets/src/commands/mutations/__tests__/set-range-values.mutation.spec.ts b/packages/sheets/src/commands/mutations/__tests__/set-range-values.mutation.spec.ts
@@ -24,6 +24,9 @@ describe('test "SetRangeValuesMutation" ', () => {
             expect(checkCellValueType('string', CellValueType.BOOLEAN)).toBe(CellValueType.STRING);
             expect(checkCellValueType('string', CellValueType.NUMBER)).toBe(CellValueType.STRING);
             expect(checkCellValueType('string', CellValueType.STRING)).toBe(CellValueType.STRING);
+            expect(checkCellValueType('0', CellValueType.STRING)).toBe(CellValueType.NUMBER);
+            expect(checkCellValueType('true', CellValueType.STRING)).toBe(CellValueType.BOOLEAN);
+            expect(checkCellValueType('false', CellValueType.STRING)).toBe(CellValueType.BOOLEAN);
 
             expect(checkCellValueType(123, CellValueType.STRING)).toBe(CellValueType.NUMBER);
             expect(checkCellValueType(123, CellValueType.BOOLEAN)).toBe(CellValueType.NUMBER); // not a valid boolean number, casted to number

diff --git a/packages/sheets/src/commands/mutations/set-range-values.mutation.ts b/packages/sheets/src/commands/mutations/set-range-values.mutation.ts
@@ -180,7 +180,8 @@ export const SetRangeValuesMutation: IMutation<ISetRangeValuesMutationParams, bo
                 const oldVal = cellMatrix.getValue(row, col) || {};
 
                 // NOTE: we may need to take `p` into account
-                const type = (newVal.t === CellValueType.FORCE_STRING)
+                // If the new value contains t, then take t directly
+                const type = newVal.t
                     ? newVal.t
                     : newVal.v !== undefined
                         ? checkCellValueType(newVal.v, newVal.t)

diff --git a/packages/sheets/src/controllers/calculate-result-apply.controller.ts b/packages/sheets/src/controllers/calculate-result-apply.controller.ts
@@ -17,11 +17,10 @@
 import type { ICellData, ICommandInfo, IObjectMatrixPrimitiveType, Nullable } from '@univerjs/core';
 import { Disposable, ICommandService, IUniverInstanceService, LifecycleStages, ObjectMatrix, OnLifecycle } from '@univerjs/core';
 import type { ISetFormulaCalculationResultMutation } from '@univerjs/engine-formula';
-import { SetFormulaCalculationResultMutation } from '@univerjs/engine-formula';
+import { handleNumfmtInCell, SetFormulaCalculationResultMutation } from '@univerjs/engine-formula';
 
 import { Inject } from '@wendellhu/redi';
 import { SetRangeValuesMutation } from '../commands/mutations/set-range-values.mutation';
-import { handleNumfmtInCell } from '../basics/numfmt-kit';
 
 @OnLifecycle(LifecycleStages.Ready, CalculateResultApplyController)
 export class CalculateResultApplyController extends Disposable {
Original file line number	Diff line number	Diff line change
Expand Up		@@ -149,3 +149,4 @@ export { IFormulaCurrentConfigService, FormulaCurrentConfigService } from './ser
		export { IActiveDirtyManagerService } from './services/active-dirty-manager.service';

		export type { IRangeChange } from './models/formula-data.model';
		export { handleNumfmtInCell } from './engine/utils/numfmt-kit';