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.
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.
Its use is already causes an issue with Phpstan. It uses interfaces marked as internal by Phpunit, and it will not work with Phpunit 12. It is more complicated than needed. This PR corrects all these problems. It also corrects a handful of other problems that will show up with Phpunit 12. Only tests are changed - no source code.
Fix#1632. Excel automatically supplies a style for cells which contain hyperlinks (e.g. underlined, and blue text changing to purple after the link has been followed). PhpSpreadsheet cannot handle the style automatically. The user can, with some effort, specify a style for the cell which mimics Excel's choice (except for the color change after following).
Examining a sheet with a hyperlink created through Excel, it appears that Excel creates 3 different entries in styles.xml, one for cellStyleXfs, one for cellXfs, and one for cellStyles. It is difficult for me to figure out how they interrelate. This is especially so since PhpSpreadsheet outputs only 1 entry (for the default style) for each of cellStyles and cellStyleXfs. However, it appears that only the cellXfs entry is required, and, when it specifies a font whose color specifies `theme="10"` rather than an rgb value, the style works as expected.
In order to implement this, it is necessary to add a `theme` property, with setter and getter, to Style/Color. There are 12 possible values for theme, 0-11 representing 0=dk1 1=lt1 2=dk2 3=lt2 4-9=accent1-6 10=hlink 11=folHlink. This PR is mainly to allow the use of hlink, but the others are also usable if a use case arises for them. If a theme is set for Color, Xlsx Writer will use the theme rather than rgb when generating the color xml. Other writers will continue to use rgb rather than theme, so there is a use case for setting both if you want to generate both Xlsx and some other format.
The `theme` property will, for now, be ignored except for Font. There is probably a case to be made for using it for Fill, and maybe for Border and other areas that I haven't yet considered. I will wait for someone to make that case before adding those.
In order to make it as easy as possible to use this, a method `setHyperlinkTheme` is added to both Style/Color and Style/Font. The one in Color sets `theme` to the appropriate value. The one in Font calls the one in Color, and also sets `underline` on (this will be honored by other writers in addition to Xlsx).
Samples which use hyperlinks are updated to use `setHyperlinkTheme`. So is `Reader\Xlsx\HyperlinkTest`, with appropriate tests added.
Fix#4362. A similar problem to 4360, Style not handling sheet name with embedded apostrophe properly. And, with two examples in hand, I was able to determine a pattern to find and fix other possible exposures.
When the Dynamic Array PR #3962 was introduced, it left the default as Return Array as Value. At some point, the default should be changed to Return Array as Array. This would, of course, be a breaking change, one which will not be part of Release 4. However, it will possibly be part of Release 5.
Rather than relying on the default setting, this PR explicitly sets Return Array as Value when tests require that setting. This will make it easier to identify potential breaks when the default is changed. The entire test suite will now succeed with either setting as default.
In making these changes, a few minor problems were discovered with how Array as Array is handled. These are fixed with this PR.
With PhpUnit 10 came the ability to use Php attributes rather than doc-block annotations for things like "data provider". PhpUnit 11 deprecates the use of annotations, and PhpUnit 12 will not not permit their use. Since PhpUnit 11 requires Php8.2+, we cannot adopt it as long as we support Php8.1, which will continue to be the case for some time. However, there is no penalty for early adoption.
Php-cs-fixer can use:
```
'php_unit_attributes' => ['keep_annotations' => false],
```
This allows us to run `composer fix` to automate all the needed changes. No manual changes were needed for any of the test members.
With this change, PhpUnit 9 can no longer be used with the test suite. File composer.json is updated to reflect that reality, and phpunit9.xml.dist, which has been supplied in case anyone needed to use PhpUnit 9, is no longer required, and is thus deleted. For now, PhpUnit 11 is not being added as a possibility.
No source code is changed in this PR.
Till now we have used a static variable/getter/setter to decide what type of result should be returned when a formula is evaluated and an array is the result. This is messy; it would be much better to use an instance variable instead. We cannot eliminate `setArrayReturnType` and `getArrayReturnType` because that would be a BC break. I am considering whether they should be deprecated. In the meantime, I have added a new instance property `instanceArrayReturnType` with getter and setter methods. The property is initially null, and, if it remains so when needed, the static property will be used instead. However, if it is set, its value will be used.
Some examples submitted by @infojunkie have demonstrated that an anchor cell without a spill operator was not being handled consistently with Excel. This means that the calculated value of such a cell needs to be a scalar when using it as part of a formula without the spill operator, but as an array when using it with the spill operator or when just getting the cell's value. This is tricky. My solution seems awfully kludgey to me, but it does seem to work.
I have another change that I will want to make in a day or so. When that change is pushed, I will take this back out of draft status, and will now aim for an install date of about August 6.
This particular commit removes the changing of array return type during calculations. It's been this way for a very long time, but I don't understand why it should have been needed in the first place. It causes problems (you need to be sure to restore the original value even when, for example, you throw an exception during calculation). I am relieved that it caused a problem only for one test member. The TEXTSPLIT function really makes sense only when you are returning arrays as arrays. Its test now needs to set that value explicitly since the Calculation engine is no longer changing it under the covers. No other tests broke as a result of this change.
One other test "broke" as a result of this commit. One of the tests for INDIRECT had been expecting a null value, and the test was commented with "Excel result is 0". The PhpSpreadsheet result is now 0 as well, so this would seem to be a bugfix rather than a breaking change.
Spill operator now works both as trailing `#` and ARRAYANCHOR function. `#` is converted to ARRAYANCHOR when writing. I do not think it is important to convert the other way when reading.
Documentation updates have started, but are a work in progress.
SINGLE function is implemented. I believe it works correctly when referring to a cell, but not when referring to a cell range. No attempt is yet made to convert leading `@` to and from SINGLE; I haven't figured out how to do so without interfering with `@` in structured references.
ISREF has problems. At least one of its tests was wrong, and many of those that were right were so accidentally. The code is changed, quite kludgily, so that almost all the tests are now deliberately correct. One very complicated test is incorrect; for now, I will skip it, and will open an issue when this PR is merged.
Implement SPILL for dynamic arrays. Calculating a dynamic array function will result in a SPILL error if it attempts to overlay a non-null cell which was not part of its previous calculation. Furthermore, it will set to null all cells which were part of its previous calculation but which are not part of the current one (i.e. one or both of the dimensions of the calculation is smaller than it had been); this should also apply for spills (whose result is reduced to 1*1).
Excel will stop you from changing the value in any cell in a dynamic array except the formula cell itself. I have not built this particular aspect into PhpSpreadsheet.
As usual, MS has taken some unusual steps here. If the result of a dynamic array calculation is #SPILL!, it will nevertheless be written to the xml as #VALUE!. It recognizes this situation by adding a new `vm` attribute to the cell, and expanding metadata.xml to recognize this.
A new optional parameter `$reduceArrays` is added to `toArray` and related functions. This will reduce a dynamic array to its first cell, which seems more useful than outputing it as an array (default).
While researching another problem, I noticed that font color was not working as expected for Xls Conditional Formats, at least not when a "non-standard" color is used. In such cases, the color might wind up being rendered as black. The reason is as follows. Xls Writer includes a color palette which is dynamically generated from the (non-Conditional) styles used in the workbook. Any colors used in the workbook are indexes to this dynamic palette. However, Conditional colors use a static palette found in class ColorMap to determine the index, so the determination of index will often not find a match, and, if a match is found, it is not necessarily correct. (Also, the ColorMap method was case-sensitive and needs to be insensitive.)
In order to correct this, the `addColor` method in Xls Writer Workbook needs to be accessible to the Conditional logic which is found in Xls Writer Worksheet. This is accomplished by passing the Workbook in the Worksheet's constructor, and changing the method to public, and changing Conditional Font to use this method rather than ColorMap.
The logic for Conditional Fill colors is similarly changed. Although Xls Conditional Fill has appeared to just not work, I was finally able to figure out the problem. Excel Xls Conditional Fill with fill type Solid requires that the fill color be specified as startColor, and that endColor be omitted. Our conditional samples used endColor, and are now changed to use startColor instead; the same is true for our online documentation, and for some tests. Xlsx continues to work as expected, and now Xls does at least some of the time. If the condition is one that Excel Xls does not recognize (e.g. cell contains), it will, of course, not work. A surprising situation that also doesn't work is the use of ISODD or ISEVEN in formulas. Those are "add-in functions" which are handled differently than other functions, and I'm not sure how to support them. I will document this in issue #3403.
Samples 08_Conditional_Formatting(_2) had produced corrupt Xls versions. This turned out to be because the code was using hash codes to avoid having to write out duplicate conditionals; this is often a good idea, but not in this case. Allowing the duplicates fixes the corruption problem.
Conditional Border colors also ought to figure in this change, but the current code does not support Border colors, and I have not yet been able to figure out how to implement it (BIFF format can be very messy to figure out).
With this change, I could delete ColorMap altogether. However, it is a public class with a static public method, so maybe someone is using it for a purpose I'm not familiar with. I will just deprecate it.
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.
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.
Because they tend to clutter our code a lot and unfortunately, there are
lots of false positives. Instead, it would probably be better to deal
with false positives out of band, via the Scrutinizer web UI.
While we might never be able to have 100% of our code strict, we can at
the very least do it for all of our tests. This ensures that our tests
are using our API with the types as intended by the test author, and not
silently be cast to what our API requires.
And quite a bit more manual changes. The idea is that typing of our
tests can be a bit more loose, so we assume PHPDoc is mostly correct. If
that happens to be wrong, it should be caught by the tests themselves.
Fix#3658. Readers will call `setCalculatedValue` to save the original calculated value for a cell that contains a formula; this affects the result of `getOldCalculatedValue`. The set routine automatically casts numeric values to float, which is not appropriate when the calculated value is explicitly set to a string type. An optional parameter is added to the set routine to indicate whether a cast is appropriate (default is true so userland uses of this method will remain unaffected). The readers for Xlsx, Xls, Xml, Ods, and Slk are all changed to set the new parameter appropriately. Gnumeric saves the formula, but does not appear to save the result, so no change is needed there. Html and Csv save the result but not the formula (for preCalculateFormulas true - they save the formula but not the result for false), so no change is needed for them.
It is not clear what the use case is for oldCalculatedValue, but it does date back to PhpExcel. A possible use case is demonstrated in the new tests - Excel function INFO is not implemented, so getCalculatedValue will generally return null on such a cell, but will return oldCalculatedValue when it sees that the function is not implemented but oldCalculatedValue has been set (typically by loading an existing spreadsheet), so the result that is on the spreadsheet will be available to the program which loaded it.
Reading a spreadsheet which has been created with preCalculateFormulas false will, or at least should, result in a null oldCalculatedValue. This was true for Xls, Xml, and Slk. Xlsx was actually storing 0 in this situation; that wasn't precisely a problem, but it wasn't necessary and seems misleading. Xlsx Writer is changed to no longer set a value in this situation. This is technically a break, but the existing code was wrong; one unit test needed a very minor modification as a result of this change.
* Split Screens
Fix#3601. Split screens are a feature that affects the display of the spreadsheet to the end user; they do not affect the data. They are conceptually similar to "freeze panes". The differences are explained in the issue. As will be explained, support is fairly full for Xlsx, and less full for Xml; no attempt is yet made to support Xls or Ods.
For freeze or split, the window can be divided into 2 horizontal panes, or 2 vertical panes, or 4 horizontal+vertical panes. In Excel, you can split or freeze on cell A1, which causes 4 panes centered at the middle of the screen. PhpSpreadsheet will not duplicate that functionality, and code is added to ignore an attempt to freeze at A1. This breaks one existing nonsensical test, which is changed to something sensible for this PR.
In the spreadsheet xml, both 'freeze' and 'split' use attributes 'xSplit' and 'ySplit' to indicate the position. Unfortunately, the attributes have different meanings for 'freeze' (*columns* from top/left) than for 'split' (*distance* from top/left). For that reason, it is difficult to change between 'freeze' and 'split', so PhpSpreadsheet will not yet support doing so.
There are 3 possible states when freeze/split is used - 'frozen', 'split', or 'frozenSplit'. All will be maintained during read/write, and the user can set them. In Excel, you get 'frozenSplit' by first splitting, then by changing to freezing. In that case, when you unfreeze, you revert to split. PhpSpreadsheet will not duplicate that functionality - when you unfreeze, the panes will go away regardless of whether you were 'frozen' or 'frozenSplit'.
Changing the selected cell will cause the 'active pane' to update when the panes are frozen. PhpSpreadsheet will not yet update the active pane when the panes are split. The programmer will, of course, have the opportunity to explicitly change the active pane in this circumstance.
I have not yet been able to figure out how Xml spreadsheets map their panes, or how to determine selected cell for the sheet as a whole or for individual panes when freeze/split is in effect. The programmer will have the opportunity to specify these explicitly. However, loading an Xml spreadsheet in PhpSpreadsheet will not fill in these values.
Each pane has its own selected cells, and you can navigate between these with the F6 key in Excel (this may work only with split but not with freeze, but Excel maintains the values for freeze and so will PhpSpreadsheet). For Xlsx, PhpSpreadsheet loads and saves these values.
* Eliminate Some Dead Code
Pointed out by Scrutinizer.
Successor to PR #3523. There are 494 single-line changes (`public function provider` to `public static function provider`) in this PR. None of these were made manually; they were all created with the following script (adapted from
https://stackoverflow.com/questions/25909820/how-to-recursively-iterate-through-files-in-php):
```php
$dir = 'C:/git/unit10prep2/tests/PhpSpreadsheetTests';
$it = new RecursiveDirectoryIterator($dir);
// Loop through files
foreach(new RecursiveIteratorIterator($it) as $file) {
if ($file->getExtension() === 'php') {
$contents = file_get_contents($file);
$new = preg_replace('/public function (\\w*)([Pp])rovider/', 'public static function $1$2rovider', $contents);
if ($new !== $contents) {
echo "changing $file\n";
file_put_contents($file, $new);
}
}
}
```
After this PR, there will be one more, with a small number of test changes, and enabling PhpUnit 10 for Php 8.1+.
* Upgrade Dev TCPDF to 6.5
Implementation of https://github.com/tecnickcom/TCPDF/pull/467, which is available in just-released Tcpdf 6.5, will improve look of Tcpdf rendering for PhpSpreadsheet. Fix#1164.
One test had been suppressed for Tcpdf, ostensibly because it was not compatible with Php8. As it turns out, the PhpSpreadsheet code which invokes Tcpdf was (harmlessly) incorrect, so the Php8 issue was actually with PhpSpreadsheet, not Tcpdf. That code is corrected, and the test is no longer suppressed.
* Update Change Log
Pick up some earlier changes as well as this one, and deprecations which had been omitted from the 1.24 change log.
* Fix Some Pdf Problems
Fix#1747. No support for text rotation in Pdf. That issue actually has a decent workaround, but PhpSpreadsheet should handle it on its own. Mpdf requires the proprietary text-rotate css attribute; Html and Dompdf will use the CSS3 attribute transform:rotate.
Fix#1713. Some paper-size values in PhpSpreadsheet are strings, some are 2-element float arrays. Dompdf accepts strings or 4-element float arrays, where the first 2 elements are always 0. Convert the PhpSpreadsheet array accordingly before passing it to Dompdf.
Some tests had been disabled when Dompdf and Tcpdf were slow to achieve PHP8 compliance. They achieved it some time ago. Re-enable the tests.
* Remove Tcpdf From One Test
No problem with the other tests I added it in for.
* Some Fixes for Scatter Charts
Chart issues have been pouring in recently. This is a partial response to issue #2762. It implements "no joins" for scatter charts, as well as having the reader and writer handle "point size", "line width", and "color" for markers. A new boolean property `scatterLines`, with setter and getter, is added to DataSeriesValues to handle joins (default is true which means scatter plot points *are* joined by lines). Some, but not yet all, default font properties for the chart title are handled (color and, surprisingly, font name present challenges).
With these changes, sample 32readwriteScatterChart1.xlsx now looks closer to its source. There are still some differences (x-axis changes), but I think this change is already large enough. I can work on the other problems later.
The code for reading charts has not yet been converted to be namespace aware. Having a tiny island of aware code in a sea of unaware makes no sense to me, so some of the new code is likewise unaware. I hope to be able to get to it eventually, but, among other considerations, it is difficult to generate suitable test cases.
* Add Formal Tests
Essentially the same as the corresponding Samples, but with formal assertions.
* Clean Up Some Code in Reader/Xlsx/Chart
Having added code to support default font attributes as well as element-specific font attributes for chart captions, there was duplicated code between the default and specific sections. I hope that this PR makes the code easier to follow.
* Add Support for Font Name and Color to XLSX Chart Titles
XML layout for these in new files differs from what program was expecting. Not sure if program expectations were wrong, or if this is a change to Excel since initial development.
* Minor Improvement
Handle theoretical case where Chart title has text but no font information.
* Support Bezier Curve and Scaling of X-Axis on Scatter Plot
For Bezier, need to specify `<c:smooth>` tag in addition to already supplied `<c:scatterStyle val="smoothMarker">`
For X-Axis, scatter needs to supply both X and y axis as `<c:valAx>` rather than `<c:catAx>` for X.
These changes have already been implemented twice, and been regressed twice. I'll try once more (with a different approach), then give up ...
As configured, Phpstan running under Php7 reports no errors. However, running under Php8, it reports 100 (!) errors. The vast majority of these are due to two reasons:
- renaming parameters in Php builtin functions in preparation for named parameters.
- using the new class GdImage rather than type resource as the argument type for many image-based functions.
Regardless of the cause, this will be a problem sooner or later. This PR is an attempt to get ahead of that problem. For source members, it mostly adds annotations or updates doc-blocks. Only 2 members have changes to executable code, and these are very minor - BitWise and Writer/Xlsx. For test members, all baseline errors are deleted and the code is fixed. Php7 and Php8 both report no errors with this configuration.
These changes were already implemented as PR #2428. They were, alas, regressed by PR #2585. If at first you don't succeed ...
As configured, Phpstan running under Php7 reports no errors. However, running under Php8, it reports 100 (!) errors. The vast majority of these are due to two reasons:
- renaming parameters in Php builtin functions in preparation for named parameters.
- using the new class GdImage rather than type resource as the argument type for many image-based functions.
Regardless of the cause, this will be a problem sooner or later. This PR is an attempt to get ahead of that problem. For source members, it tweaks only the Phpstan configuration files, without changing any PhpSpreadsheet code. For the small number of test members involved, the code is fixed. Php7 and Php8 both report no errors with this configuration.
Because this involves no changes to code, and because Phpstan baseline is a common cause of merge difficulties, I will probably merge this in a day or two, more quickly than I customarily do.
Replace mock tests with real ones when possible. The original tests are all still present; they just take place in a more representative scenario.
After this, there will be 4 remaining uses of mocking. Of these, 3 are needed for scenarios which are otherwise hard to test - WebServiceTest, CellsTest, and SampleCoverageTest. For the other one, AutoFilterTest, I just can't figure out what it's trying to accomplish, so have left it alone.
This change is almost entirely restricted to tests. There is a one-line change in src. When the first argument passed to OFFSET is null or nullstring, the returned value is currently 0. However, according to the documentation for Excel, it should be `#VALUE!`. The code is changed accordingly.
oleibman
Robin van der Vliet
Adrien Crivelli