Allow HTML Reader to load from string #1136
Conversation
|
I can add the rest of the checklist items if this is something that you think you would merge |
|
It sounds like it could be useful, but to be merged the checklist will have to be completed and the code should not be duplicated, instead create a common private method used by both |
|
@PowerKiKi Sounds good, I just didn't want to go to a whole lot of trouble if the basic idea wouldn't be accepted. I've rebased and re-worked the PR to include all checklist items. Let me know if there are any issues that need tweaking. |
|
@gnat42 Let me know if you need help with this. I was planning to put something similar in today but looks like you are 90% there. |
|
What's missing? I added tests and docs and added to the changelog? |
src/PhpSpreadsheet/Reader/Html.php
Outdated
| throw new Exception('Failed to load content as a DOM Document'); | ||
| } | ||
|
|
||
| return $this->loadDom($dom, new Spreadsheet()); |
There was a problem hiding this comment.
| return $this->loadDom($dom, new Spreadsheet()); | |
| return $this->loadDocument($dom, new Spreadsheet()); |
|
As far as I can tell, just that (picked up by scrutinizer). |
|
So I'm a bit confused as to how the code coverage decreased. I have a test that tests loadFromString and it passes, but the code coverage makes it out as if loadFromString isn't tested at all. Edited: NVM... I found the issue, my test functions weren't named testCanX... so weren't being run, which is why the tests never found that I called loadDom and loadDocument in two different places... I'm assuming that now this should pass... |
We often want to export a table as an excel sheet. The system renders the html and it seems like a waste of time to write it to the file system to use the reader. This allows us to render the html and then just pass it to a reader
|
You may want to assign @PowerKiKi as both an assignee and a reviewer. I'm not a maintainer and haven't the means to approve or deny this PR. |
|
Hey @codysnider I'm not sure I'm able to assign anyone. Hopefully @PowerKiKi sees and assigns himself. |
|
Thanks ! it has been merged as 95c8bb9 (with minor modifications) |
1.9.0 ### Added - When <br> appears in a table cell, set the cell to wrap [#1071](#1071) and [#1070](#1070) - Add MAXIFS, MINIFS, COUNTIFS and Remove MINIF, MAXIF [#1056](#1056) - HLookup needs an ordered list even if range_lookup is set to false [#1055](#1055) and [#1076](#1076) - Improve performance of IF function calls via ranch pruning to avoid resolution of every branches [#844](#844) - MATCH function supports `*?~` Excel functionality, when match_type=0 [#1116](#1116) - Allow HTML Reader to accept HTML as a string [#1136](#1136) ### Fixed - Fix to AVERAGEIF() function when called with a third argument - Eliminate duplicate fill none style entries [#1066](#1066) - Fix number format masks containing literal (non-decimal point) dots [#1079](#1079) - Fix number format masks containing named colours that were being misinterpreted as date formats; and add support for masks that fully replace the value with a full text string [#1009](#1009) - Stricter-typed comparison testing in COUNTIF() and COUNTIFS() evaluation [#1046](#1046) - COUPNUM should not return zero when settlement is in the last period [#1020](#1020) and [#1021](#1021) - Fix handling of named ranges referencing sheets with spaces or "!" in their title - Cover `getSheetByName()` with tests for name with quote and spaces [#739](#739) - Best effort to support invalid colspan values in HTML reader - [#878](#878) - Fixes incorrect rows deletion [#868](#868) - MATCH function fix (value search by type, stop search when match_type=-1 and unordered element encountered) [#1116](#1116) - Fix `getCalculatedValue()` error with more than two INDIRECT [#1115](#1115) - Writer\Html did not hide columns [#985](#985)
We often want to export a table as an excel sheet. The system renders the html and it seems like a waste of time to write it to the file system to use the reader. This allows us to render the html and then just pass it to a reader
This is:
Checklist:
Why this change is needed?
Just allows Reader\Html to accept html content as a string instead of just a file.