API fix #687

izulin · 2021-06-06T08:56:19Z

Context

Fixes API of getFillRangeData so it conforms to how we pass ranges.

How has this been tested?

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature or improvement (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Additional language file or change to the existing one (translations)

Other: change to API.

Related issue(s):

Checklist:

My code follows the code style of this project,
My change requires a change to the documentation,
I described the modification in the CHANGELOG.md file.

codecov · 2021-06-06T09:27:49Z

Codecov Report

Merging #687 (97ceab9) into develop (ee22e12) will decrease coverage by 0.13%.
The diff coverage is 75.36%.

@@             Coverage Diff             @@
##           develop     #687      +/-   ##
===========================================
- Coverage    92.27%   92.14%   -0.14%     
===========================================
  Files          166      166              
  Lines        39412    39548     +136     
  Branches      3575     3606      +31     
===========================================
+ Hits         36368    36442      +74     
- Misses        3038     3100      +62     
  Partials         6        6

Impacted Files	Coverage Δ
src/interpreter/Interpreter.ts	`51.42% <0.00%> (ø)`
src/interpreter/plugin/NumericAggregationPlugin.ts	`92.17% <66.66%> (+0.01%)`	⬆️
src/AbsoluteCellRange.ts	`86.17% <71.42%> (-0.49%)`	⬇️
src/Cell.ts	`94.76% <75.00%> (-0.79%)`	⬇️
src/HyperFormula.ts	`97.53% <75.11%> (-1.26%)`	⬇️
src/DependencyGraph/DependencyGraph.ts	`86.08% <92.30%> (+0.12%)`	⬆️
src/DependencyGraph/Graph.ts	`92.98% <100.00%> (-0.07%)`	⬇️
src/index.ts	`100.00% <100.00%> (ø)`
src/parser/RowAddress.ts	`91.34% <0.00%> (-1.93%)`	⬇️
src/parser/addressRepresentationConverters.ts	`88.32% <0.00%> (-0.51%)`	⬇️
... and 1 more

lgtm-com · 2021-06-09T22:54:01Z

This pull request introduces 1 alert when merging 0db693e into 6f64f7c - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2021-06-09T23:08:55Z

This pull request introduces 1 alert when merging 53a5af5 into 6f64f7c - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2021-06-10T08:24:18Z

This pull request introduces 1 alert when merging 3284b60 into 6f64f7c - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2021-06-10T09:00:06Z

This pull request introduces 1 alert when merging d3c36c1 into 6f64f7c - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

wojciechczerniak

LGTM from API pov 👍

I'm adding @kirszenbaum as reviewer as he's responsible for the docs, and we're touching public API here. We probably should check if we're not using those methods anywhere in docs

CHANGELOG.md

src/HyperFormula.ts

lgtm-com · 2021-06-11T15:55:55Z

This pull request introduces 1 alert when merging 831818a into 0041732 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

kirszenbaum · 2021-06-14T11:52:45Z

CHANGELOG.md

@@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Breaking change**: Changed API of many sheet-related methods to take sheetId instead of sheetName as an argument. (#645)
 - **Breaking change**: Removed support for matrix formulas (`{=FORMULA}`) notation. Engine now supports formulas returning array of values (instead of only scalars). (#652)
 - **Breaking change**: Removed numeric matrix detection along with matrixDetection and matrixDetectionThreshold config options. (#669)
+- **Breaking change**: Changed API of many function to take SimpleCellRange type argument instead of corner + width + height. (#687)


Suggested change

- **Breaking change**: Changed API of many function to take SimpleCellRange type argument instead of corner + width + height. (#687)

- **Breaking change**: Changed API of the following methods to take `SimpleCellRange` type argument: `copy`, `cut`, `getCellDependents`, `getCellPrecedents`, `getFillRangeData`, `getRangeFormulas`, `getRangeSerialized`, `getRangeValues`, `isItPossibleToMoveCells`, `isItPossibleToSetCellContents`, `moveCells`. (#687)

@wojciechczerniak FYI: The release note gets kinda long when we list all affected methods

@izulin Looking at the API ref, I think there are two more code examples that need updating. GitHub doesn't allow me to suggest changes to those lines, so I'm listing them here:

getRangeFormulas

In HyperFormula.ts, change line 2259 from:
* const rangeFormulas = hfInstance.getRangeFormulas({ sheet: 0, col: 0, row: 0 }, 2, 2);
to:
* const rangeFormulas = hfInstance.getRangeFormulas({start: { sheet: 0, col: 0, row: 0 }, end: { sheet: 0, col: 1, row: 1 }});

getRangeSerialized

In HyperFormula.ts, change line 2289 from:
* const rangeSerialized = hfInstance.getRangeSerialized({ sheet: 0, col: 0, row: 0 }, 2, 2);
to:
* const rangeSerialized = hfInstance.getRangeSerialized({start: { sheet: 0, col: 0, row: 0 }, end: { sheet: 0, col: 1, row: 1 }});

Co-authored-by: kirszenbaum <jakub.wisniewski@handsontable.com>

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

lgtm-com · 2021-06-14T13:38:58Z

This pull request introduces 2 alerts when merging ca619d0 into 9831e63 - view on LGTM.com

new alerts:

1 for Syntax error
1 for Unused variable, import, function or class

lgtm-com · 2021-06-14T14:39:20Z

This pull request introduces 1 alert when merging 47252a7 into 9831e63 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2021-06-14T14:47:51Z

This pull request introduces 1 alert when merging a0af181 into 9831e63 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

wojciechczerniak

At this point we validate all arguments? Maybe we have to update changelog entry "Added validation of API argument types for simple types." then?

wojciechczerniak · 2021-06-14T20:57:25Z

src/HyperFormula.ts

@@ -892,6 +905,9 @@ export class HyperFormula implements TypedEmitter {
   *
   * @param {Partial<ConfigParams>} newParams configuration options to be updated or added
   *
+   * @throws [[ExpectedValueOfTypeError]] when some parameters of config are of wrong type (e.g. currencySymbol)
+   * @throws [[ConfigValueEmpty]] when some parameters of config are of invalid value (e.g. currencySymbol)


"value empty" error for "invalid value" ? This line may not be correct

no, it's actually like that: if you try to provide "" as a currencySymbol then you get this error

src/HyperFormula.ts

wojciechczerniak · 2021-06-14T21:04:30Z

src/HyperFormula.ts

@@ -2699,26 +2754,30 @@ export class HyperFormula implements TypedEmitter {
   *
   * @category Helpers
   */
-  public getCellDependents(address: SimpleCellAddress | AbsoluteCellRange): (AbsoluteCellRange | SimpleCellAddress)[] {
+  public getCellDependents(address: SimpleCellAddress | SimpleCellRange): (SimpleCellRange | SimpleCellAddress)[] {


@kirszenbaum Probably for another issue, should we add another example with SimpleCellRange as input? Currently only SimpleCellAddress is demonstrated

@wojciechczerniak - sure, I added a note to the backlog: https://github.com/handsontable/hyperformula/projects/1#card-63587403

wojciechczerniak · 2021-06-14T21:05:09Z

src/HyperFormula.ts

@@ -2730,20 +2789,22 @@ export class HyperFormula implements TypedEmitter {
   *
   * @category Helpers
   */
-  public getCellPrecedents(address: SimpleCellAddress | AbsoluteCellRange): (AbsoluteCellRange | SimpleCellAddress)[] {
+  public getCellPrecedents(address: SimpleCellAddress | SimpleCellRange): (SimpleCellRange | SimpleCellAddress)[] {


@kirszenbaum Same here, we may show another way to use this method in the @example tag

@wojciechczerniak - sure, I added a note to the backlog: https://github.com/handsontable/hyperformula/projects/1#card-63587403

src/HyperFormula.ts

lgtm-com · 2021-06-15T19:03:23Z

This pull request introduces 1 alert when merging e2ced12 into 8f02f1d - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

src/HyperFormula.ts

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

lgtm-com · 2021-06-17T10:54:55Z

This pull request introduces 1 alert when merging 97ceab9 into ee22e12 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

wojciechczerniak

👍

addressed

API fix

faa5978

izulin requested a review from wojciechczerniak June 6, 2021 08:56

wszymanski mentioned this pull request Jun 8, 2021

Handling undo/redo actions using HF's UndoRedo mechanism #8078 handsontable/handsontable#8129

Merged

10 tasks

izulin added 5 commits June 8, 2021 20:22

refactor

e6628ab

big refactor

e788fca

not needed

81b95d6

changelog

5054b7a

Merge branch 'develop' into pu/apifix

0db693e

cleanup

53a5af5

.

3284b60

.

d3c36c1

wojciechczerniak suggested changes Jun 11, 2021

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

src/HyperFormula.ts Show resolved Hide resolved

src/HyperFormula.ts Outdated Show resolved Hide resolved

wojciechczerniak requested a review from kirszenbaum June 11, 2021 15:08

Merge branch 'develop' into pu/apifix

831818a

kirszenbaum previously requested changes Jun 14, 2021

View reviewed changes

izulin and others added 2 commits June 14, 2021 15:31

Update CHANGELOG.md

c81d8ab

Co-authored-by: kirszenbaum <jakub.wisniewski@handsontable.com>

Update src/HyperFormula.ts

ca619d0

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

izulin added 2 commits June 14, 2021 16:33

doc fixes

47252a7

docs

c4771e9

Merge branch 'develop' into pu/apifix

a0af181

izulin requested a review from wojciechczerniak June 14, 2021 14:41

wojciechczerniak suggested changes Jun 14, 2021

View reviewed changes

wojciechczerniak requested a review from kirszenbaum June 14, 2021 21:10

izulin commented Jun 15, 2021

View reviewed changes

src/HyperFormula.ts Outdated Show resolved Hide resolved

Update src/HyperFormula.ts

b86cb9c

izulin commented Jun 15, 2021

View reviewed changes

src/HyperFormula.ts Outdated Show resolved Hide resolved

izulin added 2 commits June 15, 2021 20:56

Update src/HyperFormula.ts

5c76a39

Merge branch 'develop' into pu/apifix

e2ced12

izulin requested a review from wojciechczerniak June 15, 2021 18:58

wojciechczerniak approved these changes Jun 17, 2021

View reviewed changes

izulin and others added 9 commits June 17, 2021 12:43

Update src/HyperFormula.ts

65777f6

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

3b99810

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

6365518

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

7f6db63

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

fb61109

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

f4382bb

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

acbd98d

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Update src/HyperFormula.ts

a3f4658

Co-authored-by: Wojciech Czerniak <wojciech.czerniak@gmail.com>

Merge branch 'develop' into pu/apifix

97ceab9

izulin requested a review from wojciechczerniak June 17, 2021 11:33

wojciechczerniak approved these changes Jun 17, 2021

View reviewed changes

izulin merged commit 6a4121a into develop Jun 17, 2021

izulin deleted the pu/apifix branch June 17, 2021 11:53

AMBudnik mentioned this pull request Jul 15, 2021

HyperFormula 1.0.0 has landed #769

Closed

kirszenbaum mentioned this pull request Jul 15, 2021

Docs: add SimpleCellRange examples #772

Open

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API fix #687

API fix #687

izulin commented Jun 6, 2021

codecov bot commented Jun 6, 2021 •

edited

Loading

lgtm-com bot commented Jun 9, 2021

lgtm-com bot commented Jun 9, 2021

lgtm-com bot commented Jun 10, 2021

lgtm-com bot commented Jun 10, 2021

wojciechczerniak left a comment

lgtm-com bot commented Jun 11, 2021

kirszenbaum Jun 14, 2021

kirszenbaum Jun 14, 2021

lgtm-com bot commented Jun 14, 2021

lgtm-com bot commented Jun 14, 2021

lgtm-com bot commented Jun 14, 2021

wojciechczerniak left a comment

wojciechczerniak Jun 14, 2021

izulin Jun 15, 2021

wojciechczerniak Jun 14, 2021

kirszenbaum Jun 22, 2021

wojciechczerniak Jun 14, 2021

kirszenbaum Jun 22, 2021

lgtm-com bot commented Jun 15, 2021

lgtm-com bot commented Jun 17, 2021

wojciechczerniak left a comment

	- Breaking change: Changed API of many function to take SimpleCellRange type argument instead of corner + width + height. (#687)
	- Breaking change: Changed API of the following methods to take `SimpleCellRange` type argument: `copy`, `cut`, `getCellDependents`, `getCellPrecedents`, `getFillRangeData`, `getRangeFormulas`, `getRangeSerialized`, `getRangeValues`, `isItPossibleToMoveCells`, `isItPossibleToSetCellContents`, `moveCells`. (#687)

API fix #687

API fix #687

Conversation

izulin commented Jun 6, 2021

Context

How has this been tested?

Types of changes

Related issue(s):

Checklist:

codecov bot commented Jun 6, 2021 • edited Loading

Codecov Report

lgtm-com bot commented Jun 9, 2021

lgtm-com bot commented Jun 9, 2021

lgtm-com bot commented Jun 10, 2021

lgtm-com bot commented Jun 10, 2021

wojciechczerniak left a comment

Choose a reason for hiding this comment

lgtm-com bot commented Jun 11, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

getRangeFormulas

getRangeSerialized

lgtm-com bot commented Jun 14, 2021

lgtm-com bot commented Jun 14, 2021

lgtm-com bot commented Jun 14, 2021

wojciechczerniak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lgtm-com bot commented Jun 15, 2021

lgtm-com bot commented Jun 17, 2021

wojciechczerniak left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 6, 2021 •

edited

Loading