Fix#4061. CONCATENATE, which has slightly different behavior than CONCAT, and which MS has deprecated for that reason, behaves in an unexpected way when a cell range is presented to it and the spreadsheet does not allow for array results. This would almost certainly occur only for Legacy spreadsheets, but such is what was presented in the issue. The code is changed so that when an array of cells is presented to CONCATENATE, and RETURN_ARRAY_AS_VALUE is in effect, the array will be treated as if it were wrapped in the SINGLE pseudo-function (which is what Excel does by somewhat mysteriously prefixing the cell range with `@`).
This is a niche case. This one stands out because of its deprecation and replacement function. It is possible that other functions exhibit this behavior. I have made no attempt to identify others. A similar approach can probably be applied if issues are raised for others.
Fix#606 (marked stale in 2018, but now reopened). User reports that, under some circumstances, `oldCalculatedValue` will be used as the result when an unsupported function is part of a formula (this seems like a sensible treatment), but, in other cases, the unsupported function is ignored, or has an unpredictable effect on the result.
To explain a little further, an unsupported function will return a "magic" value which will eventually be replaced by `oldCalculatedValue` (or null if there is none). However, the magic value is not being propagated. In the issue, there are 3 places where this propagation is currently omitted - when performing a binary comparison, when evaluating the condition in the `IF` function, and when evaluating the `SUM` function. This PR causes propagation to happen in all of those places, and a few others (MIN, MINA, MAX, MAXA, ROWS, COLUMNS). I would not be surprised to have missed one or more of the other situations.
Fix#820, which went stale in 2019 and is now reopened. Another weird Excel implementation detail. When SUBTOTAL's first argument is 101 through 111, it excludes all hidden rows. When that argument is 1 through 11, it excludes all rows which are hidden due to an auto-filter, but includes rows which were manually hidden. PhpSpreadsheet had not been distinguishing between the two types of "hidden-ness". Add a new property to RowDimension to indicate whether it is hidden due to an auto-filter, and use that property when evaluating SUBTOTAL 1-11.
Fix#954, which went stale 6 years ago and which I have now reopened. The `setLocale` method in `Calculation` and `Settings` does not affect the 3 localizable properties in `StringHelper` - `currencyCode`, `decimalSeparator`, and `thousandsSeparator`. One way to work around this problem is to set those properties separately; this is probably how most people handle it. Another way is to call Php's own `setlocale` function; this does not require any foreknowledge of what the values need to be, but it comes with baggage (data is maintained at process level rather than thread level), so its use is discouraged.
This PR adds a new `setLocale` method to `StringHelper`. It sets the 3 properties and the `Calculation` language all at once. It depends on the `Intl` extension, which is a recommendation but not a formal requirement for PhpSpreadsheet; the method will return `false` if `Intl` is not available or it thinks the supplied locale is not valid.
This PR started because the documentation on how to configure an Http client was out of date. However, as I investigated further, I found problems. GuzzleHttp, a client mentioned in the documentation, for example - it probably works fine when executed from a browser, but it does not work for `https:` requests from the Windows command line. See https://github.com/guzzle/guzzle/issues/2601, where a user notes that Guzzle's own program to demonstrate how to use it doesn't work, a result that I can confirm is still true. A linked explanation says that the solution is to explicitly specify a path to a CA bundle. For starters, this is impractical from our perspective. One possible solution is to change a php.ini option which is not needed for any other purpose, and which probably needs to change frequently - a burden on users who follow that route. An alternative solution is to use a method `request` to specify the path to the certificate store; this also will need to change from time to time, and, worse, the only method defined in ClientInterface is `sendRequest`, so using this solution isn't client-agnostic, which is a stated goal of https://github.com/PHPOffice/PhpSpreadsheet/commit/7cb4884b96174eb611621635e6fb603ef54178f7. Additionally, it is not clear why an external interface is needed rather than a call to file_get_contents, used elsewhere in PhpSpreadsheet, and not requiring a path to a certificate store.
I also believe that automatically evaluating WEBSERVICE for any arbitrary argument is not a good idea. I am adding a domain whitelist which the user must populate. For domains not in the whitelist, the calculation will revert to `oldCalculatedValue`, which is good enough for pass-through purposes, which probably encompasses most cases. That is how Excel behaves by default - it disables WEBSERVICE calls when it opens a spreadsheet which contains them. For cases where the user adds a new WEBSERVICE call, there is a choice of whitelisting the domain, or getting the result in some other way and using `setCalculatedValue` to store it.
Finally, when a WEBSERVICE call *is* evaluated, it will now accept a cell-address argument rather than just a literal string as is now the case.
T.DIST takes 3 parameters. When the 3rd parameter, a boolean, is true, we now have code to support the request. We still do not have code to handle the false case.
Fix#4167. That problem was initially reported as a problem with TINV, however, at its root, it was actually a problem with TDIST (which TINV needs to call). Both of these are deprecated by Microsoft in favor of some newer functions. PhpSpreadsheet incorrectly treated T.INV as an alias for TINV, and did not implement T.INV.2T, which actually is an alias for TINV.
To solve this problem, I adapted the Perl CPAN module `Statistics::Disttributions`, written by Michael Kospach around 2003. Unit test results have been manually checked against Excel; a few had to be changed because they were wrong (see the original issue). I believe that all of TDIST, T.DIST.2T, T.DIST.RT, TINV, T.INV, and T.INV.2T now function correctly, at least most of the time. I have not found code or an algorithm that I can use for T.DIST nor for T.TEST, both of which remain unimplemented.
Current tests use only direct calls. Using them in spreadsheet context would be better.
There were some minor bugs. Excel treats an argument consisting of a single-cell as if it were a range; PhpSpreadsheet, which currently returns the content of the single cell, is changed to match that behavior, which is important if one of the other arguments causes a VALUE error.
Excel can do a column sort in SORTBY, despite the fact that Microsoft doesn't even mention that possibility in its documentation. I found documentation of that possibility on a 3rd-part site https://exceljet.net/functions/sortby-function, confirmed it in Excel, and took my best guess as to how to implement it. The existing test member for SORTBY included one such test, but it succeeds mostly by accident. The new member has a more reliable test.
The tests stay; they just no longer run in separate processes. I think that only 2 tests still require separate processes - Helper/SampleTest and Tcpdf/NoDieTest. Aside from a new internal method in IOFactory, all the changes here are to test members.
PhpSpreadsheet treats CODE and UNICODE as equivalent, likewise for CHAR and UNICHAR. They are, in fact, different. CODE and CHAR deal only with single-byte character sets (Windows-1252 or MacRoman), while UNICODE and UNICHAR deal with all of Unicode. This PR separates them. The existing unit test for CODE was, in many cases, applicable to UNICODE (for which there was no separate test). The tests are corrected for CODE, new tests are added, and a separate test for UNICODE is added. CHAR was mostly okay, new tests are added, and a separate test for UNICHAR is added.
They aren't quite interchangeable. Both are used in the test suite, with no indication of why one or the other. I think we'd be best off being consistent. Based on the names, I think `_calculateFormulaValue` was intended as a private, or at least internal, method, so favor `calculateFormula`. I do not intend to rename or re-categorize `_calculateFormulaValue`, just remove its usage when it isn't clearly warranted.
Fix#4656. (Also fix#503, which went stale years ago, and which I reopened, and which I re-closed in a state of confusion.) Continuing the work of PR #4596. Calculation engine was unable to parse a formula which used union arguments. (I find it very difficult to parse as well.) This PR will, I hope, fix that. More tests are needed.
This is a theoretical problem. Building FalseTrueArray only looks at languages, not countries. The only example we have is `pt` vs `pt_br`, and since True and False have the same translation for both locales, there is no problem. This PR will help it from becoming a problem in future if a new language/country combo is added.
oleibman
zhukoff
zhukoff74
Robin van der Vliet