* ListWorksheetInfo/Names for Html/Csv/Slk
Fix#3706. ListWorksheetInfo is implemented for all Readers except Html. For most (not all), ListWorksheetInfo is more efficient than reading the spreadsheet. I can't think of a way to make that so for Html, but that shouldn't be a reason to leave it unimplemented.
ListWorksheetNames is not implemented for Html, Csv, or Slk. It isn't terribly useful for those formats, but that isn't a reason to omit it. The requester's use case consists of using IOFactory to create a reader for a file of unknown format and determining the first sheet name. That seems legitimate, but it is currently not possible without extra user code if the file is Html, Csv, or Slk; this PR will make it possible.
When Excel opens a Slk or Csv file, the sheet name is based on the file name. PhpSpreadsheet does this for Slk, but it uses a default name for Csv. I am not interested in creating a break for that behavior, but I have added a new boolean property `sheetNameIsFileName` with a setter to Csv Reader. The requester actually mentioned that possibility in our discussion, although it is not essential to the request.
As an adjunct to the issue, the requester wishes to use the worksheet name in `setLoadSheetsOnly`. That is already possible for Html, Csv, and Slk, but that particular property is ignored for those formats. I do not see a reason to change that behavior. This treatment is now explicitly noted in the documentation for property `loadSheetsOnly`.
There had been no tests for what happens when `loadSheetsOnly` is specified but no sheets match the criteria for the formats for which this makes sense (Xlsx, Xls, Ods, Gnumeric, Xml). The behavior was not consistent - some formats threw an Exception while others continued with a single empty worksheet. All cases attempt to set the active sheet, and they will now all throw identical Exceptions when they attempt to do so in this situation. Tests are added for each.
There also had been no tests for `loadSheetsOnly` returning more than one sheet. One is added.
* Update LoadSheetsOnlyTest.php
Add strict types to this new test, consistent with work being done in PR #3718.
* Update LoadSheetsOnlyTest.php
Add strict types to this new test, consistent with work being done in PR #3718.
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#3565. Requester suggested adding viewport as a property of Html Writer. I think it would fit better as a Document Property, so that it can be both read and written, just like other meta properties, making it easier to load an Html spreadsheet and save it as a new one preserving this property. It is an open question whether we should try to preserve this property when writing to other spreadsheet types, e.g. as a custom property for Xlsx. For now, I am not attempting that. Requester also suggested this as a boolean property, but there are other parameters and different values that might be used for viewport. So I am defining it as a string property, with a constant SUGGESTED_VIEWPORT that can be used for requester's use case.
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+.
* PhpUnit 10 Compatibility Part 1
This is not a change to move to PhpUnit 10. There is no compelling reason to do so at this time, although it is bound to happen eventually. There are a staggering number of problems (somewhere around 3,000) with the current test suite under PhpUnit 10; this is an attempt to get ahead of the curve by addressing them now.
Method `setOutputCallback` has gone away. This affects only Helper/SampleTest. It appears that `ob_start` and its allies provide an effective equivalent. FWIW, the absence of `setOutputCallback` is a good indication of whether or not PhpUnit 10 is in use, and I will use that fact in a few tests.
Class `ComplexAssert` with no constructor, and always used with `new ComplexAssert()`, extends `TestCase`. Apparently, the constructor for TestCase requires an argument, and PhpUnit 10 complains about not supplying one. Adding an empty constructor to ComplexAssert avoids this problem.
There are two very minor source changes, to Calculation/Calculation and Reader/Xlsx, where problems were exposed with PhpUnit 10 that had not been previously been exposed. AFAIK, these are the only source changes required; the rest of the changes are to test members.
The bulk of the problems are because PhpUnit 10 insists that provider methods be static. Most of those can be changed by a script without any further action; those changes will constitute the Part 2 counterpart of this PR. In this PR you will find the exceptional cases that can't be automated for one reason or another. The tests for Database functions have mild complications that are easily handled. Most of the other provider changes in this PR are because the method names didn't follow an established pattern ('provider' isn't part of the method name); those are also easily handled manually. Modifying the following tests provided significant challenges:
- Writer/Xls/WorkbookTest testAddColor
- Worksheet/Table/TableTest testSetRangeValidRange
The handling of warning messages issued by the code differs in PhpUnit 10. According to the change log, "This means that using PHP functionality which triggers E_DEPRECATED, E_NOTICE, E_STRICT, or E_WARNING or calling code which triggers E_USER_DEPRECATED, E_USER_NOTICE, or E_USER_WARNING can no longer hide a bug in your code." To me, the effect of that change seems to be exactly the opposite - such messages were available to the test with PhpUnit 9 (so we could test for them), and are no longer available (so we can't). I haven't even succeeded with a custom error message handler as part of the script. I will continue to investigate, but, for now, will skip some tests under PhpUnit 10 for the following:
- Shared/OleTest testChainedWriteMode and testChainedBadPath
- Reader/Html/HtmlLoadStringTest testLoadInvalidString
- Reader/Html/HtmlTest testBadHtml
* Scrutinize, and Parent Construct
Parent construct suggested by @MarkBaker.
* Redo Tests Dependent on Warning Messages
Warning (and other) messages are handled differently in PhpUnit 10 than in earlier versions.
* Specify data type in html tags using attributes #3444
* Set data types using attributes in flushCell function and add some unit tests
* Check TYPE_INLINE in flushCell and some changes in test cases
No source changes. Act on some items that have come up in recent discussions.
- Sample 33_Chart_create_line creates a stacked line chart. According to @MarkBaker, the stacking is done on the wrong variable, and, even were that not the case, stacking is unusual for line charts. Since this is our primary sample showing how to create a line chart, remove the stacking. Another sample, with a more appropriate choice of chart (33_Chart_create_bar_stacked), still shows how to create a stacked chart.
- The test for reading a styled cell from Html is flawed. It sets a date format for a string date/time, but the format is applied only to numeric data, so the format, although set correctly, is ineffective. Keep the test, but add some explanation in the assertion, and add some new more effective tests, also with explanations in the assertions.
- Wrong namespace used for Writer/Xlsx/ConditionalFillTest.
* Phpstan Baseline Fixes 2022-09-21
Eliminate about 200 more lines from Phpstan baseline. For Helper/Sample and Helper/Html and others, many properties are declared as protected despite the fact that the classes do not extend any other class, and there are no classes which extend them. They are changed to private; this could be a breaking change in circumstances for which I cannot think of a use case (user extends class for some reason).
* Slightly Botched Merge Commit
Hope this fixes phpstan.
* Html Reader Not Handling non-ASCII Data Correctly
Fix#2942. Code was changed by #2894 because PHP8.2 will deprecate how it was being done. See linked issue for more details. Dom loadhtml assumes ISO-8859-1 in the absence of a charset attribute or equivalent, and there is no way to override that assumption. Sigh. The suggested replacements are unsuitable in one way or another. I think this will work with minimal disruption (replace ampersand, less than, and greater than with entities representing illegal characters, then use htmlentities, then restore ampersand, less than, and greater than).
* Better Implementation
Use regexp to escape non-ASCII. Less kludgey, less reliant on the vagaries of the PHP maintainers.
* Additional Tests
Test non-ASCII outside of cell contents: sheet title, image alt attribute.
* Apply Same Change in Second Location
Forgot to change loadFromString.
* Additional Test
Confirm escaped ampersand is handled correctly.
Mostly new tests, some code annotations, some minor code changes:
- RichText clone logic is wrong
- TextElement doesn't have object properties, doesn't need clone
Fix#2810. Repairing some Phpstan diagnostics, used `?:` rather than `??` in a few places.
2 different Html modules are affected. Also, Ods Reader, but its problem is with sheet title rather than cell contents. And, as it turns out, Ods Reader was already not handling sheets with a title of `0` correctly - it made a truthy test before setting sheet title. That is now changed to truthy or numeric. Other readers are not susceptible to this problem. Tests are added.
Allows basic column width conversion when importing from Html that includes UoM... while not overly-sophisticated in converting units to MS Excel's column width units, it should allow import without errors
Also provides a general conversion helper class, and allows column width getters/setters to specify a UoM for easier usage
* Fix for Issue 2029 (Invalid Cell Coordinate A-1)
Fix for #2021. When Html Reader encounters an embedded table, it tries to shift it up a row. It obviously should not attempt to shift it above row 1. @danmodini reported the problem, and suggests the correct solution. This PR implements that and adds a test case.
Performing some additional testing, I found that Html Reader cannot handle inline column width or row height set in points rather than pixels (and HTML writer with useInlineCss generates these values in points). It also doesn't handle border style when the border width (which it ignores) is omitted. Fixed and added tests.
This issue arose while researching issue #1823. The issue was not a bug;
it just required clarification to the author of how to use the software.
But, while researching, I discovered that loading html into 2
sheets of a spreadsheet has a problem if the html title tag is the same
for the 2 sheets. PhpSpreadsheet would be able to save the resulting file,
but Excel would not be able to read it properly because of the duplicate title.
The worksheet setTitle method allows for disambiguation is such a circumstance.
The html reader passed a parameter indicating "don't disambiguate", but I can't
see any harm in changing that to "disambiguate". An extremely simple fix,
with tests to back it up.
Reader/Html is now covered except for 1 statement.
There is some coverage of RichText when you know in advance that the
html will expand into a single cell.
It is a tougher nut, one that I have not yet cracked,
to try to handle rich text while converting unkown html to multiple cells.
The original author left this as a TODO, and so for now must I.
It made sense to restructure some of the code. There are some changes.
- Issue #1532 is fixed (links are now saved when using rowspan).
- Colors can now be specified as html color name. To accomplish this,
Helper/Html function colourNameLookup was changed from protected
to public, and changed to static.
- Superfluous empty lines were eliminated in a number of places, e.g.
<ul><li>A</li><li>B</li><li>C</li></ul>
had formerly caused a wrapped cell to be created with 2 empty lines
followed by A, B, and C on separate lines; it will now just have the
3 A/B/C lines, which seems like a more sensible interpretation.
- Img alt tag, which had been cast to float, is now used as a string.
Private member "encoding" is not used. Functions getEncoding and setEncoding
have therefore been marked deprecated. In fact, I was unable to get
SecurityScanner to pass *any* html which is not UTF-8. There are
possibly ways of getting around this (in Reader/Html - I have no
intention of messing with Security Scanner), as can be seen in my
companion pull request for Excel2003 Xml Reader. Doing this would be
easier for ASCII-compatible character sets (like ISO-8859-1),
than for non-compatible charsets (like UTF-16). I am not
convinced that the effort is worth it, but am willing to investigate
further.
I added a number of tests, creating an Html directory, and moving
HtmlTest to that directory.
oleibman
Adrien Crivelli
Pouria Seyfi
Mark Baker
oleibman