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

Improve import/export functions for sheetjs with x-spreadsheet #2411

Closed
wants to merge 2 commits into from

Conversation

ThibautSF
Copy link
Contributor

Pull Request for #2410

Integration of improves import and export functions in x-spreadsheet demo from myliang/x-spreadsheet#419.

Allow optional arguments to support new QoL functionalities (default to previous behaviour)

  • keep merged cells between formats
  • choose to keep formulas (as string ie '=SUM(...)') or formula result (default behaviour)

Note: Incorporate suggestions by @Liyaa

@SheetJSDev
Copy link
Contributor

Thanks for looking into this!

Is there a reason not to enable keepMerges? (I think the option should be removed and it should always keep merges)

Is there a way to distinguish the text =1+1 from the formula =1+1? If not, the formula logic is fine.

Ideally we'd merge one commit, so please squash down

@ThibautSF
Copy link
Contributor Author

Is there a reason not to enable keepMerges? (I think the option should be removed and it should always keep merges)

I did that mainly to have the least interferences on the function behaviour. But indeed it can be removed (myself I never use with "false" value).

Is there a way to distinguish the text =1+1 from the formula =1+1? If not, the formula logic is fine.

I have some knowledge about x-spreadsheet due to developments around this lib. And while SheetJS has the cell.f attribute which helps to make the difference in stox(); on the other hand, I didn't see that kind of information in data from x-spreadsheet.
In fact, inside the lib, functions are calc based on the equal sign presence.

Ideally we'd merge one commit, so please squash down

Ok I will do that (after modifications for keepMerges option if you confirm it's needed)

@SheetJSDev
Copy link
Contributor

If x-spreadsheet detects functions based on leading =, then get rid of the option and always keep formulae. It would be better for stox to manually loop the worksheet instead of using sheet_to_json, but that can be fixed another time.

@SheetJSDev
Copy link
Contributor

5c02c81

@SheetJSDev SheetJSDev closed this Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants