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

Formula helpers and named expressions (part 2) #192

Merged
merged 25 commits into from
Feb 26, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3ef31fe
Add test for 255-long named expression
swistak35 Feb 18, 2020
e59d7eb
Add more validations for named expressions
swistak35 Feb 18, 2020
a5108a0
Export ExportedCellChange object instead of class
swistak35 Feb 18, 2020
d681b3e
It no longer exports only values
swistak35 Feb 18, 2020
19824b2
Don't leak internal detail of workbook named expressions, -1 sheet
swistak35 Feb 20, 2020
aa3e614
Add tests for named expression changes
swistak35 Feb 20, 2020
6321e58
Added test for matrix formula without braces
swistak35 Feb 20, 2020
3a3185a
Add tests for errors in named expressions with something other than f…
swistak35 Feb 20, 2020
e6cae9c
lint-fix
swistak35 Feb 20, 2020
96f58f8
Add ability to calculate fire-and-forget formulas
swistak35 Feb 20, 2020
b9e3bfd
Refactor common part
swistak35 Feb 20, 2020
d8589d7
lint-fix
swistak35 Feb 20, 2020
a1b4bd1
Add documentation
swistak35 Feb 20, 2020
b7a1cec
Fix tests
swistak35 Feb 20, 2020
3f9301e
Make it more clear what is the reason we initialize -1 sheet
swistak35 Feb 25, 2020
00eaded
Add possibility to calculateFormula in context of some sheet
swistak35 Feb 26, 2020
40c667a
Rename spec
swistak35 Feb 26, 2020
631de60
Change the test other way around
swistak35 Feb 26, 2020
db8c071
Support any raw cell content as an expression
swistak35 Feb 26, 2020
dcaa092
Refactoring: Rename NamedExpressions#storeFormulaInCell -> #storeExpr…
swistak35 Feb 26, 2020
0147afa
Refactoring: Rename #changeNamedExpressionFormula -> #changeNamedExpr…
swistak35 Feb 26, 2020
64def07
Support other types of input when changing named expressions
swistak35 Feb 26, 2020
62c9be1
Refactoring: change naming external formula to temporary formula
swistak35 Feb 26, 2020
daad4b2
Refactoring: Make HyperFormula know less
swistak35 Feb 26, 2020
22bef30
lint-fix
swistak35 Feb 26, 2020
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
57 changes: 54 additions & 3 deletions src/CellValue.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,44 @@
import {CellError, ErrorType, InternalCellValue, NoErrorCellValue} from './Cell'
import {CellError, ErrorType, InternalCellValue, NoErrorCellValue, simpleCellAddress, SimpleCellAddress} from './Cell'
import {CellValueChange} from './ContentChanges'
import {Config} from './Config'
import {NamedExpressions} from './NamedExpressions'

export type CellValue = NoErrorCellValue | DetailedCellError

export type ExportedChange = ExportedCellChange | ExportedNamedExpressionChange

export class ExportedCellChange {
constructor(
public readonly address: SimpleCellAddress,
public readonly newValue: CellValue,
) {
}

public get col() {
return this.address.col
}

public get row() {
return this.address.row
}

public get sheet() {
return this.address.sheet
}

public get value() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should ExportedNamedExpressionChange have value alias as well?

Or we don't need newValue when we have value? It's a little bit confusing why we have both while they are always equal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@wojciechczerniak only for compatibility reasons (I didn't want to break the integration), I'd leave only newValue as it betters conveys intent, but the value is already there, if our current integration doesn't rely on that then I will gladly fallback to newValue only

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jansiegel has to review the integration anyway. A lot has changed since we did it, and since we haven't released HF or HOT integration yet so we should break the compatibility if it's necessary. Better now than later or keeping it. Maintaining the unnecessary code is a chore.

return this.newValue
}
}

export class ExportedNamedExpressionChange {
constructor(
public readonly name: string,
public readonly newValue: CellValue,
) {
}
}

export class DetailedCellError {
constructor(
public readonly error: CellError,
Expand All @@ -19,13 +55,28 @@ export class DetailedCellError {
}
}

export class CellValueExporter {
export class Exporter {
constructor(
private readonly config: Config,
private readonly namedExpressions: NamedExpressions,
) {
}

public export(value: InternalCellValue): CellValue {
public exportChange(change: CellValueChange): ExportedChange {
if (change.sheet === NamedExpressions.SHEET_FOR_WORKBOOK_EXPRESSIONS) {
return new ExportedNamedExpressionChange(
this.namedExpressions.fetchNameForNamedExpressionRow(change.row),
this.exportValue(change.value),
)
} else {
return new ExportedCellChange(
simpleCellAddress(change.sheet, change.col, change.row),
this.exportValue(change.value),
)
}
}

public exportValue(value: InternalCellValue): CellValue {
if (this.config.smartRounding && typeof value == 'number' && !Number.isInteger(value)) {
return this.cellValueRounding(value)
} else if (value instanceof CellError) {
Expand Down
15 changes: 6 additions & 9 deletions src/ContentChanges.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {InternalCellValue, SimpleCellAddress} from './Cell'
import {CellValueExporter} from './CellValue'
import {Matrix} from './Matrix'

export interface CellValueChange {
Expand All @@ -9,6 +8,9 @@ export interface CellValueChange {
value: InternalCellValue,
}

export interface IChangeExporter<T> {
exportChange: (arg: CellValueChange) => T,
}
export type ChangeList = CellValueChange[]

export class ContentChanges {
Expand Down Expand Up @@ -38,15 +40,10 @@ export class ContentChanges {
this.changes.push(...change)
}

public exportChanges(exporter: CellValueExporter): ChangeList {
const ret: ChangeList = []
public exportChanges<T>(exporter: IChangeExporter<T>): T[] {
const ret: T[] = []
this.changes.forEach((e, i) => {
ret[i] = {
sheet: this.changes[i].sheet,
col: this.changes[i].col,
row: this.changes[i].row,
value: exporter.export(this.changes[i].value),
}
ret[i] = exporter.exportChange(this.changes[i])
})
return ret
}
Expand Down
3 changes: 3 additions & 0 deletions src/Evaluator.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import {ContentChanges} from './ContentChanges'
import {InternalCellValue, SimpleCellAddress} from './Cell'
import {Ast} from './parser'
import {Vertex} from './DependencyGraph'

export interface Evaluator {
run(): void,
partialRun(vertices: Vertex[]): ContentChanges,
runAndForget(ast: Ast, address: SimpleCellAddress): InternalCellValue,
destroy(): void,
}
Loading