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

Xls Conditional Border #4033

Merged
merged 4 commits into from
May 26, 2024
Merged

Xls Conditional Border #4033

merged 4 commits into from
May 26, 2024

Conversation

oleibman
Copy link
Collaborator

@oleibman oleibman commented May 21, 2024

Xls Writer Conditional Border had been creating corrupt spreadsheets. This was mainly because a pack statement that should have specified V instead specified v. Even changing that, the logic was still slightly wrong on write, and missing altogether on read. This PR corrects the write problems and adds the missing read code. It also adds italic and strikethrough support for Xls Writer Conditional Font (read code was already in place). With this, Xls Conditional Writer is completely supported except for NumberFormat. Xls does support that, but I cannot figure out how from the available documentation.

This is:

  • 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 and a link to the pull request if applicable
  • 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.

Sorry, something went wrong.

Xls Writer Conditional Border had been creating corrupt spreadsheets. This was mainly because a pack statement that should have specified `V` instead specified `v`. Even changing that, the logic was still slightly wrong on write, and missing altogether on read. This PR corrects the write problems and adds the missing read code. It also adds italic and strikethrough support for Xls Writer Conditional Font italic and strikethrough (read code was already in place). With this, Xls Conditional Writer is completely supported except for NumberFormat. Xls does support that, but I cannot figure out how from the available documentation.
@oleibman oleibman mentioned this pull request May 21, 2024
8 tasks
@oleibman
Copy link
Collaborator Author

Scrutinizer's 2 new issues are both false positives and will be suppressed.

oleibman added 2 commits May 24, 2024 07:58
Xls Reader processing Conditionals interferes with the previously established SelectedCells. Make sure that value is restored.

StopIfTrue should always be set for Xls spreadsheet.

Set NoFormatSet to true unless any of Font, Fill, or Borders is specified in Conditional Style.

In my notes for PR PHPOffice#3372, I mentioned that I could not include some Xls tests because of errors in the software at that time. This PR fixes those errors, so I am adding the missing test, and making the equivalent Xlsx test more comprehensive.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@oleibman oleibman added this pull request to the merge queue May 26, 2024
Merged via the queue into PHPOffice:master with commit 68218c1 May 26, 2024
12 of 13 checks passed
@oleibman oleibman deleted the condxlsborder branch May 26, 2024 02:20
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.

None yet

1 participant