Allow HTML Reader to load from string by gnat42 · Pull Request #1136 · PHPOffice/PhpSpreadsheet

gnat42 · 2019-08-09T19:55:27Z

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:

- [ ] a bugfix
- [x] a new feature

Checklist:

Changes are covered by unit tests
Code style is respected
Commit message explains why the change is made (see https://github.com/erlang/otp/wiki/Writing-good-commit-messages)
CHANGELOG.md contains a short summary of the change
Documentation is updated as necessary

Why this change is needed?

Just allows Reader\Html to accept html content as a string instead of just a file.

gnat42 · 2019-08-09T19:57:02Z

I can add the rest of the checklist items if this is something that you think you would merge

PowerKiKi · 2019-08-12T02:20:34Z

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 load and loadFromString. Most likely that new method would receive a DOMDocument. Also be sure to make use of $this->securityScanner

gnat42 · 2019-08-12T16:24:51Z

@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.

codysnider · 2019-08-13T21:01:13Z

@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.

gnat42 · 2019-08-13T21:51:35Z

What's missing? I added tests and docs and added to the changelog?

codysnider · 2019-08-13T22:03:42Z

src/PhpSpreadsheet/Reader/Html.php

+            throw new Exception('Failed to load content as a DOM Document');
+        }
+
+        return $this->loadDom($dom, new Spreadsheet());


Suggested change

return $this->loadDom($dom, new Spreadsheet());

return $this->loadDocument($dom, new Spreadsheet());

codysnider · 2019-08-13T22:06:11Z

As far as I can tell, just that (picked up by scrutinizer).

gnat42 · 2019-08-14T15:57:16Z

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

codysnider · 2019-08-14T18:36:47Z

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.

gnat42 · 2019-08-14T18:40:54Z

Hey @codysnider I'm not sure I'm able to assign anyone. Hopefully @PowerKiKi sees and assigns himself.

PowerKiKi · 2019-08-17T19:55:31Z

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 Closes PHPOffice#1136

1.9.0 ### Added - When <br> appears in a table cell, set the cell to wrap [PHPOffice#1071](PHPOffice#1071) and [PHPOffice#1070](PHPOffice#1070) - Add MAXIFS, MINIFS, COUNTIFS and Remove MINIF, MAXIF [PHPOffice#1056](PHPOffice#1056) - HLookup needs an ordered list even if range_lookup is set to false [PHPOffice#1055](PHPOffice#1055) and [PHPOffice#1076](PHPOffice#1076) - Improve performance of IF function calls via ranch pruning to avoid resolution of every branches [PHPOffice#844](PHPOffice#844) - MATCH function supports `*?~` Excel functionality, when match_type=0 [PHPOffice#1116](PHPOffice#1116) - Allow HTML Reader to accept HTML as a string [PHPOffice#1136](PHPOffice#1136) ### Fixed - Fix to AVERAGEIF() function when called with a third argument - Eliminate duplicate fill none style entries [PHPOffice#1066](PHPOffice#1066) - Fix number format masks containing literal (non-decimal point) dots [PHPOffice#1079](PHPOffice#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 [PHPOffice#1009](PHPOffice#1009) - Stricter-typed comparison testing in COUNTIF() and COUNTIFS() evaluation [PHPOffice#1046](PHPOffice#1046) - COUPNUM should not return zero when settlement is in the last period [PHPOffice#1020](PHPOffice#1020) and [PHPOffice#1021](PHPOffice#1021) - Fix handling of named ranges referencing sheets with spaces or "!" in their title - Cover `getSheetByName()` with tests for name with quote and spaces [PHPOffice#739](PHPOffice#739) - Best effort to support invalid colspan values in HTML reader - [PHPOffice#878](PHPOffice#878) - Fixes incorrect rows deletion [PHPOffice#868](PHPOffice#868) - MATCH function fix (value search by type, stop search when match_type=-1 and unordered element encountered) [PHPOffice#1116](PHPOffice#1116) - Fix `getCalculatedValue()` error with more than two INDIRECT [PHPOffice#1115](PHPOffice#1115) - Writer\Html did not hide columns [PHPOffice#985](PHPOffice#985)

codysnider reviewed Aug 13, 2019

View reviewed changes

Allow HTML Reader to load from string

6787de1

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

Minor doc fixes

717d9ca

PowerKiKi closed this in 95c8bb9 Aug 17, 2019

gnat42 deleted the patch-1 branch August 23, 2019 21:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow HTML Reader to load from string#1136

Allow HTML Reader to load from string#1136
gnat42 wants to merge 2 commits intoPHPOffice:masterfrom
gnat42:patch-1

gnat42 commented Aug 9, 2019 •

edited

Loading

Uh oh!

gnat42 commented Aug 9, 2019

Uh oh!

PowerKiKi commented Aug 12, 2019

Uh oh!

gnat42 commented Aug 12, 2019

Uh oh!

codysnider commented Aug 13, 2019

Uh oh!

gnat42 commented Aug 13, 2019

Uh oh!

codysnider Aug 13, 2019

Uh oh!

codysnider commented Aug 13, 2019

Uh oh!

gnat42 commented Aug 14, 2019 •

edited

Loading

Uh oh!

codysnider commented Aug 14, 2019

Uh oh!

gnat42 commented Aug 14, 2019

Uh oh!

PowerKiKi commented Aug 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

	return $this->loadDom($dom, new Spreadsheet());
	return $this->loadDocument($dom, new Spreadsheet());

Conversation

gnat42 commented Aug 9, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why this change is needed?

Uh oh!

gnat42 commented Aug 9, 2019

Uh oh!

PowerKiKi commented Aug 12, 2019

Uh oh!

gnat42 commented Aug 12, 2019

Uh oh!

codysnider commented Aug 13, 2019

Uh oh!

gnat42 commented Aug 13, 2019

Uh oh!

codysnider Aug 13, 2019

Choose a reason for hiding this comment

Uh oh!

codysnider commented Aug 13, 2019

Uh oh!

gnat42 commented Aug 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codysnider commented Aug 14, 2019

Uh oh!

gnat42 commented Aug 14, 2019

Uh oh!

PowerKiKi commented Aug 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

3 participants

gnat42 commented Aug 9, 2019 •

edited

Loading

gnat42 commented Aug 14, 2019 •

edited

Loading