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#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.
* 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
* Chart Dynamic Title and Special Font Properties
Fix#3797. Excel allows a Chart Title to be a formula, albeit a very rigidly limited one. It can only be a reference to a single cell, and the worksheet name must be specified, and the column and row must be absolute. Methods are added to Chart/Title to accommodate this (and styling for it). This will be handled for input/output for Xlsx, and for output for Html.
The sample file which was submitted with this issue demonstrated that something else was missing. When setting the font for a chart title in Excel, you can specify all-caps or small-caps, options not available for most cell formatting. These are now added.
The sample file also fell into the category of spreadsheets which lose one or more charts when converted to Html. I have redone the "extend rows and charts" logic in Html Writer. It is now clearer (I hope) and more efficient, and hopefully this problem will not arise again.
* Scrutinizer 50/50
One false positive, one correct "unused parameter".
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.
* Cleanup for Phpstan, PHP-CS-Fixer Upgrades
Dependabot wanted to upgrade, but changes resulted in new "errors". Fix them.
* Scrutinizer
It says value must be numeric; Phpstan says it might not be. Try to reconcile them.
* PhpUnit 10 Compatibility Part 3 (Last)
Final changes for PhpUnit 10, including enabling it for testing. This finishes the work of PR #3523 and PR #3526.
The major remaining problem with PhpUnit 10 is that earlier releases converted notices and warnings to exceptions, and 10 does not. Not having the information provided by the messages seems risky to me. Fortunately, it appears that you can add an error handler to the test bootstrap for 10 and make it act like earlier versions; I have done so. In order to demonstrate the effectiveness of this handler, a new otherwise unused class Helper/Handler and tests for that class are added.
As part of the testing of this change, it became apparent that the fopen in OLE::getStream attempts to create a dynamic property $context in Shared/OLE/ChainedBlockStream, and that action is deprecated in Php8.2. Adding the property to the class eliminates that problem. No executable code is added, and this is the only change to source code.
There also seems to have been a change in assertXmlStringEqualsXmlString in PhpUnit 10. The only test which uses that method is Chart/Issue589Test, and both the places which use that method could just as easily and effectively use assertSame. They are changed to do so.
* Remove Phpunit Verbose Option
Not supported in PhpUnit 10. I'm not at all sure that this is the correct solution for this problem.
* Try Changing Phpunit Command for Different Php Releases
Not sure how to test this locally. We'll see if it works on github.
* Another main.yml attempt
We shall see.
* Eliminate One Test
Not sure why testDeprecated is not working in Github; it works locally. Disable it for now and continue to research.
* Show Incomplete and Other Messages in PhpUnit 10
6 new command line args to replace the 1 they got rid of.
* Restore Disabled Test
Deprecated messages are suppressed by default setting for error_reporting. Switch that to E_ALL in bootstrap and restore original test.
* Add Deprecation Tests for PhpUnit 9-
Default configuration option caused deprecation messages to be suppressed. Change the option.
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+.
* Font and Effects Support for Chart Data Labels and Axis
Addresses some remaining issues with 32readwriteLineChart5 (see issue #1797). Font size is covered. So are effects, although the results are a bit odd. For the new spreadsheet 32readwriteLineChart6, the Axis labels have a yellow-ish glow, but reading and writing the spreadsheet in PhpSpreadsheet gives them a purple-ish glow. Nevertheless, the new test shows that the output file uses schemeClr accent4, as does the input file. So the effect is handled correctly, but it seems there is likely to be a difference between theme colors (Writer/Xlsx/Theme appears to write hard-coded color schemes, and, in any case, Reader/Xlsx does not appear to handle schemeClr). Fixing that will be a great deal more difficult, with a large chance of regression, and will need to happen in a separate PR (one that I am not currently investigating, but I will open a new issue). Effects using srgbClr (and probably sysclr) should be okay.
* Better Theme Support
When reading Xlsx, the theme colors will now also be used for writing. This means that a file can be loaded and saved and its chart colors will now be preserved. If the spreadsheet is created new, Excel 2007-2010 colors are used. The writer is currently hard-coded to use them, so this avoids making this a breaking change. The theme colors can be explicitly changed if desired, and Excel 2013+ colors can be introduced very easily.
```php
$spreadsheet->getTheme()
->setThemeColorName(Theme::COLOR_SCHEME_2013_PLUS_NAME);
```
Likewise, if the old behavior of changing to the 2007-2010 scheme rather than using the input values is desired, that is easy to achieve after the load has taken place.
```php
$spreadsheet->getTheme()
->setThemeColorName(Theme::COLOR_SCHEME_2007_2010_NAME);
```
The new Theme class introduced by this change can easily be extended to include Fonts and Effects. Unlike Colors, I am unsure what the practical effects of changing those to, say, the 2013+ defaults would be.
* Scrutinizer
Use an alias in a use statement.
* Update Change Log
Due to potential behavior change.
* Support Border for Charts
All chart linestyles will be supported for Chart Border. Also add fill color for Chart. Also 'nofill' for Axis (allows suppressing of vertical axis line). These are demonstrated in sample 32_readwriteChartLine5, and a new unit test member is added. This addresses some vague problems added to issue #1797 over 8 months after it was closed; there is still at least one problem, much more complicated than the 2 being addressed in this PR.
* Handle Legend Borders in Same Way as Chart Borders
Consistency is good, and the implementation of Chart Borders offers more possibilities than the implementation of Legend Borders had. Since Legend Borders hasn't made it to a release yet, there is no need for deprecations.
* Allow Color and Effects on Data Points on Chart Axis
Fix#3414. There had been no way to do this. It is now supported via a new AxisText class, which derives from Properties, and has FillColor and effects (glow/shadow/softEdges) properties. The code changes in chart sample 33_Chart_create_scatter2 illustrate usage.
* Minor Performance Improvement
May also get rid of Scrutinizer complaint.
* Eliminate New Spacing Patch
No need for it.
* Add Legend Formatting to Change
Much in common with Data Point formatting.
* Misplaced Xml Writing Chart Label FillColor
Fix#3397. Move code to correct location.
* Update Issue3397Test.php
No need for spacing patch - just fix it now.
* Minor Fix
See discussion at bottom of PR #2856.
* Update PR template
* Trim trailing space
* Support for configuring a Chart Title's overlay
* Provide test coverage for getting/setting a Chart Legend's overlay
Add missing docblock
* Fix Unintential Deprecated Calls - Everything Else
I think it's best to install these before PR #3166. This one, which I hope to be the last in this series, does have some minor changes to source code, as well as to doc-blocks and to test members which continue to inadvertently use calls to deprecated functions.
* Some Remaining Deprecations in Tests
Fix them now.
* Minor Docblock Updates
Worksheet::unprotectCellsByColumnAndRow was incorrect. Other changes are cosmetic, leading to slightly better documentation.
* Update Worksheet.php
Fix#3011. Some properties for Trendlines were omitted in the original request for this feature. Also, the trendlines sample spreadsheet included two charts. The rendering script 35_Chart_render handles this, but overlays the first output file with the second. It is changed to produce files with different names.
* Charts - Add Support for Date Axis
Fix#2967. Fix#2969 (which had already been fixed prior to opening the issue, but had added urgency for Date Axes). Add ability to set axis type to date axis, in addition to original possiblities of value axis and category axis.
* Update 33_Chart_create_line_dateaxis.php
No idea why php-cs-fixer is complaining. It didn't do so when I first uploaded. I can't duplicate problem on my own system. Not enough detail in error message for me to act. Grasping at straws, I have moved the function definition (which is the only use of braces in the entire script) from the end of the script to the beginning.
* Update 33_Chart_create_line_dateaxis.php
Some comments were mis-aligned. This may be related to the reasons behind PR #3025, which didn't take care of this because this script had not yet been merged.
The setup for unit testing in Github in the "Install dependencies" log reports 11 members as "does not comply with with psr-4 autoloading standard." In each case, it is because the test namespace does not match the directory; in most cases, it was caused by the member being moved from one directory to another without changing the namespace declaration. No harm results from these problems, but there's also no reason to not correct them.
* Replace Dev jpgraph/jpgraph with mitoteam/jpgraph
PR #2979 added support for mitoteam/jpgraph as an alternative to jpgraph/jpgraph. The package jpgraph/jpgraph is abandoned in composer, and the version loaded with composer has been unusable for some time. This PR removes the dev requirement for jpgraph/jpgraph, and adds a dev requirement for mitoteam/jpgraph in its place.
With a usable graph library, a number of tests and samples that had been disabled are now re-enabled. A lot of new functionality has been added to Charts recently. Some of that new code has exposed bugs in JpgraphRendererBase. I have fixed those where I could. A handful of exceptions remain; I will investigate, and hopefully fix, those over time, but I don't feel it is necessary to fix them all before installing this PR - we are already way ahead of the game with the graphs that are working.
Three members had been ignoring code coverage in whole or in part because of the unavailability of a usable graph libray. Code coverage is restored in them. I am relieved to report that, although they aren't completely covered, adding them did not reduce code coverage by much - it is still over 90.4%.
I took a look at JpgraphRendererBase and Phpstan. Phpstan reports 128 problems. When I added some docblocks to correct some of those, the number increased to 284. Sigh. I will investigate over time, but, for now, we will still suppress Phpstan for JpgraphRendererBase.
I do not find a License file for mitoteam. However, there also wasn't one for jpgraph in the first place. Based on that and the discussion in #2996 (mitoteam will be used in exactly the same manner as mpdf), I don't think this is a problem. IANAL.
* PHP 8.2 Problems
Tons of "cannot create dynamic property" deprecations in jpgraph. Disable the test with most of those for now; leave the two with only a handful of messages enabled.
* Correct Failures in 2 Stock Charts
Down to 6 templates on which Render fails.
Addresses a problem identified in issue #2991. Chart name is set in constructor, but there is no method to subsequently change it. This PR adds a method to do so.
This is a start in addressing issue #2965 (and earlier issue #749). Chart Titles are usually entered as strings or Rich Text strings, and PhpSpreadsheet supports that. They can also be entered as formulas (typically a pointer to a cell with the title text), and, not only did PhpSpreadsheet not support that, it threw an exception when reading a spreadsheet that did so.
This change does:
- eliminate the exception
- set a static chart title when it can determine it from the Xml
This change does not:
- fully support dynamic titles (e.g. if you change the contents of the source cell, or delete or insert cells or rows or columns)
- permit the user to set the title to a formula
- allow the use of formulas when writing a chart title to a spreadsheet
- provide styling for titles when it has read them as a formula
Fix#2257. Fix#2929. Fix#2935 (probably in a way that will not satisfy the requester). 2257 and 2929 requested changes that ultimately affect the same section of code, so it's appropriate to deal with them together. 2257 requests the ability to make the chart background transparent (so that the Excel gridlines are visible beneath the chart), and the ability to hide an Axis. 2929 requests the ability to set a gradient background on the chart.
Fix#2705. Add to Axis class, Reader Xlsx Chart, and Writer Xlsx Chart. Add feature to an existing 32* sample, to an existing 33* sample, and a formal unit test.
Fix#2931. If surface charts are written with c:grouping tag, Excel will treat them as corrupt. Also, some 3D-ish Xml tags are required when 2D surface charts are written, else they will be treated as 3D. Also eliminate a duplicate line identified by #2932.
This was supposed to be mopping up some longstanding chart issues. But one of the sample files exposed a memory leak in Xlsx Writer, unrelated to charts. Since that is my best sample file for this problem, I would like to fix both problems at the same time.
Xlsx Writer for Worksheets calls getRowDimension for all rows on the sheet. As it happens, the sample file had data in the last rows after a huge gap of rows without any data. It correctly did not write anything for the unused rows. However, the call to getRowDimension actually creates a new RowDimension object if it doesn't already exist, and so it wound up creating over a million totally unneeded objects. This caused it to run out of memory when I tried to make a copy of the 8K input file. The logic is changed to call getRowDimension if and only if (there is data in the row or the RowDimension object already exists). It still has to loop through a million rows, but it no longer allocates the unneeded storage.
As for the Chart problems - fix#1797. This is where the file that caused the memory leak originated. Many of its problems were already resolved by the earlier large set of changes to Charts. However, there were a few new properties that needed to be added to Layout to make things complete - numberFormat code and source-linked, and dLblPos (position for labels); and autoTitleDeleted needs to be added to Charts.
Also fix#2077, by allowing the format to be specified in the Layout rather than the DataSeriesValues constructor.
* Move Gridlines from Chart to Axis
This could, I hope, be my last major change to Chart for a while. When I first noticed this problem, I thought it would be a breaking change. However, although this change establishes some deprecations, I don't think it breaks anything. Major and minor gridlines had only been settable by the Chart constructor. This PR moves them where they belong, to Axis (eexisting Chart constructor code will still work). This allows them to be specified from both X and Y axis.
Chart is now entirely covered except for 2 statements, one deprecated and one that I just can't figure out. 99.71% for Charts, 88.96% overall. All references to the Chart directory in Phpstan baseline are eliminated.
* Minor Fixes, Unit Tests
Line style color type should default to null not prstClr.
Chart X-axis and Y-axis should alway be Axis, never null.
Add some unit tests.
* More Tests, Some Improvements
Make it easier to change line styles, adding an alternate method besides a setter function with at least a dozen parameters.
* Charts Additional Support for Layout and DataSeriesValues
The dLbls tag in more or less the Xml equivalent of the Layout class. It is currently read and written only for the Chart as a whole. It can, however, also be applied to DataSeriesValues. Further it has properties which are currently ignored, namely label fill, border, and font colors. All of these omissions are handled by this PR. There are other properties which can be applied to the labels, but, for now, only the 3 colors are added.
DataSeriesValues can have effects (like glow). Since DSV now descends from Properties, these are already supported, but support needs to be added to the Reader and Writer to handle them. This PR adds the support.
* Add Unit Tests
Based on new samples.
* Minor Improvements
Slight increase to coverage.
* Additional Support for Chart DataSeriesValues
Fix#2863. DataSeriesValues now extends Properties, allowing it to share code in common with Axis and Gridlines. This causes some minor breakages; in particular line width is now initialized to null instead of Excel's default value, and is specified in points, as the user would expect from Excel, rather than the value stored in Xml.
This change:
- adds support for 1 or 2 marker colors.
- adds support for `smoothLine` to DataSeriesValues.
- will determine `catAx` or `valAx` for Axis based on what is read from the Xml when available, rather than guessing based on format. (Another minor break.)
- reads `formatCode` and `sourceLinked` for Axis.
- correct 2 uses of `$plotSeriesRef` to `$plotSeriesIndex` in Writer/Xlsx/Chart.
- pushes coverage over 90% for Chart (88.70% overall).
* Update Change Log
I had updated previously but forgot to stage the member.
* Adopt Some Suggestions
Incorporate some changes suggested by @bridgeplayr.
* Use ChartColor for DSV Fill And Font Text
DataSeriesValues Fill could be a scalar or an array, so I saved it till last.
* Some Final Cleanup
No code changes.
Illustrate even more of the new features in existing sample files.
Deprecate *_ARGB in Properties/ChartColors in favor of *_RGB, because it uses only 6 hex digits. The alpha value is stored separately.
Unit testing now results in 100% coverage for Axis and Properties. All the properties in methods in Gridlines were more or less duplicated in Axis, and these duplications are moved to the common ancestor Properties. So, there isn't anything left in Gridlines. PhpSpreadsheet Chart is now over 85% covered (it was below 35% until recently).
Properties are in many cases set to default to null/null-string, rather than the default values they receive from Excel, and are not written to Xml if unchanged. This is consistent with how Excel behaves. A new property `crossBetween` is added to Axis, and, with support for that added to Xlsx Reader and Writer, some minor Sample peculiarities are corrected, in particular, the charts were sometimes slightly truncated on the left and right edges.
* Expand Chart Support for schemeClr and prstClr
Fix#2219. Address some, but not all, issues mentioned in #2863.
For Pie Charts, fill color is stored in XML as part of dPt node, which had been ignored by Reader/Xlsx/Chart. Add support for it, including when specified as schemeClr or prstClr rather than srgbClr. Add support for prstClr in other cases where schemeClr is supported.
* Update Change Log
Add this PR.
* Add Support to Chart/Axis and Gridlines for Shadow
Continuing the work from #2865. Support is added for Shadow properties for Axis and Gridlines, and Glow and SoftEdges are extended to Gridlines. Tests are added. Some chart tests are moved from Reader/Xlsx and Writer/Xlsx so that most chart tests are under a single directory.
This is a minor breaking change. Since the support for these properties was just added, it can't really affect much in userland. Some properties had been stored in the form which the XML requires them rather than as the user would enter them to Excel. So, for example, setting the Glow size to 10 points would have caused it to be stored internally as 127,000. This change will store the size internally as 10, obviously making the appropriate conversion when reading from or writing to XML. This makes unit tests much simpler, and I think this is also what a user would expect, especially considering the difficulties in keeping track of the trailing zeros.
* More Tests
Confirm value change between internal and xml.
* Still More Tests
Add a little more coverage, and use a neat trick suggested by @MarkBaker in the discussion of PR #2724 to greatly simplify MultiplierTest.
* Add Support to Chart/Axis for Glow/SoftEdges
Chart is very under-covered in unit tests. I was hoping to just add some tests and be done with it, but that just won't suffice - many code changes are required. Adding Glow properties for Axis turned out to be a real mixed bag - no support in Xlsx/Reader at all, support in Xlsx/Writer ... for the X-axis only. So we'll just inch forward in many stages.
This change does not address other Axis properties (Fill, Shadow, Line, LineStyle, and maybe others, and their sub-properties). On my to-do list.
GridLines, and maybe other Chart objects, also should support these properties. This change does not address those. On my to-do list.
No support is added for spreadsheet formats other than Xlsx.
* Refactoring
This should make the code easier to follow, and I hope it will also be extensible to other object types (e.g. GridLines).
* More Refactoring
Make scheme/srgb easier to handle.
* Fix Typo
In a very rarely used function.
This is a leftover Scrutinizer change, but it needed more attention than most others. Chart/Title DocBlocks define caption as `null|string`. However, in the wild, Excel usually presents the caption as an array, and not an array of strings but rather of RichText items. I am not sure why an array is needed since a RichText item can contain many text runs, but things are what they are.
Reader/Xlsx/ChartTitleTest reads a spreadsheet with the captions stored as a RichText array. Since it performs array operations on something the DocBlock says cannot be an array, Scrutinizer objects, although not seriously enough to fail the module. Phpstan also objects; its objection is silenced with an annotation. Aside from this test, there are other tests which do set the caption to a string, and Excel seems to handle that without a problem. So, I have changed the DocBlock to specify `array|RichText|String`. I have dropped null as a possibility; nullstring will do equally well.
Because getCaption can now return multiple datatypes, I think a new function which can return the text portion of the entire caption as a single string is needed. I have added it. This simplifies the test named above, and some code in Writer/Html. The latter is not part of unit testing because the version of JpGraph found in Composer is too antiquated. I verified the Html change manually by running samples/Chart/32_Chart_read_write_HTML.php using a recent version of JpGraph. It was as a result of this test that I uncovered issue #2203. I did not see anything about Charts in docs, so did not add a description of the new function there.
Phpstan is happy with the changes. We'll see how Scrutinizer feels when I push it.
oleibman
Adrien Crivelli
Jonathan Goode
fox34
Owen Leibman