Skip to content

Xls Reader Vertical Break and Writer Page Order#3306

Merged
oleibman merged 2 commits intoPHPOffice:masterfrom
oleibman:issue3305
Jan 21, 2023
Merged

Xls Reader Vertical Break and Writer Page Order#3306
oleibman merged 2 commits intoPHPOffice:masterfrom
oleibman:issue3305

Conversation

@oleibman
Copy link
Collaborator

Fix #3305. Xls Reader can set vertical break specifying row 0, causing an exception. It is doubtful that Excel needs a row for a vertical break; code is changed to use row 1 if the input file specifies row 0 (or lower). Code in question has not been exercised in unit test suite. Similarly, code to set horizontal break (which probably does not have a bug) is not exercised in test suite. Finally, page order in Writer incorrectly uses value in opposite way that Reader does. A new sample is added to illustrate that these are all handled correctly; it is easier to verify this by visually comparing the source spreadsheet and the copy made from it. A unit test is also added for the same spreadsheet to formally assert that the 3 properties in question are both read and written correctly.

This is:

- [x] a bugfix
- [ ] a new feature
- [ ] refactoring
- [ ] additional unit tests

Checklist:

  • Changes are covered by unit tests
    • Changes are covered by existing unit tests
    • New unit tests have been added
  • Code style is respected
  • Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
  • CHANGELOG.md contains a short summary of the change
  • Documentation is updated as necessary

Why this change is needed?

Provide an explanation of why this change is needed, with links to any Issues (if appropriate).
If this is a bugfix or a new feature, and there are no existing Issues, then please also create an issue that will make it easier to track progress with this PR.

Fix PHPOffice#3055. Xls Reader can set vertical break specifying row 0, causing an exception. It is doubtful that Excel needs a row for a vertical break; code is changed to use row 1 if the input file specifies row 0 (or lower). Code in question has not been exercised in unit test suite. Similarly, code to set horizontal break (which probably does not have a bug) is not exercised in test suite. Finally, page order in Writer incorrectly uses value in opposite way that Reader does. A new sample is added to illustrate that these are all handled correctly; it is easier to verify this by visually comparing the source spreadsheet and the copy made from it. A unit test is also added for the same spreadsheet to formally assert that the 3 properties in question are both read and written correctly.
@oleibman
Copy link
Collaborator Author

@MarkBaker Since other comments indicate you are ready for a new release, this probably ought to be part of it (corrects a problem introduced in 1.26 and one that may always have been wrong). I opened it so recently that I just want to make sure you have the opportunity to review before I merge it (or before you do).

@MarkBaker
Copy link
Member

@MarkBaker Since other comments indicate you are ready for a new release, this probably ought to be part of it (corrects a problem introduced in 1.26 and one that may always have been wrong). I opened it so recently that I just want to make sure you have the opportunity to review before I merge it (or before you do).

I'd like to get a new release out this weekend if I can, although there's a couple of issues that I'm currently trying to resolve so that they can also be included in that release, Hopefully I'll get them fixed over the weekend.

@oleibman oleibman merged commit 6443416 into PHPOffice:master Jan 21, 2023
@oleibman oleibman deleted the issue3305 branch February 10, 2023 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Row and column integer error when calling IOFactory::load

2 participants