5 Commits

Author SHA1 Message Date
oleibman 9df8ceb5c0 Minor Improvements to Reader/Writer Coverage 2025-09-14 00:18:08 -07:00
Adrien Crivelli 627f76a803 Merge branch 'master' into powerkiki 2023-09-12 11:07:32 +08:00
oleibman 0d1c9e4e0e ListWorksheetInfo/Names for Html/Csv/Slk (#3709)
* 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.
2023-09-07 18:29:45 -07:00
Adrien Crivelli ec4098c8fd Strict mode for all tests
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.
2023-09-07 17:44:56 +08:00
oleibman 5de82981d8 Html Reader Not Handling non-ASCII Data Correctly (#2943)
* 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.
2022-07-16 22:08:44 -07:00