Merged
Conversation
Most of the remaining 32-bit-unsafe date handling that remains in PhpSpreadsheet is in AutoFilter. Cleaning this up demonstrated that there are a lot of problems with AutoFilter, and I will do it in two pieces. Part 1 was PR PHPOffice#2141 which I have just merged. In this PR: - Fix remaining 32-bit dates in filterTestInDateGroupSet. - Also in some of the existing AutoFilter samples. Note that the comments in two of those said the filter was being set for the first day of each month, but the code specifies the last day - I have corrected the comments. - Remove mocking in unit tests for AutoFilter in favor of 'real' tests. - Code coverage is now 100% in all of AutoFilter, AutoFilter/Column, and AutoFilter/Common/Rule. - No remaining AutoFilter(/Column(/Rule)) exceptions in Phpstan baseline. - Documentation for escaping of asterisk, question mark, and tilde in text filters included spurious backslashes which are now removed. - Text filter escaping of question mark did not work. There had been no unit tests for any text filtering. - Likewise there had been no testing for TopTen. - Above- and below- average filters were not working because they acquired their Calculation instance incorrectly. There had been no tests. - Several unchanging private static arrays in Rule were changed to private const arrays. - Clones are now tested. - RuleTest is moved to same directory as other tests.
24 minor problems, almost all of them unused code in tests.
MarkBaker
requested changes
Jun 16, 2021
Member
MarkBaker
left a comment
MarkBaker
left a comment
There was a problem hiding this comment.
Id recommend switching to use of the WildCard class for handling wildcards in text searches, because it handles a lot of edge cases that this original simplistic method didn't take into account
Per suggestion from @MarkBaker. WildcardMatch did not handle double tilde correctly. It has been changed to do so and its logic simplified (and commented). Existing AutoFilter test covered this situation, but I added a test for MATCH as well.
Closed
MarkBaker
approved these changes
Jun 24, 2021
Member
|
Note that we have a few issues with PHP8.1 nightly that will need addressing before the planned November release of that PHP version; although some of these issues are with phpunit dependencies |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Most of the remaining 32-bit-unsafe date handling that remains in PhpSpreadsheet is in AutoFilter. Cleaning this up demonstrated that there are a lot of problems with AutoFilter, and I will do it in two pieces. Part 1 was PR #2141 which I have just merged.
In this PR:
This is:
Checklist:
Why this change is needed?