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
* 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
Fix#3782. A signature of the ReflectionMethod constructor will be deprecated, and PhpSpreadsheet runs afoul of that change in one place. Php8.4 is not yet available in any form, and I am reluctant to make this change until we see that the issue is real (and that this PR fixes it), so am leaving this PR in draft status till then.
I note that one Excel function `PI` is implemented not as a class method in PhpSpreadsheet, but rather as a call to the native Php function `pi`. The ReflectionMethod call is subject to a TypeError in the changed code, but that is already the case. We haven't seen a TypeError because (a) it will arise only if the caller supplies an argument to the function (which must be called with zero arguments), and (b) there are no test cases for that function. The code is slightly cleaned up, and test cases are now added. This is not an important enough problem to rush this PR - the existing code (and the changed code), rather than failing with TypeError, will fail with a CalculationException (wrong number of arguments) before it gets to the TypeError; that is the correct behavior.
* Performance Improvement for Xlsx Reader
Fix#3683. PR #3497 fixed a problem involving formulas and the quotePrefix style attribute. It did so by automatically turning off quotePrefix for any formulas encountered by Xlsx Reader. Under the right circumstances, it turns out that that change can cause a file read to take noticeably more time than previously. This change will turn off quotePrefix only if it is already on, and that appears to eliminate the performance problem while continuing to solve the original problem.
* Very Minor Improvement
* Update CHANGELOG.md
Fix#3807. Function attempts to rationalize `..` in filenames in a way that normally works just fine. Reporter notes that at least one of the filenames that will be analyzed when a spreadsheet is read can be maliciously altered in a manner which does not harm Excel when reading the file, but which puts PhpSpreadsheet into a loop. This PR fixes the problem.
* Xlsx Support Flipping of Image
Fix#731. Opened over 5 years ago, probably the second oldest problem I've worked on. Images attached to an Xlsx spreadsheet can be rotated, which is supported by PhpSpreadsheet. They can also be flipped along their horizontal and/or vertical axes, and that has not been supported. This PR adds that support.
* Update CHANGELOG.md
* 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".
* Sheet Background Images
Fix#1649, a 3-year-old issue long marked "stale". Excel supports background images on sheets; now PhpSpreadsheet will as well. Support is limited to Xlsx (read and write) and Html (write only). As far as I can tell, Excel Xml and Gnumeric do not support this, nor, of course, do Csv and Slk; Excel Xls does, but, as usual, how to handle it in BIFF format is a mystery; LibreOffice ODS supports it differently than Excel, and this is just another of many ODS style properties not currently supported by PhpSpreadsheet.
* Update CHANGELOG.md
* check if coordinate is inside range
* Added coordinateIsInsideRange tests
* fix coordinateIsInsideRange error throwing
* fix coordinateIsInsideRange error throwing
* add support to worksheet name
* validateReferenceAndGetData type
* change validate and add tests data
* fix tests data reference
* fix absolute reference error
* fix boundaries to get range
* fix scrutinizer erros
* Additional Test Cases
---------
Co-authored-by: oleibman <10341515+oleibman@users.noreply.github.com>
* Two Problems with Html Chart Rendering - Minor Break
Several problems are noted in #3783. This PR addresses those problems which make the rendering unsatisfactory (see following paragraphs). It does not address some items where the rendering is IMO satisfactory although it doesn't match Excel. In particular, the use of a different color palette and the rotation of charts are not addressed. I will leave the issue open for now because of those.
As for the items which are addressed, in some cases the Html was omitting a chart altogether. This is because it had been extending the column range for charts only when it decided that extending the row range was needed. The code is changed to now extend the column range whenever the chart begins beyond the current column range of the sheet.
Also, the rendering always produced a fixed-size image, so saving as Html could result in charts overlaying each other or other parts of the spreadsheet. New properties `renderedWidth` and `renderedHeight` are added to Chart, along with setters and getters. Writer/Html is changed to set these values using the chart's top left and bottom right cells to try to determine the actual size that is needed. Users can also set these properties outside of Writer/Html if they wish. Thanks to @f1mishutka for determining the source of this problem and suggesting an approach to resolving it.
Because the size of the rendered image in Html/Pdf is changed, this could be considered a breaking change. To restore the prior behavior, do the following for all charts before saving as Html:
```php
$chart->setRenderedWidth(640.0);
$chart->setRenderedHeight(480.0);
```
* Update CHANGELOG.md
* Address Some Chart Problems
Fix#3767. The basic problem in that issue was user error, but it exposed a couple of problems in code which are being addressed.
When a spreadsheet with charts is loaded without the includeCharts option and then saved, a corrupt spreadsheet is created. I believe this has been the case for quite some time. Nothing in the test suite covers this scenario. It is, in fact, a difficult thing to test, since the problem is exposed only when the file is opened through Excel. The specific problem is that a rels file generated by the output writer continues to refer to the drawing file which described the chart, but that file is not included (and not needed) in the output spreadsheet. The resolution is kludgey. The information that the file will not be needed is not available when the rels file is written. But, when it comes time to write the drawing file, it is known whether the rels file included it. So, if nothing else has caused the file to be generated, it is written out as a basically empty xml file after all. This solves the problem, but I will continue to search for a less kludgey solution. This solution is, at least, testable; if a different solution is applied later on, the test being introduced here is likely to break so a new one will be needed.
When the provided spreadsheet is loaded with the includeCharts option and then saved, an error is exposed in processing the Chart Title caption when it doesn't exist. The change to Writer/Xlsx/Chart is simple and clearly justifiable. What is peculiar is that the error does not arise with release 1.29, but does arise with master. It is not at all clear to me what has changed since the release to expose the error - the code in question certainly hasn't changed. It is difficult to isolate changes because of the extensive number of changes following on the elimination of Php7.4 as a supported platform.
The provided spreadsheet is unusual in at least two senses. When opened in Excel, it will show a clearly default value for the chart title, namely 'Chart Title'. I cannot find anything in the xml corresponding to that text. Since I have no idea why Excel is using that title, I will not try to duplicate its behavior, so that loading and saving the provided spreadsheet will omit the chart title. I will continue to investigate.
The other sense in which it is unusual is that it includes some style files in the same directory as the chart. I doubt that PhpSpreadsheet looks at these. The styling after load and save seems to mostly match the original, although there is at least one color in the graph which does not match. I imagine it would be pretty complicated to formally support these files.
* Unused Assignment
Fix#3754. Because `PhpSpreadsheetException` extends `Exception`, PhpStorm warns that calls to `setValueExplicit` and `setValue`, among others, do not handle the exception, because PhpStorm treats `Exception`, by default, as "checked". On the other hand, PhpStorm treats `RunTimeException` as "unchecked" so won't flag such calls. It is reasonable to let `PhpSpreadsheetException` extend `RuntTimeException`, and will eliminate the problem, without having to wrap code in all-but-useless try-catch blocks (e.g. for `setValue`, the code would raise an exception only if you try to set the cell's value to an unstringable object).
* Slk Shared Formulas
Fix#2267. The Slk format has a way to express a "shared formula", but the Slk reader does not yet understand it. Thanks to @SheetJSDev for documenting the problem and pointing the way towards a solution. It has taken a long time to get there. Part of the problem is that I have not been successful in getting Excel to use this type of construction when saving a Slk file. So I have resorted to saving a Slk file where shared formulas *could* be used, and then editing it by hand to actually use them. It would not surprise me in the least to have neglected one or more possible ways to specify a shared formula; but, at least the issue as documented is resolved, and if new issues arise, we'll probably be in better shape to deal with them.
* Update CHANGELOG.md
* Theme File Missing But Referenced in Spreadsheet
Fix#3770. A rels file points to a non-existent theme file in the spreadsheet. In other similar cases (e.g. PR #3771), Excel opens such a spreadsheet, but with an error pop-up. Not so with this file; it just opens the spreadsheet without the pop-up. PhpSpreadsheet will now account for this unusual situation as well.
* Update CHANGELOG.md
* Performance Improvements for Csv Reader
Investigating issue #381, a means was suggested to duplicated a problem, but no problem occurred ... except for performance. This involved a spreadsheet with a large number of cells, definitely not PhpSpreadsheet's strong point; even so, the program (entirely available in the issue) took a disastrous two or so hours to complete on my system. Looking at the Csv Reader code, several opportunities to cache results and avoid function calls jumped out, none of which seem to materially add to the maintenance burden of the program. Testing these changes resulted in a run time of about 20 minutes, still hardly a thing of beauty, but a huge improvement over the original and therefore worth proceeding with.
* Redo CsvIssue2232Test
Test cases included duplicates, and didn't account for some things (e.g. French locale will treat both 'vrai' and 'true' as true).
* Additional Optimization
Fix#3760. That problem actually was easily fixed, by enabling calendar extension in user's environment. Astonishingly, however, this is the only use of calendar in the entire project. Since the same functionality is available in DateTime, which is used throughout the project, use that instead to eliminate the dependency on calendar.
* Eliminate Some Phpstan Annotations
Some changes are possible due to Php8+ features like null-safe operators and Stringable.
* Possible Uninitialized Variable
Scrutinizer might be technically correct.
* CellAddress Static -> Self
I don't understand the use case for extending it, which would be the reason for using static rather than self.
* Update Worksheet.php
* Allow Users to Support Additional Tags in Helper/Html
Fix#3751. User wants to add bullets for list items, and possibly handle table rows and other currently unsupported tags where one size might not fit all.
* Update CHANGELOG.md
* Added Conditional Formatting: ColorScale for Xlsx
* Add Reader Support, Tests, Sample
Also correct Phpstan and phpcs problems.
* Update cond08_colorscale.php
* Improve Coverage
* More Coverage Improvements
* Use StyleReader for Colors for ColorScale and DataBar
The implementation of DataBar looks for an rgb attribute, but the color may be provided via theme attribute instead. The initial implementation for ColorScale did the same. Change both to use the existing code in Reader\Xlsx\Styles to parse the color.
* Change Some Doc Blocks to Type Declarations
---------
Co-authored-by: oleibman <10341515+oleibman@users.noreply.github.com>
So that composer will not update our dependencies to something that
would not work with PHP 8.0. And so that PHPStan analyzes assuming we
are running 8.0
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.
Fix#3739. User is seeing an intermittent notice when running samples using a web browser. The notice is comming from ob_clean, complaining that there is no buffer to delete. I frankly do not understand what the ob_clean is supposed to be paired with, nor why I cannot duplicate this result. Possibly, a better solution would be to eliminate the ob_clean; but using ob_get_length beforehand to see if there is anything to clean up seems safer, and I can't think of a downside.
The code in question is never executed when a sample is run from the command line. Consequently, no formal unit test is possible. The user reporting the problem was asked to test the change, and confirmed that the problem went away; no new problem arose on my system.
* fix: Load tables when flag IReader::READ_DATA_ONLY is used
* Add test for table read when IReader:::READ_DATA_ONLY is used
---------
Co-authored-by: Kevin Verschaeve <kevin.verschaeve@exotec.com>
* Break Some More Circular References
After identifying a circular reference between Worksheet and Table, which could lead to memory leaks, I identified several other similar relationships. I have added a destructor, or added added code to an existing destructor, for those possibilities. This didn't actually lead to any lowering of the memory usage of the unit test suite, so maybe there's no practical benefit, but it does seem like it should be theoretically useful. I am not aware of any practical way to unit test a destructor, although the added code will be exercised many times in most of our unit tests.
The only change here not involving a destructor is to `Worksheet::addChart` (it came to my attention because Worksheet and Chart have a circular reference). It is currently defined with two arguments, the second allowing you to place the chart in a specific location in the containing ArrayObject. However, as Phpstan and Scrutinizer both warn, the code to support this option will fail if it is ever executed (can't use array_splice on an ArrayObject); needless to say, this possibility is not exercised when executing the test suite. It could be made to work, but I really can't see any kind of use case to support this parameter. So I'm removing the second parameter. In theory, this is a breaking change, but, since the code can never work, in practice it won't be.
* Update Worksheet.php
* Update Worksheet.php
oleibman
Adrien Crivelli
dependabot[bot]
Pinchon Karim
Adrian
Serhii Petrov
redtailmatt
jenasnic
Progi1984
Kevin Verschaeve
dmytromikhieiev1985