mcndave/spout
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Code review changes

Browse Source
This commit is contained in:
Antoine Lamirault 2021-02-22 20:04:35 +01:00 committed by Adrien Loison
parent 197fb9987a
commit 11d91e1740
20 changed files with 105 additions and 59 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/Spout/Common/Entity/Cell.php
View File

@ -2,7 +2,6 @@
namespace Box\Spout\Common\Entity;
use Box\Spout\Common\Entity\Style\EmptyStyle;
use Box\Spout\Common\Entity\Style\Style;
use Box\Spout\Common\Helper\CellTypeHelper;
@ -105,7 +104,7 @@ class Cell
*/
public function setStyle($style)
{
$this->style = $style ?: new EmptyStyle();
$this->style = $style ?: new Style();
}
/**

7
src/Spout/Common/Entity/Style/EmptyStyle.php
View File

@ -1,7 +0,0 @@
<?php
namespace Box\Spout\Common\Entity\Style;
class EmptyStyle extends Style
{
}

31
src/Spout/Common/Entity/Style/Style.php
View File

@ -87,6 +87,9 @@ class Style
/** @var bool */
private $isRegistered = false;
/** @var bool */
private $isEmpty = true;
/**
* @return int|null
*/
@ -122,6 +125,7 @@ class Style
{
$this->shouldApplyBorder = true;
$this->border = $border;
$this->isEmpty = false;
return $this;
}
@ -150,6 +154,7 @@ class Style
$this->fontBold = true;
$this->hasSetFontBold = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -178,6 +183,7 @@ class Style
$this->fontItalic = true;
$this->hasSetFontItalic = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -206,6 +212,7 @@ class Style
$this->fontUnderline = true;
$this->hasSetFontUnderline = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -234,6 +241,7 @@ class Style
$this->fontStrikethrough = true;
$this->hasSetFontStrikethrough = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -263,6 +271,7 @@ class Style
$this->fontSize = $fontSize;
$this->hasSetFontSize = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -294,6 +303,7 @@ class Style
$this->fontColor = $fontColor;
$this->hasSetFontColor = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -323,6 +333,7 @@ class Style
$this->fontName = $fontName;
$this->hasSetFontName = true;
$this->shouldApplyFont = true;
$this->isEmpty = false;
return $this;
}
@ -353,6 +364,7 @@ class Style
$this->cellAlignment = $cellAlignment;
$this->hasSetCellAlignment = true;
$this->shouldApplyCellAlignment = true;
$this->isEmpty = false;
return $this;
}
@ -389,6 +401,7 @@ class Style
{
$this->shouldWrapText = $shouldWrap;
$this->hasSetWrapText = true;
$this->isEmpty = false;
return $this;
}
@ -418,6 +431,7 @@ class Style
{
$this->hasSetBackgroundColor = true;
$this->backgroundColor = $color;
$this->isEmpty = false;
return $this;
}
@ -447,6 +461,7 @@ class Style
{
$this->hasSetFormat = true;
$this->format = $format;
$this->isEmpty = false;
return $this;
}
@ -467,11 +482,6 @@ class Style
return $this->hasSetFormat;
}
public function setRegistered(bool $isRegistered = true) : void
{
$this->isRegistered = $isRegistered;
}
/**
* @return bool
*/
@ -485,4 +495,15 @@ class Style
$this->setId($id);
$this->isRegistered = true;
}
public function unregister() : void
{
$this->setId(0);
$this->isRegistered = false;
}
public function isEmpty() : bool
{
return $this->isEmpty;
}
}

27
src/Spout/Writer/Common/Manager/Style/ManagedStyle.php Normal file
View File

@ -0,0 +1,27 @@
<?php
namespace Box\Spout\Writer\Common\Manager\Style;
use Box\Spout\Common\Entity\Style\Style;
class ManagedStyle
{
private $style;
private $isUpdated;
public function __construct(Style $style, bool $isUpdated)
{
$this->style = $style;
$this->isUpdated = $isUpdated;
}
public function getStyle() : Style
{
return $this->style;
}
public function isUpdated() : bool
{
return $this->isUpdated;
}
}

18
src/Spout/Writer/Common/Manager/Style/StyleManager.php
View File

@ -50,9 +50,9 @@ class StyleManager implements StyleManagerInterface
* Typically, set "wrap text" if a cell contains a new line.
*
* @param Cell $cell
* @return Style|null The eventually updated style
* @return ManagedStyle The eventually updated style
*/
public function applyExtraStylesIfNeeded(Cell $cell) : ?Style
public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle
{
return $this->applyWrapTextIfCellContainsNewLine($cell);
}
@ -67,23 +67,19 @@ class StyleManager implements StyleManagerInterface
* on the Windows version of Excel...
*
* @param Cell $cell The cell the style should be applied to
* @return Style|null The eventually updated style
* @return ManagedStyle The eventually updated style
*/
protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : ?Style
protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : ManagedStyle
{
$cellStyle = $cell->getStyle();
// if the "wrap text" option is already set, no-op
if ($cellStyle->hasSetWrapText()) {
return null;
}
if ($cell->isString() && \strpos($cell->getValue(), "\n") !== false) {
if (!$cellStyle->hasSetWrapText() && $cell->isString() && \strpos($cell->getValue(), "\n") !== false) {
$cellStyle->setShouldWrapText();
return $cellStyle;
return new ManagedStyle($cellStyle, true);
}
return null;
return new ManagedStyle($cellStyle, false);
}
}

4
src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php
View File

@ -24,7 +24,7 @@ interface StyleManagerInterface
* Typically, set "wrap text" if a cell contains a new line.
*
* @param Cell $cell
* @return Style|null The eventually updated style
* @return ManagedStyle|null The eventually updated style
*/
public function applyExtraStylesIfNeeded(Cell $cell) : ?Style;
public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle;
}

3
src/Spout/Writer/Common/Manager/Style/StyleRegistry.php
View File

@ -114,8 +114,7 @@ class StyleRegistry
{
// In order to be able to properly compare style, set static ID value and reset registration
$currentId = $style->getId();
$style->setId(0);
$style->setRegistered(false);
$style->unregister();
$serializedStyle = \serialize($style);

17
src/Spout/Writer/ODS/Manager/WorksheetManager.php
View File

@ -4,7 +4,6 @@ namespace Box\Spout\Writer\ODS\Manager;
use Box\Spout\Common\Entity\Cell;
use Box\Spout\Common\Entity\Row;
use Box\Spout\Common\Entity\Style\EmptyStyle;
use Box\Spout\Common\Entity\Style\Style;
use Box\Spout\Common\Exception\InvalidArgumentException;
use Box\Spout\Common\Exception\IOException;
@ -158,13 +157,13 @@ class WorksheetManager implements WorksheetManagerInterface
*/
private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $currentCellIndex, $nextCellIndex)
{
if ($cell->getStyle() instanceof EmptyStyle) {
if ($cell->getStyle()->isEmpty()) {
$cell->setStyle($rowStyle);
$extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
$managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
if ($extraStyle) {
$registeredStyle = $this->styleManager->registerStyle($extraStyle);
if ($managedStyle->isUpdated()) {
$registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle());
} else {
$registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle);
}
@ -172,7 +171,13 @@ class WorksheetManager implements WorksheetManagerInterface
$mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle);
$cell->setStyle($mergedCellAndRowStyle);
$newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle;
$managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
if ($managedStyle->isUpdated()) {
$newCellStyle = $managedStyle->getStyle();
} else {
$newCellStyle = $mergedCellAndRowStyle;
}
$registeredStyle = $this->styleManager->registerStyle($newCellStyle);
}

18
src/Spout/Writer/XLSX/Manager/WorksheetManager.php
View File

@ -4,7 +4,6 @@ namespace Box\Spout\Writer\XLSX\Manager;
use Box\Spout\Common\Entity\Cell;
use Box\Spout\Common\Entity\Row;
use Box\Spout\Common\Entity\Style\EmptyStyle;
use Box\Spout\Common\Entity\Style\Style;
use Box\Spout\Common\Exception\InvalidArgumentException;
use Box\Spout\Common\Exception\IOException;
@ -186,13 +185,13 @@ EOD;
*/
private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $rowIndexOneBased, $columnIndexZeroBased)
{
if ($cell->getStyle() instanceof EmptyStyle) {
if ($cell->getStyle()->isEmpty()) {
$cell->setStyle($rowStyle);
$extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
$managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
if ($extraStyle) {
$registeredStyle = $this->styleManager->registerStyle($extraStyle);
if ($managedStyle->isUpdated()) {
$registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle());
} else {
$registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle);
}
@ -200,7 +199,14 @@ EOD;
$mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle);
$cell->setStyle($mergedCellAndRowStyle);
$newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle;
$managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
if ($managedStyle->isUpdated()) {
$newCellStyle = $managedStyle->getStyle();
} else {
$newCellStyle = $mergedCellAndRowStyle;
}
$registeredStyle = $this->styleManager->registerStyle($newCellStyle);
}

2
tests/Spout/Writer/CSV/WriterPerfTest.php
View File

@ -61,7 +61,7 @@ class WriterPerfTest extends TestCase
*/
private function getNumWrittenRows($resourcePath)
{
$lineCountResult = `wc -l $resourcePath`;
$lineCountResult = shell_exec("wc -l $resourcePath");
return (int) $lineCountResult;
}

14
tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php
View File

@ -28,10 +28,10 @@ class StyleManagerTest extends TestCase
$this->assertFalse($style->shouldWrapText());
$styleManager = $this->getStyleManager();
$updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
$managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
$this->assertNotNull($updatedStyle);
$this->assertTrue($updatedStyle->shouldWrapText());
$this->assertTrue($managedStyle->isUpdated());
$this->assertTrue($managedStyle->getStyle()->shouldWrapText());
}
public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextNotNeeded() : void
@ -40,9 +40,9 @@ class StyleManagerTest extends TestCase
$this->assertFalse($style->shouldWrapText());
$styleManager = $this->getStyleManager();
$updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style));
$managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style));
$this->assertNull($updatedStyle);
$this->assertFalse($managedStyle->isUpdated());
}
public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextAlreadyApplied() : void
@ -51,8 +51,8 @@ class StyleManagerTest extends TestCase
$this->assertTrue($style->shouldWrapText());
$styleManager = $this->getStyleManager();
$updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
$managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
$this->assertNull($updatedStyle);
$this->assertFalse($managedStyle->isUpdated());
}
}

2
tests/Spout/Writer/ODS/WriterPerfTest.php
View File

@ -88,7 +88,7 @@ class WriterPerfTest extends TestCase
copy($pathToContentXmlFile, $tmpFile);
// Get the last 200 characters
$lastCharacters = `tail -c 200 $tmpFile`;
$lastCharacters = shell_exec("tail -c 200 $tmpFile");
// remove the temporary file
unlink($tmpFile);

2
tests/Spout/Writer/XLSX/WriterPerfTest.php
View File

@ -135,7 +135,7 @@ class WriterPerfTest extends TestCase
copy($filePath, $tmpFile);
// Get the last 200 characters
$lastCharacters = `tail -c $numCharacters $tmpFile`;
$lastCharacters = shell_exec("tail -c $numCharacters $tmpFile");
// remove the temporary file
unlink($tmpFile);