Switching to different approach. Revert changes in Operations.ts.

Author: sequba

src/Operations.ts

@@ -22,10 +22,10 @@ import {
   ParsingErrorVertex,
   SheetMapping,
   SparseStrategy,
-  ValueCellVertex,
+  ValueCellVertex
 } from './DependencyGraph'
 import { FormulaVertex } from './DependencyGraph/FormulaCellVertex'
-import { RawAndParsedValue, ValueCellVertexValue } from './DependencyGraph/ValueCellVertex'
+import { RawAndParsedValue } from './DependencyGraph/ValueCellVertex'
 import { AddColumnsTransformer } from './dependencyTransformers/AddColumnsTransformer'
 import { AddRowsTransformer } from './dependencyTransformers/AddRowsTransformer'
 import { CleanOutOfScopeDependenciesTransformer } from './dependencyTransformers/CleanOutOfScopeDependenciesTransformer'
@@ -51,7 +51,7 @@ import {
   NamedExpressions
 } from './NamedExpressions'
 import { NamedExpressionDependency, ParserWithCaching, ParsingErrorType, RelativeDependency } from './parser'
-import { AstNodeType, ErrorWithRawInputAst, ParsingError } from './parser/Ast'
+import { ParsingError } from './parser/Ast'
 import { ParsingResult } from './parser/ParserWithCaching'
 import { findBoundaries, Sheet } from './Sheet'
 import { ColumnsSpan, RowsSpan } from './Span'
@@ -245,66 +245,11 @@ export class Operations {
     this.columnSearch.removeSheet(sheetId)
   }
 
-  /**
-   * TODO
-   */
-  public addSheet(name?: string): string {
+  public addSheet(name?: string) {
     const sheetId = this.sheetMapping.addSheet(name)
-    const addedSheetName = this.sheetMapping.fetchDisplayName(sheetId)
-    this.dependencyGraph.addressMapping.autoAddSheet(sheetId, findBoundaries([]))
-    this.reparseFormulasDependingOnSheet(addedSheetName)
-    return addedSheetName
-  }
-
-  /**
-   * TODO
-   */
-  private reparseFormulasDependingOnSheet(sheetName: string): void {
-    const quotedSheetName = `'${sheetName.replace(/'/g, "''")}'`
-
-    const containsSheetName = (formulaText: string): boolean => {
-      const lowerInput = formulaText.toLowerCase()
-      return lowerInput.includes(`${sheetName}!`)
-          || lowerInput.includes(`${quotedSheetName}!`)
-    }
-
-    for (const node of this.dependencyGraph.formulaNodes()) {
-      const oldAst = node.getFormula(this.dependencyGraph.lazilyTransformingAstService)
-      if (oldAst.type === AstNodeType.ERROR_WITH_RAW_INPUT && containsSheetName(oldAst.rawInput)) {
-        this.reparseFormulaNode(node, oldAst)
-      }
-    }
-  }
-
-  /**
-   * TODO
-   */
-  private reparseFormulaNode(node: FormulaCellVertex, ast: ErrorWithRawInputAst): void {
-    const address = node.getAddress(this.dependencyGraph.lazilyTransformingAstService)
-    const formulaText = `=${ast.rawInput}`
-    const parserResult = this.parser.parse(formulaText, address)
-    const { ast: newAst, errors } = parserResult
-
-    if (errors.length > 0) {
-      this.setParsingErrorToCell(formulaText, errors, address)
-    } else {
-      try {
-        const size = this.arraySizePredictor.checkArraySize(newAst, address)
-
-        if (size.width <= 0 || size.height <= 0) {
-          throw Error('Incorrect array size')
-        }
-
-        this.setFormulaToCell(address, size, parserResult)
-      } catch (error) {
-        if (!(error as Error).message) {
-          throw error
-        }
-
-        const parsingError: ParsingError = { type: ParsingErrorType.InvalidRangeSize, message: 'Invalid range size.' }
-        this.setParsingErrorToCell(formulaText, [parsingError], address)
-      }
-    }
+    const sheet: Sheet = []
+    this.dependencyGraph.addressMapping.autoAddSheet(sheetId, findBoundaries(sheet))
+    return this.sheetMapping.fetchDisplayName(sheetId)
   }
 
   public renameSheet(sheetId: number, newName: string) {
@@ -514,6 +459,8 @@ export class Operations {
 
   /**
    * Restores a single cell.
+   * @param {SimpleCellAddress} address
+   * @param {ClipboardCell} clipboardCell
    */
   public restoreCell(address: SimpleCellAddress, clipboardCell: ClipboardCell): void {
     switch (clipboardCell.type) {
@@ -623,6 +570,7 @@ export class Operations {
 
           this.setFormulaToCell(address, size, parserResult)
         } catch (error) {
+
           if (!(error as Error).message) {
             throw error
           }
@@ -650,66 +598,45 @@ export class Operations {
     }
   }
 
-  /**
-   * Sets cell content to an instance of parsing error.
-   * Creates a ParsingErrorVertex and updates the dependency graph and column search index.
-   */
   public setParsingErrorToCell(rawInput: string, errors: ParsingError[], address: SimpleCellAddress) {
-    this.removeCellValueFromColumnSearch(address)
-
+    const oldValue = this.dependencyGraph.getCellValue(address)
     const vertex = new ParsingErrorVertex(errors, rawInput)
     const arrayChanges = this.dependencyGraph.setParsingErrorToCell(address, vertex)
-
+    this.columnSearch.remove(getRawValue(oldValue), address)
     this.columnSearch.applyChanges(arrayChanges.getChanges())
     this.changes.addAll(arrayChanges)
     this.changes.addChange(vertex.getCellValue(), address)
   }
 
-  /**
-   * Sets cell content to a formula.
-   * Creates a FormulaCellVertex and updates the dependency graph and column search index.
-   */
   public setFormulaToCell(address: SimpleCellAddress, size: ArraySize, {
     ast,
     hasVolatileFunction,
     hasStructuralChangeFunction,
     dependencies
   }: ParsingResult) {
-    this.removeCellValueFromColumnSearch(address)
-
+    const oldValue = this.dependencyGraph.getCellValue(address)
     const arrayChanges = this.dependencyGraph.setFormulaToCell(address, ast, absolutizeDependencies(dependencies, address), size, hasVolatileFunction, hasStructuralChangeFunction)
-
+    this.columnSearch.remove(getRawValue(oldValue), address)
     this.columnSearch.applyChanges(arrayChanges.getChanges())
     this.changes.addAll(arrayChanges)
   }
 
-  /**
-   * Sets cell content to a value.
-   * Creates a ValueCellVertex and updates the dependency graph and column search index.
-   */
   public setValueToCell(value: RawAndParsedValue, address: SimpleCellAddress) {
-    this.changeCellValueInColumnSearch(address, value.parsedValue)
-
+    const oldValue = this.dependencyGraph.getCellValue(address)
     const arrayChanges = this.dependencyGraph.setValueToCell(address, value)
-
+    this.columnSearch.change(getRawValue(oldValue), getRawValue(value.parsedValue), address)
     this.columnSearch.applyChanges(arrayChanges.getChanges().filter(change => !equalSimpleCellAddress(change.address, address)))
     this.changes.addAll(arrayChanges)
     this.changes.addChange(value.parsedValue, address)
   }
 
-  /**
-   * Sets cell content to an empty value.
-   * Creates an EmptyCellVertex and updates the dependency graph and column search index.
-   */
   public setCellEmpty(address: SimpleCellAddress) {
     if (this.dependencyGraph.isArrayInternalCell(address)) {
       return
     }
-
-    this.removeCellValueFromColumnSearch(address)
-
+    const oldValue = this.dependencyGraph.getCellValue(address)
     const arrayChanges = this.dependencyGraph.setCellEmpty(address)
-
+    this.columnSearch.remove(getRawValue(oldValue), address)
     this.columnSearch.applyChanges(arrayChanges.getChanges())
     this.changes.addAll(arrayChanges)
     this.changes.addChange(EmptyValue, address)
@@ -996,45 +923,6 @@ export class Operations {
     }
     return this.dependencyGraph.fetchCellOrCreateEmpty(expression.address).vertex
   }
-
-  /**
-   * Removes a cell value from the columnSearch index.
-   * Ignores the non-computed formula vertices.
-   */
-  private removeCellValueFromColumnSearch(address: SimpleCellAddress): void {
-    if (this.isNotComputed(address)) {
-      return
-    }
-
-    const oldValue = this.dependencyGraph.getCellValue(address)
-    this.columnSearch.remove(getRawValue(oldValue), address)
-  }
-
-  /**
-   * Changes a cell value in the columnSearch index.
-   * Ignores the non-computed formula vertices.
-   */
-  private changeCellValueInColumnSearch(address: SimpleCellAddress, newValue: ValueCellVertexValue): void {
-    if (this.isNotComputed(address)) {
-      return
-    }
-
-    const oldValue = this.dependencyGraph.getCellValue(address)
-    this.columnSearch.change(getRawValue(oldValue), getRawValue(newValue), address)
-  }
-
-  /**
-   * Checks if the FormulaCellVertex or ArrayVertex at the given address is not computed.
-   */
-  private isNotComputed(address: SimpleCellAddress): boolean {
-    const vertex = this.dependencyGraph.getCell(address)
-
-    if (!vertex) {
-      return false
-    }
-
-    return 'isComputed' in vertex && !vertex.isComputed()
-  }
 }
 
 export function normalizeRemovedIndexes(indexes: ColumnRowIndex[]): ColumnRowIndex[] {

test/unit/cruds/adding-sheet.spec.ts

@@ -125,7 +125,7 @@ describe('add sheet to engine', () => {
     expect(engine.getCellValue(adr('A1', engine.getSheetId(table1Name)))).toBe(10)
   })
 
-  it.only('recalculates formulas with range references and aggregate functions (#1116)', () => {
+  it('recalculates formulas with range references and aggregate functions (#1116)', () => {
     const engine = HyperFormula.buildEmpty()
     const sheet1Name = 'Sheet1'
     const sheet2Name = 'Sheet2'

test/unit/undo-redo.spec.ts

@@ -1005,8 +1005,8 @@ describe('UndoRedo - at the Operations layer', () => {
 
   it('clearUndoStack should clear out all undo entries', () => {
     expect(undoRedo.isUndoStackEmpty()).toBe(true)
-    undoRedo.saveOperation(new AddSheetUndoEntry('Sheet 1', 0))
-    undoRedo.saveOperation(new AddSheetUndoEntry('Sheet 2', 0))
+    undoRedo.saveOperation(new AddSheetUndoEntry('Sheet 1'))
+    undoRedo.saveOperation(new AddSheetUndoEntry('Sheet 2'))
 
     expect(undoRedo.isUndoStackEmpty()).toBe(false)