diff --git a/src/Spout/Common/Entity/Style/Style.php b/src/Spout/Common/Entity/Style/Style.php index 7e989a4..bda0eb7 100644 --- a/src/Spout/Common/Entity/Style/Style.php +++ b/src/Spout/Common/Entity/Style/Style.php @@ -84,6 +84,9 @@ class Style /** @var bool */ private $hasSetFormat = false; + /** @var bool */ + private $isRegistered = false; + /** * @return int|null */ @@ -463,4 +466,23 @@ class Style { return $this->hasSetFormat; } + + public function setRegistered(bool $isRegistered = true) : void + { + $this->isRegistered = $isRegistered; + } + + /** + * @return bool + */ + public function isRegistered() : bool + { + return $this->isRegistered; + } + + public function register(int $id) : void + { + $this->setId($id); + $this->isRegistered = true; + } } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManager.php b/src/Spout/Writer/Common/Manager/Style/StyleManager.php index 77eec73..bf87958 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManager.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManager.php @@ -50,13 +50,11 @@ class StyleManager implements StyleManagerInterface * Typically, set "wrap text" if a cell contains a new line. * * @param Cell $cell - * @return Style + * @return Style|null The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell) + public function applyExtraStylesIfNeeded(Cell $cell) : ?Style { - $updatedStyle = $this->applyWrapTextIfCellContainsNewLine($cell); - - return $updatedStyle; + return $this->applyWrapTextIfCellContainsNewLine($cell); } /** @@ -69,21 +67,23 @@ class StyleManager implements StyleManagerInterface * on the Windows version of Excel... * * @param Cell $cell The cell the style should be applied to - * @return \Box\Spout\Common\Entity\Style\Style The eventually updated style + * @return Style|null The eventually updated style */ - protected function applyWrapTextIfCellContainsNewLine(Cell $cell) + protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : ?Style { $cellStyle = $cell->getStyle(); // if the "wrap text" option is already set, no-op if ($cellStyle->hasSetWrapText()) { - return $cellStyle; + return null; } if ($cell->isString() && \strpos($cell->getValue(), "\n") !== false) { $cellStyle->setShouldWrapText(); + + return $cellStyle; } - return $cellStyle; + return null; } } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php index 312588e..9929eeb 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php @@ -24,7 +24,7 @@ interface StyleManagerInterface * Typically, set "wrap text" if a cell contains a new line. * * @param Cell $cell - * @return Style The updated style + * @return Style|null The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell); + public function applyExtraStylesIfNeeded(Cell $cell) : ?Style; } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php index 4d8062c..ea7b7a4 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php @@ -38,7 +38,7 @@ class StyleRegistry if (!$this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle)) { $nextStyleId = \count($this->serializedStyleToStyleIdMappingTable); - $style->setId($nextStyleId); + $style->register($nextStyleId); $this->serializedStyleToStyleIdMappingTable[$serializedStyle] = $nextStyleId; $this->styleIdToStyleMappingTable[$nextStyleId] = $style; @@ -112,9 +112,10 @@ class StyleRegistry */ public function serialize(Style $style) { - // In order to be able to properly compare style, set static ID value + // 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); $serializedStyle = \serialize($style); diff --git a/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php b/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php index 6c580d4..42484f2 100644 --- a/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php @@ -22,6 +22,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry */ public function registerStyle(Style $style) { + if ($style->isRegistered()) { + return $style; + } + $registeredStyle = parent::registerStyle($style); $this->usedFontsSet[$style->getFontName()] = true; diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 970cace..b7c4e6e 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -156,18 +156,26 @@ class WorksheetManager implements WorksheetManagerInterface * @throws InvalidArgumentException If a cell value's type is not supported * @return string */ - private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $currentCellIndex, $nextCellIndex) + private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $currentCellIndex, $nextCellIndex) { if ($cell->getStyle() instanceof EmptyStyle) { $cell->setStyle($rowStyle); + + $extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + + if ($extraStyle) { + $registeredStyle = $this->styleManager->registerStyle($extraStyle); + } else { + $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle); + } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); $cell->setStyle($mergedCellAndRowStyle); + + $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle; + $registeredStyle = $this->styleManager->registerStyle($newCellStyle); } - $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - - $registeredStyle = $this->styleManager->registerStyle($newCellStyle); $styleIndex = $registeredStyle->getId() + 1; // 1-based $numTimesValueRepeated = ($nextCellIndex - $currentCellIndex); diff --git a/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php b/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php index ace607c..14eb986 100644 --- a/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php @@ -119,6 +119,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry */ public function registerStyle(Style $style) { + if ($style->isRegistered()) { + return $style; + } + $registeredStyle = parent::registerStyle($style); $this->registerFill($registeredStyle); $this->registerFormat($registeredStyle); diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index e49c1d8..ae1ecbb 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -184,17 +184,25 @@ EOD; * @throws InvalidArgumentException If the given value cannot be processed * @return string */ - private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndexOneBased, $columnIndexZeroBased) + private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $rowIndexOneBased, $columnIndexZeroBased) { if ($cell->getStyle() instanceof EmptyStyle) { $cell->setStyle($rowStyle); + + $extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + + if ($extraStyle) { + $registeredStyle = $this->styleManager->registerStyle($extraStyle); + } else { + $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle); + } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); $cell->setStyle($mergedCellAndRowStyle); - } - $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - $registeredStyle = $this->styleManager->registerStyle($newCellStyle); + $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle; + $registeredStyle = $this->styleManager->registerStyle($newCellStyle); + } return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId()); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php index 82292bb..d326f9d 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php @@ -14,7 +14,7 @@ class StyleManagerTest extends TestCase /** * @return StyleManager */ - private function getStyleManager() + private function getStyleManager() : StyleManager { $style = (new StyleBuilder())->build(); $styleRegistry = new StyleRegistry($style); @@ -22,10 +22,7 @@ class StyleManagerTest extends TestCase return new StyleManager($styleRegistry); } - /** - * @return void - */ - public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine() + public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine() : void { $style = (new StyleBuilder())->build(); $this->assertFalse($style->shouldWrapText()); @@ -33,13 +30,22 @@ class StyleManagerTest extends TestCase $styleManager = $this->getStyleManager(); $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); + $this->assertNotNull($updatedStyle); $this->assertTrue($updatedStyle->shouldWrapText()); } - /** - * @return void - */ - public function testApplyExtraStylesIfNeededShouldDoNothingIfWrapTextAlreadyApplied() + public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextNotNeeded() : void + { + $style = (new StyleBuilder())->build(); + $this->assertFalse($style->shouldWrapText()); + + $styleManager = $this->getStyleManager(); + $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style)); + + $this->assertNull($updatedStyle); + } + + public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextAlreadyApplied() : void { $style = (new StyleBuilder())->setShouldWrapText()->build(); $this->assertTrue($style->shouldWrapText()); @@ -47,6 +53,6 @@ class StyleManagerTest extends TestCase $styleManager = $this->getStyleManager(); $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); - $this->assertTrue($updatedStyle->shouldWrapText()); + $this->assertNull($updatedStyle); } }