Before it was a bug when flooring negative numbers added to the length of
the number for the sprintf mask.
For example -0.091 becomes -9.1 in percentage, floors to -10 which is going to
increase $wholePartSize by 1 and add a leading zero to the result: -09.1% with
format 0.0%.
This happened for negative ranges where floor will increse the length of
the original number.
Fix#3909. SUMPRODUCT is mishandling multi-row ranges. In Calculation/Calculation, `checkMatrixOperands` will often resize its operands. When it does so, it needs to recalculate the dimensions of each. This fixes the reported problem.
Likely cause was PR #3260. That ticket noted the poor coverage of the code being replaced. Tests of the problem in this ticket were absent and are now added. Despite this, I note that `resizeMatricesShrink` is virtually uncovered, and `resizeMatricesExpand` has substantial gaps in its coverage. I have covered some, but not all, of the Expand gaps. I am struggling to come up with examples to fill its remaining gaps and those for Shrink. However, I will merge this fix in about a week even if I don't succeed.
Fix#3907. After row/column insertion/deletion, PhpSpreadsheet updates formulas which include cells which have moved. However, it can mis-identify cell addresses within the formula. Examples:
- `=SUM(A2,'F1 (SETTINGS)'!A1:B1)` It identifes F1 as a cell address.
- `=SUM(A2,'x F1 (SETTINGS)'!A1:B1)` It identifes F1 as a cell address. (This looks the same as the above, but, for technical reasons, it's different.)
- `=SUM(A2,definedname1A1)` It identifes A1 as a cell address.
- Sheet names in formulas are compared case-sensitively, and should be compared insensitively. This can make a difference if the formula includes its own sheet name, e.g. on sheet `Data`, formula `=SUM(DATA!A1:A2)` might have to change, but it will not do so with the existing logic.
The defined name part is fairly straightforward. The regular expressions that identify a cell address just have to be a bit more robust. It was doing a negative look-behind for an alphabetic character or dollar sign; underscore, period, and digits, all of which can be part of a defined name, need to be added to that list.
The other situations need a bit of a kludge, but not one so bad that I'm ashamed of it. The formulas will be altered before analysis so that sheet names are replaced with Unicode FFFD (sheetname does not match current sheet), FFFC (sheetname, enclosed in apostrophes, matches current sheet), and FFFB (sheetname, not enclosed in apostrophes, matches current sheet). This prevents the existing regular expressions from finding a cell address within a sheet name, and makes it easy to restore the original, with or without apostrophes, when the sheet name matches the current sheet and the cell(s) which it qualifies have to be changed.
Tests are added for all the situations mentioned above. No existing tests required changes.
Fix#3900. The code to adjust Decimal and Thousands separators to user-specified choices expects to convert periods to commas and vice versa - hard-coded without taking the user specification into account. This is likely to be the only significant use case, however, as the issue demonstrates, it is not the only possibility. In particular, it will mess up Csv output if decimal separator is kept as period, but thousands separator is set to null string. The code is altered to replace period with the user-selected decimal separator, and comma with the user-selected thousands separator. No existing tests broke as a result of this change in behavior.
This PR resolves about half the statements which we tell Phpstan to ignore. There will still be 23 such annotations spread over 10 source modules for various reasons (too complicated, suspect PhpSpreadsheet code, one Phpstan bug), plus some deliberate errors in the test suite.
The samples have become unwieldy when running them from a browser. In particular, the drop-down lists are fixed size with no scrolling, and many of them are now just too large. I have moved all the Calculation samples up a level, and broken several categories (Basic, Chart, DateTime, Engineering, Financial, and Reader) into several pieces.
Convert-Online (now found in the Engineering category) had a number of different problems which are now resolved. It is the only member with any significant code change.
Fix#3875. Even better, fix#2146, which has been open for 2.5 years.
Empty arguments are improperly placed on the stack; in particular, they are added without `onlyIf` and `onlyIfNot` attributes.This results in problems described in 3875.
IF has a somewhat unexpected design. In Excel, `IF(false, valueIfTrue)` evaluates as `false`, but `IF(false, valueIfTrue,)` evaluates as 0. This means that IF empty arguments should be handled in the same manner as MIN/MAX/MINA/MAXA, but you need to be careful to distinguish empty from omitted.
Also note that IF requires 2 operands - `IF(true)` is an error, but `IF(true,)` evaluates to 0.
I check from time to time. There are a number of problems now, mostly due to the elimination of Php 7.4 and replacement of doc-block typing with explicit Php typing.
- Bitwise functions were particularly affected by PR #3718 and PR #3793.
- Chart/Axis and Writer/Xlsx were amusingly affected by PR #3836, which added a scaling option which included an array indexed by the known allowable factors, one of which is 1 trillion, which cannot be represented as an integer on a 32-bit system. Issue3833Test, introduced by the same PR (and not suffering any errors) was expanded to test this value.
- Some minor changes to Reader/Xls and Shared/OLE/PPS to accommodate hex values which are negative in 32-bit but which Php-32 may wind up casting to large floating point numbers; it is not clear to me why these hadn't shown up as problems previously. Possibly this is the result of changes in the most recent Php versions.
- BitAndTest, BitOrTest, BitXorTest and Shared/DateTest were adversely affected by PR #3859 when arguments and/or expected results too large for a 32-bit integer were supplied.
- ImExpTest required a slightly reduced precision for 32-bit. No idea why this hadn't shown up earlier.
Fix#3866. Excel normally treats a missing argument (e.g. `PRODUCT(2,3,)`) as null and ignores it. Not so for MIN/MAX/MINA/MAXA; for those, an empty argument is treated as zero. PhpSpreadsheet is changed to do the same.
Fix#3863. Data Validation default operator is `between`. When Excel writes out a data validation item, it may omit the operator. Xlsx reader will therefore initialize operator to null string. Issue indicates that user wants `between` returned for `getOperator`. A more serious problem is that `isValid` method does not handle this situation correctly. Data Validation is changed to set Operator to the default value if an attempt is made to set it to null string.
Change "mixed" declarations to more accurate types in test members; in particular, change those that would be flagged if we were to run Phpstan at level 9 (we currently run level 8). I may or may not follow up with source code (over 700 level-9 problems remain for src), but, as with strict typing, there is no reason to avoid the effort for test members.
It was necessary to update some doc blocks in src to accommodate this change. However, no executable code is touched.
Fix#3687. Worksheet methods insertNewRowBefore and insertNewColumnBefore call ReferenceHelper insertNewBefore. That function fills in "missing" cells with null values. However, for boundaries, it uses getHighestRow and getHighestColumn. It should be sufficient to use getHighestDataRow and getHighestDataColumn. When there is a big gap between getHighest... and getHighestData..., this can result in a big increase in memory usage, and in file space when saving the spreadsheet. New test InsertTest demonstrates the problem by populating a worksheet with cells A1:D5 (so highestDataRow is 5), but also setting row 1000 to invisible (so highestRow is 1000).
The major part of the change is in ReferenceHelper::insertNewBefore, which will now use getHighestData... for its boundaries when filling in the missing cells. Changes of less impact are made to duplicateStylesByColumn and duplicateStylesByRow so that cells which don't yet exist are not created unless the style that will be applied is not the workbook default style.
As for reducing the file size, Writer/Xlsx/Worksheet is changed so that cells whose value is null or null-string and which use the workbook default style are not written to the output spreadsheet. This requires some changes to existing test ReadBlankCellsTest; I don't think the difference should matter to the end-user.
Fix#3847. Person posing the question calls this behavior "pull formulas", and wants equivalent handling to be available in PhpSpreadsheet. I can do this for formulas in a single cell; I can go horizontal or vertical or (unlike Excel) both. Nevertheless, I call this "partial support" because I cannot think how I can do something similar that Excel does, e.g. putting 2 in cell A1 and 4 in Cell A2, selecting them both and using the fill handle to extend those to rows below, so that cell A5 will contain 10.
Fix#3843. Spreadsheet XML had a lingering Xml drawing tag despite having no drawings. When a new drawing was added, PhpSpreadsheet became confused about which drawings to keep. This PR ensures that it keeps the new and drops the old.
There are different rules for Title between Excel and Html, in particular maximum length and which characters are valid. Html Reader can throw an exception because of this difference. This PR allows it to tolerate an invalid title, retaining the default title if the Html suggests an invalid one.
These glitches were probably due to PR #3622.
- Excel may not properly identify the selected cell on a sheet if freeze panes is in effect, even though the Xml points to the correct pane and the correct selected cell is specified for that pane.
- Worksheet::calculateColumnWidths may alter activePane.
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#3833. Among the options for chart axes in Excel are the ability to show labels as multiples of specific powers of 10, with or without an explanatory label. The Excel spec seems to indicate that that you can customize the multiples to other values, but I don't see how to do that in Excel, so that can be a project for another day if someone figures out how. The label title can also be styled; that is not part of this PR, but I will look into it in future. For now, this PR supports the use of multiples with or without labels.
The axes can also be displayed on a logarithmic scale (any base between 2 and 1000). This PR supports that.
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.
oleibman
Nicholas Ruunu
Adrien Crivelli