-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this 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.
src/SpoutReader.php
Outdated
*/ | ||
public function __construct(\SplFileObject $file, $headerRowNumber = null, $activeSheet = null, $shouldPreserveEmptyRows = true) | ||
{ | ||
$reader = $this->createReaderForFile($file, $shouldPreserveEmptyRows); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to require PHP7
src/SpoutReader.php
Outdated
* | ||
* @return array | ||
*/ | ||
public function getRow($number) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
I have added it because the ExcelReader implement it https://github.com/portphp/excel/blob/458d5cf105206f6391e4350ee095110d18eda090/src/ExcelReader.php#L201
composer.json
Outdated
@@ -23,16 +23,17 @@ | |||
"docs": "http://docs.portphp.org" | |||
}, | |||
"require": { | |||
"php": ">=5.4.0" | |||
"php": ">=5.5.0", |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
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 butspout
doesn't manage this format. I converted the XLS file in XLSX format withlibreoffice
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