Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Spout implementation #1

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

aaa2000
Copy link

@aaa2000 aaa2000 commented Mar 5, 2018

I tried to implement the spout reader and writer.

Spout read and write spreadsheet files in CSV, XLSX and ODS. But, the reader is only compatible with XLSX.

I based myself on the code of portphp/excel, the fixtures are XLS format but spout doesn't manage this format. I converted the XLS file in XLSX format with libreoffice

The writer can't edit an existing spreadsheet, the data will be overwritten.

http://opensource.box.com/spout/guides/add-data-to-existing-spreadsheet/
http://opensource.box.com/spout/guides/edit-existing-spreadsheet/

Close portphp/portphp#61

  • Make the reader compatible with CSV and ODS
  • Allow to edit an existing spreadsheet with the writer

Copy link

@Baachi Baachi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @aaa2000

thanks for your contribution! Really appreciated.
Looking forward to merging this PR.

*/
public function __construct(\SplFileObject $file, $headerRowNumber = null, $activeSheet = null, $shouldPreserveEmptyRows = true)
{
$reader = $this->createReaderForFile($file, $shouldPreserveEmptyRows);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather inject the ReaderInterface directly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed, the user must call the ReaderInterface::open method before use the SpoutReader

Copy link
Author

@aaa2000 aaa2000 Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With this fix, the SpoutReader is compatible with CSV, XLSX and ODS.

But the method count of the ODS format throw an exception because the RowIterator can't be rewind more than once. See https://github.com/box/spout/blob/3681a3421a868ab9a65da156c554f756541f452b/src/Spout/Reader/ODS/RowIterator.php#L101

Maybe, I will not implement the CountableReader interface

*/
public function setHeaderRowNumber($rowNumber)
{
$rowNumber = (int) $rowNumber;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather use a static type hint for this method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to require PHP7

*
* @return array
*/
public function getRow($number)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the use case for this method?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

composer.json Outdated
@@ -23,16 +23,17 @@
"docs": "http://docs.portphp.org"
},
"require": {
"php": ">=5.4.0"
"php": ">=5.5.0",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should require at least php 5.6, because portphp/portphp requires at least php 5.6.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use box/spout instead of phpoffice/phpexcel
2 participants