-
Notifications
You must be signed in to change notification settings - Fork 108
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
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 e59d7eb
Add more validations for named expressions
swistak35 a5108a0
Export ExportedCellChange object instead of class
swistak35 d681b3e
It no longer exports only values
swistak35 19824b2
Don't leak internal detail of workbook named expressions, -1 sheet
swistak35 aa3e614
Add tests for named expression changes
swistak35 6321e58
Added test for matrix formula without braces
swistak35 3a3185a
Add tests for errors in named expressions with something other than f…
swistak35 e6cae9c
lint-fix
swistak35 96f58f8
Add ability to calculate fire-and-forget formulas
swistak35 b9e3bfd
Refactor common part
swistak35 d8589d7
lint-fix
swistak35 a1b4bd1
Add documentation
swistak35 b7a1cec
Fix tests
swistak35 3f9301e
Make it more clear what is the reason we initialize -1 sheet
swistak35 00eaded
Add possibility to calculateFormula in context of some sheet
swistak35 40c667a
Rename spec
swistak35 631de60
Change the test other way around
swistak35 db8c071
Support any raw cell content as an expression
swistak35 dcaa092
Refactoring: Rename NamedExpressions#storeFormulaInCell -> #storeExpr…
swistak35 0147afa
Refactoring: Rename #changeNamedExpressionFormula -> #changeNamedExpr…
swistak35 64def07
Support other types of input when changing named expressions
swistak35 62c9be1
Refactoring: change naming external formula to temporary formula
swistak35 daad4b2
Refactoring: Make HyperFormula know less
swistak35 22bef30
lint-fix
swistak35 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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, | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should
ExportedNamedExpressionChange
havevalue
alias as well?Or we don't need
newValue
when we havevalue
? It's a little bit confusing why we have both while they are always equal.There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.