There are severe inefficiencies in `toArray` (or, to be more precise, `rangeToArray`, which toArray calls). For small ranges, the inefficiencies hardly matter, and, for large ranges, the code was likely to exhaust memory before the execution speed mattered. But, with PR #3834 now merged into the codebase, the memory problem is much lessened, and there is now merit in looking into performance.
The code currently accesses each cell in the range. This PR changes it to access only cells which have been initialized - fewer cells processed, and many fewer function calls for those that are. For ranges which are densely filled, there will be little difference. However, for ranges which are lightly filled (a very common occurrence in issues which are raised for this project), the effect will be staggering. The sample file which accompanied 3834 saw an enormous reduction in memory consumption from 20GB to 32MB when toArray was used on it. The same file still took well over an hour to process even after the merge; after this PR, it is processed in approximately a second.
While testing this change, it became apparent that Collection/Cells does not handle column XFD (highest allowed) correctly. That is corrected and new tests added.
Fix#2809. User felt font files should be searched recursively. The problem could be overcome merely by specifying a more precise font location. However, user has a point - the file layout on user's system is pretty common, and can be integrated into PhpSpreadsheet code easily. As a bonus, the DocBlock for setTrueTypeFontPath, which the user felt was misleading, doesn't even have to change.
Fix#3819. Excel can add these prefixes (basically invisible to end-user). Formula translation in PhpSpreadsheet fails when dealing with these unexpected prefixes, and, even if it handled it correctly, the unexpected prefixes confuse the users. I have changed to strip those prefixes when translating to a locale. This is probably not perfect, but is almost certainly good enough. I could easily add the same change when translating from a locale to English, but I don't think there's a good use case for that, so am opting not to do so for now.
The documentation mentions `translateFormulaToLocale` and `translateFormulaToEnglish`. Neither of these exist; both names are preceded by an underscore. I have changed the code to match the documentation rather than vice versa, retaining deprecated versions of the underscored routines which merely invoke the non-underscored routines.
readthedocs.org build every commit pushed on `master`. However
builds have been failing for the past two months, because they
now require a config file. I assumed that config file was added,
but I was wrong. So I added it.
Fix#3820. When cloning a worksheet, special handling is already included for `cellCollection` and `drawingCollection`. It needs to be added for `tableCollection` as well. In theory, `chartCollection` also needs it. However, there are no clone methods for any of the Chart objects, so that will be a substantially larger effort. There is no need to delay this fix waiting for that.
* Automate Function-by-Name and Function-by-Category Docs
Examining whether it is possible to analyze the 2 Php scripts in the bin directory even though they lack a .php extension, I found that the docs generated by `generate-document` had not been updated in over a year even though support for a number of functions (e.g. ARRAYTOTEXT) had long ago been implemented. The files generated by the other script `generate-locales` are automatically generated as part of the unit test suite, and I can't see any reason not to do the same for `generate-document`.
It isn't totally clear that the 2 scripts in bin are needed any more now that each has an equivalent in the unit test suite. For now, I'm keeping them, and allowing them to be analyzed by phpcs, php-cs-fixer, and phpstan.
* Reflection Not Required
Remove it from bin scripts and DocumentGeneratorTest.
* Better Html/Css Display for Function Lists
* Redesign Features Cross Reference Doc
It was inaccurate, and pretty much impossible to read in entirety. This version should be easier to read, and to correct. That's important, because there are almost certainly new inaccuracies in this version.
* Add Html to Features Cross-Reference Readers
* Reader Xls Dead Code
Recent changes left Xls Reader with a ton of Scrutinizer messages. I have filtered away the false positives. Most of what is left is dead code, and this PR addresses those. Most of those are used to parse the BIFF data, but we either don't need to implement, or haven't yet implemented, those particular parsed data items. I think it is more useful to comment those out for documetary purposes rather than deleting them.
* Missed a Few
This was suggested by the investigation of issue #3811. No fix is necessary for the issue. However, two possible code solutions (Php setlocale, which comes with certain design flaws, and StringHelper set(Decimal/Thousands)Separator were suggested, and neither is adequately tested. This PR adds such tests.
Unusually, getting StringHelper Decimal Separator, Thousands Separator, and Currency Code can result in a change to those properties. So, the existing design in several tests where those properties are captured in Setup and restored in Teardown do not work quite as designed. Instead, the ability to set those properties to their default value (null) is added, and the tests re-done to restore the default in Teardown.
The two methods yield the same results when parsing input. However, they diverge when examining output fields through `getFormattedValue`. Such output is currently correct (usually) when using setlocale, but not when using StringHelper. The former works through the 'trick' of using `sprintf(%f)`, which generates a locale-aware string. However, using non-locale-aware `sprintf(%F)` followed by `str_replace` will produce the correct result for both setlocale and StringHelper. One place in the code uses a cast to string, which is incorrect for both methods. Following that up with the same str_replace makes it correct for both. These changes permit, but do not require, the user to avoid setlocale altogether.
It remains an open question whether Settings/Calculation::setLocale should set DecimalSeparator, CurrencySeparator, and CurrencyCode. That makes logical sense, but it would be a breaking change, and having to explicitly set those values when using setLocale does not seem especially burdensome. For now, such a change will not be made.
* Let Phpstan Run on Samples
Phpstan currently analyzes all source and test members. We already run phpcs and php-cs-fixer on samples as well. I would expect that samples are often used as templates for code in userland; it behooves us to be at least as careful with those members as for the others which are already being analyzed. Aside from 1300+ messages `Variable $helper might not be defined.`, which will be suppressed in phpstan.neon.dist, there are really only a few changes needed for sample members, so that part of the code base was already in good shape, and is now even better. No annotations were needed.
* Scrutinizer 2 out of 3
1 false positive, now suppressed; fix other 2.
* Remove Dead Code
* Very Minor Changes
* Add infra
oleibman
sji
dependabot[bot]
Adrien Crivelli