Two Cell Anchor Drawing #2632
Closed
Two Cell Anchor Drawing #2632
Conversation
This file contains hidden or 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 change was introduced by @naotake51 as PR PHPOffice#2532, but it was closed before I had a chance to review it, and I have been unable to communicate with the author since. I will gladly defer to the original. Please see the original PR for details. Similarly, PR PHPOffice#2237 (@AdamGaskins) is open, but with changes requested for several months. It covers a lot of the same ground as 2532. It makes sense to me to combine them. To those changes I added support for the 'editAs' attribute - I imagine that there are instances of 'absolute' out in the wild. There have been several other tickets referencing two cell anchors, including issue PHPOffice#1159 and PR PHPOffice#1160 (@sgarwood, who also added support for editAs), PR PHPOffice#643, and issue PHPOffice#126, all now closed but not necessarily entirely resolved. I will try to ensure that those tickets are addressed with this one. And, in trying to make sure 1160 is covered, I stumbled upon a bug. If you use the same image resource to create two+ memory drawings, the MemoryDrawing destructor for the first will cause the rest to generate a very long warning message. This is not a problem for Php8+, only for Php7-. I have suppressed the message in the MemoryDrawing constructor. 1160 went stale due to an unresolved test error, but I don't think this was the problem. At any rate, its test works now.
|
Interesting. 20_Read_Xls and 30_Template both fail in PHP8.1. This is new, but ... they both create corrupt Xlsx files in the current release. I need to figure out those problems. |
Xls Reader can read drawing offsetX and offsetY as float. However, Excel Xlsx (and PhpSpreadsheet) wants them only as int. This leads 20_Read_Xls and 30_Template to produce corrupt Xlsx files for any Php release. Thanks to moving the typehinting from docblock to function signature, this showed up as an error in Php8.1 unit tests. Change Xls Reader to treat values as int, eliminating both the Php8.1 problem and the corrupt files. Running the complete unit test suite in 8.1, I get a handful of errors on my system which are not (yet) showing up on Github. This is mostly the usual "null supplied where string is expected". However, a new problem shows up - `float % 2` gets an "implicit conversion from float to int" warning. These problems are all fixed with minor changes for this commit.
|
Some functions that need to be changed have moved to different locations. This leads to merge conflicts which are too difficult to resolve. I will rebase and try again later. |
5 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This change was introduced by @naotake51 as PR #2532, but it was closed before I had a chance to review it, and I have been unable to communicate with the author since. I will gladly defer to the original. Please see the original PR for details.
Similarly, PR #2237 (@AdamGaskins) is open, but with changes requested for several months. It covers a lot of the same ground as 2532. It makes sense to me to combine them.
To those changes I added support for the 'editAs' attribute - I imagine that there are instances of 'absolute' out in the wild.
There have been several other tickets referencing two cell anchors, including issue #1159 and PR #1160 (@sgarwood, who also added support for editAs), PR #643, and issue #126, all now closed but not necessarily entirely resolved. I will try to ensure that those tickets are addressed with this one.
And, in trying to make sure 1160 is covered, I stumbled upon a bug. If you use the same image resource to create two+ memory drawings, the MemoryDrawing destructor for the first will cause the rest to generate a very long warning message. This is not a problem for Php8+, only for Php7-. I have suppressed the message in the MemoryDrawing constructor. 1160 went stale due to an unresolved test error, but I don't think this was the problem. At any rate, its test works now.
This is:
Checklist:
Why this change is needed?