-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
Autofilter Part 2 #2162
Autofilter Part 2 #2162
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
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 |
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?