From 816596183fbcf7dd7dd158630b15b7e0620d7f4b Mon Sep 17 00:00:00 2001 From: Petr Skoda Date: Tue, 29 Dec 2020 09:18:59 +1300 Subject: [PATCH 01/17] Add full support for PHP 8.0 Unfortunately due to PHPUnit 8.5 dependency this also drops support for PHP 7.1 --- .gitignore | 1 + .travis.yml | 8 ++++---- README.md | 2 +- UPGRADE-3.0.md | 2 +- composer.json | 6 +++--- docs/_pages/getting-started.md | 2 +- phpunit.xml | 2 +- tests/Spout/Common/Entity/RowTest.php | 2 +- tests/Spout/Common/Helper/FileSystemHelperTest.php | 2 +- tests/Spout/Common/Manager/OptionsManagerTest.php | 2 +- tests/Spout/Reader/ODS/ReaderTest.php | 8 ++++++++ .../Reader/XLSX/Manager/SharedStringsManagerTest.php | 4 ++-- tests/Spout/Reader/XLSX/ReaderTest.php | 4 ++++ tests/Spout/Writer/CSV/WriterTest.php | 4 ++-- tests/Spout/Writer/Common/Entity/SheetTest.php | 2 +- .../Spout/Writer/Common/Manager/Style/StyleMergerTest.php | 2 +- .../Writer/Common/Manager/Style/StyleRegistryTest.php | 2 +- tests/Spout/Writer/ODS/SheetTest.php | 4 ++-- tests/Spout/Writer/ODS/WriterTest.php | 6 +++--- tests/Spout/Writer/ODS/WriterWithStyleTest.php | 2 +- tests/Spout/Writer/XLSX/SheetTest.php | 4 ++-- tests/Spout/Writer/XLSX/WriterTest.php | 6 +++--- tests/Spout/Writer/XLSX/WriterWithStyleTest.php | 2 +- 23 files changed, 46 insertions(+), 33 deletions(-) diff --git a/.gitignore b/.gitignore index 048320b..6253664 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,4 @@ /vendor /composer.lock /.php_cs.cache +/.phpunit.result.cache diff --git a/.travis.yml b/.travis.yml index ac931ed..9babd77 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,16 +5,16 @@ language: php matrix: include: - - php: 7.1 - env: WITH_CS=true - - php: 7.1 - env: WITH_PHPUNIT=true WITH_COVERAGE=true - php: 7.2 env: WITH_PHPUNIT=true - php: 7.3 env: WITH_PHPUNIT=true + - php: 7.3 + env: WITH_PHPUNIT=true WITH_COVERAGE=true - php: 7.4 env: WITH_PHPUNIT=true + - php: 8.0 + env: WITH_PHPUNIT=true cache: diff --git a/README.md b/README.md index 517f5a4..a9a0a9c 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,7 @@ Full documentation can be found at [https://opensource.box.com/spout/](https://o ## Requirements -* PHP version 7.1 or higher +* PHP version 7.2 or higher * PHP extension `php_zip` enabled * PHP extension `php_xmlreader` enabled diff --git a/UPGRADE-3.0.md b/UPGRADE-3.0.md index 3952443..9b2e370 100644 --- a/UPGRADE-3.0.md +++ b/UPGRADE-3.0.md @@ -11,7 +11,7 @@ With the 3.0 version, this is now possible: each cell can have its own style. Spout 3.0 tries to enforce better typing. For instance, instead of using/returning generic arrays, Spout now makes use of specific `Row` and `Cell` objects that can encapsulate more data such as type, style, value. -Finally, **_Spout 3.0 only supports PHP 7.1 and above_**, as other PHP versions are no longer supported by the community. +Finally, **_Spout 3.2 only supports PHP 7.2 and above_**, as other PHP versions are no longer supported by the community. Reader changes -------------- diff --git a/composer.json b/composer.json index 8943469..4d9642b 100644 --- a/composer.json +++ b/composer.json @@ -12,12 +12,12 @@ } ], "require": { - "php": ">=7.1.0", + "php": ">=7.2.0", "ext-zip": "*", "ext-xmlreader" : "*" }, "require-dev": { - "phpunit/phpunit": "^7", + "phpunit/phpunit": "^8", "friendsofphp/php-cs-fixer": "^2" }, "suggest": { @@ -36,7 +36,7 @@ }, "config": { "platform": { - "php": "7.1" + "php": "7.2" } } } diff --git a/docs/_pages/getting-started.md b/docs/_pages/getting-started.md index 0004d4d..0e0b336 100755 --- a/docs/_pages/getting-started.md +++ b/docs/_pages/getting-started.md @@ -10,7 +10,7 @@ This guide will help you install {{ site.spout_html }} and teach you how to use ## Requirements -* PHP version 7.1 or higher +* PHP version 7.2 or higher * PHP extension `ext-zip` enabled * PHP extension `ext-xmlreader` enabled diff --git a/phpunit.xml b/phpunit.xml index 4c56189..4e410e2 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -1,6 +1,6 @@ setStyle($this->getStyleMock()) ->setCells([]); - $this->assertInternalType('object', $row); + $this->assertInstanceOf(Row::class, $row); } } diff --git a/tests/Spout/Common/Helper/FileSystemHelperTest.php b/tests/Spout/Common/Helper/FileSystemHelperTest.php index f99ab30..7eb0183 100644 --- a/tests/Spout/Common/Helper/FileSystemHelperTest.php +++ b/tests/Spout/Common/Helper/FileSystemHelperTest.php @@ -16,7 +16,7 @@ class FileSystemHelperTest extends TestCase /** * @return void */ - public function setUp() + public function setUp() : void { $baseFolder = \sys_get_temp_dir(); $this->fileSystemHelper = new FileSystemHelper($baseFolder); diff --git a/tests/Spout/Common/Manager/OptionsManagerTest.php b/tests/Spout/Common/Manager/OptionsManagerTest.php index e2f0c5c..b1cac58 100644 --- a/tests/Spout/Common/Manager/OptionsManagerTest.php +++ b/tests/Spout/Common/Manager/OptionsManagerTest.php @@ -14,7 +14,7 @@ class OptionsManagerTest extends TestCase */ protected $optionsManager; - protected function setUp() + protected function setUp(): void { $this->optionsManager = new class() extends OptionsManagerAbstract { protected function getSupportedOptions() diff --git a/tests/Spout/Reader/ODS/ReaderTest.php b/tests/Spout/Reader/ODS/ReaderTest.php index ce56e4b..cd30ae5 100644 --- a/tests/Spout/Reader/ODS/ReaderTest.php +++ b/tests/Spout/Reader/ODS/ReaderTest.php @@ -295,6 +295,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstBillionLaughsAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $fileName = 'attack_billion_laughs.ods'; @@ -318,6 +322,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstQuadraticBlowupAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $fileName = 'attack_quadratic_blowup.ods'; diff --git a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php index 2f85f34..22df444 100644 --- a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php +++ b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php @@ -25,7 +25,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->sharedStringsManager = null; } @@ -33,7 +33,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function tearDown() + public function tearDown(): void { if ($this->sharedStringsManager !== null) { $this->sharedStringsManager->cleanup(); diff --git a/tests/Spout/Reader/XLSX/ReaderTest.php b/tests/Spout/Reader/XLSX/ReaderTest.php index 51a7b0e..7d30f97 100644 --- a/tests/Spout/Reader/XLSX/ReaderTest.php +++ b/tests/Spout/Reader/XLSX/ReaderTest.php @@ -539,6 +539,10 @@ class ReaderTest extends TestCase */ public function testReadShouldBeProtectedAgainstQuadraticBlowupAttack() { + if (function_exists('xdebug_code_coverage_started') && xdebug_code_coverage_started()) { + $this->markTestSkipped('test not compatible with code coverage'); + } + $startTime = microtime(true); $this->getAllRowsForFile('attack_quadratic_blowup.xlsx'); diff --git a/tests/Spout/Writer/CSV/WriterTest.php b/tests/Spout/Writer/CSV/WriterTest.php index 0b3b238..deb3f22 100644 --- a/tests/Spout/Writer/CSV/WriterTest.php +++ b/tests/Spout/Writer/CSV/WriterTest.php @@ -101,7 +101,7 @@ class WriterTest extends TestCase ]); $writtenContent = $this->writeToCsvFileAndReturnWrittenContent($allRows, 'csv_with_utf8_bom.csv'); - $this->assertContains(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should contain a UTF-8 BOM'); + $this->assertStringStartsWith(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should contain a UTF-8 BOM'); } /** @@ -114,7 +114,7 @@ class WriterTest extends TestCase ]); $writtenContent = $this->writeToCsvFileAndReturnWrittenContent($allRows, 'csv_no_bom.csv', ',', '"', false); - $this->assertNotContains(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should not contain a UTF-8 BOM'); + $this->assertStringNotContainsString(EncodingHelper::BOM_UTF8, $writtenContent, 'The CSV file should not contain a UTF-8 BOM'); } /** diff --git a/tests/Spout/Writer/Common/Entity/SheetTest.php b/tests/Spout/Writer/Common/Entity/SheetTest.php index cb19d6f..3c50734 100644 --- a/tests/Spout/Writer/Common/Entity/SheetTest.php +++ b/tests/Spout/Writer/Common/Entity/SheetTest.php @@ -18,7 +18,7 @@ class SheetTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->sheetManager = new SheetManager(new StringHelper()); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php index 46820c0..a91d13f 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php @@ -18,7 +18,7 @@ class StyleMergerTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->styleMerger = new StyleMerger(); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php index 41cc571..20a764c 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php @@ -20,7 +20,7 @@ class StyleRegistryTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->defaultStyle = (new StyleBuilder())->build(); $this->styleRegistry = new StyleRegistry($this->defaultStyle); diff --git a/tests/Spout/Writer/ODS/SheetTest.php b/tests/Spout/Writer/ODS/SheetTest.php index a964a59..3bd2193 100644 --- a/tests/Spout/Writer/ODS/SheetTest.php +++ b/tests/Spout/Writer/ODS/SheetTest.php @@ -89,7 +89,7 @@ class SheetTest extends TestCase $pathToContentFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToContentFile); - $this->assertContains(' table:display="false"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); + $this->assertStringContainsString(' table:display="false"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); } /** @@ -164,6 +164,6 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains("table:name=\"$expectedName\"", $xmlContents, $message); + $this->assertStringContainsString("table:name=\"$expectedName\"", $xmlContents, $message); } } diff --git a/tests/Spout/Writer/ODS/WriterTest.php b/tests/Spout/Writer/ODS/WriterTest.php index b30a8f6..31dcccf 100644 --- a/tests/Spout/Writer/ODS/WriterTest.php +++ b/tests/Spout/Writer/ODS/WriterTest.php @@ -547,7 +547,7 @@ class WriterTest extends TestCase $pathToContentFile = $resourcePath . '#content.xml'; $xmlContents = file_get_contents('zip://' . $pathToContentFile); - $this->assertContains($value, $xmlContents, $message); + $this->assertStringContainsString($value, $xmlContents, $message); } /** @@ -562,7 +562,7 @@ class WriterTest extends TestCase $sheetXmlAsString = $this->getSheetXmlNodeAsString($fileName, $sheetIndex); $valueAsXmlString = "$value"; - $this->assertContains($valueAsXmlString, $sheetXmlAsString, $message); + $this->assertStringContainsString($valueAsXmlString, $sheetXmlAsString, $message); } /** @@ -577,7 +577,7 @@ class WriterTest extends TestCase $sheetXmlAsString = $this->getSheetXmlNodeAsString($fileName, $sheetIndex); $valueAsXmlString = "$value"; - $this->assertNotContains($valueAsXmlString, $sheetXmlAsString, $message); + $this->assertStringNotContainsString($valueAsXmlString, $sheetXmlAsString, $message); } /** diff --git a/tests/Spout/Writer/ODS/WriterWithStyleTest.php b/tests/Spout/Writer/ODS/WriterWithStyleTest.php index dcdd364..669d438 100644 --- a/tests/Spout/Writer/ODS/WriterWithStyleTest.php +++ b/tests/Spout/Writer/ODS/WriterWithStyleTest.php @@ -30,7 +30,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->defaultStyle = (new StyleBuilder())->build(); } diff --git a/tests/Spout/Writer/XLSX/SheetTest.php b/tests/Spout/Writer/XLSX/SheetTest.php index cebd72e..a243161 100644 --- a/tests/Spout/Writer/XLSX/SheetTest.php +++ b/tests/Spout/Writer/XLSX/SheetTest.php @@ -89,7 +89,7 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains(' state="hidden"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); + $this->assertStringContainsString(' state="hidden"', $xmlContents, 'The sheet visibility should have been changed to "hidden"'); } /** @@ -166,6 +166,6 @@ class SheetTest extends TestCase $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile); - $this->assertContains("assertStringContainsString("assertContains((string) $inlineData, $xmlContents, $message); + $this->assertStringContainsString((string) $inlineData, $xmlContents, $message); } /** @@ -624,7 +624,7 @@ class WriterTest extends TestCase $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml'; $xmlContents = file_get_contents('zip://' . $pathToSheetFile); - $this->assertNotContains((string) $inlineData, $xmlContents, $message); + $this->assertStringNotContainsString((string) $inlineData, $xmlContents, $message); } /** @@ -639,6 +639,6 @@ class WriterTest extends TestCase $pathToSharedStringsFile = $resourcePath . '#xl/sharedStrings.xml'; $xmlContents = file_get_contents('zip://' . $pathToSharedStringsFile); - $this->assertContains($sharedString, $xmlContents, $message); + $this->assertStringContainsString($sharedString, $xmlContents, $message); } } diff --git a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php index da0aec2..da063e8 100644 --- a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php +++ b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php @@ -32,7 +32,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp() + public function setUp(): void { $this->defaultStyle = (new StyleBuilder())->build(); } From c29d1877b8a5aef44c3a39930aa5b800db7cb35f Mon Sep 17 00:00:00 2001 From: jmsche Date: Fri, 29 May 2020 11:42:25 +0200 Subject: [PATCH 02/17] Fixed code style (probably due to recent php-cs-fixer version) --- src/Spout/Reader/ODS/SheetIterator.php | 4 ++-- src/Spout/Reader/XLSX/Manager/SharedStringsManager.php | 2 +- src/Spout/Reader/XLSX/RowIterator.php | 2 +- src/Spout/Writer/Common/Manager/WorkbookManagerAbstract.php | 2 +- src/Spout/Writer/ODS/Creator/ManagerFactory.php | 2 +- src/Spout/Writer/WriterAbstract.php | 2 +- src/Spout/Writer/XLSX/Creator/ManagerFactory.php | 2 +- tests/Spout/Reader/CSV/SpoutTestStream.php | 4 ++-- 8 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/Spout/Reader/ODS/SheetIterator.php b/src/Spout/Reader/ODS/SheetIterator.php index f35b852..c7b8cd9 100644 --- a/src/Spout/Reader/ODS/SheetIterator.php +++ b/src/Spout/Reader/ODS/SheetIterator.php @@ -28,13 +28,13 @@ class SheetIterator implements IteratorInterface const XML_ATTRIBUTE_TABLE_STYLE_NAME = 'table:style-name'; const XML_ATTRIBUTE_TABLE_DISPLAY = 'table:display'; - /** @var string $filePath Path of the file to be read */ + /** @var string Path of the file to be read */ protected $filePath; /** @var \Box\Spout\Common\Manager\OptionsManagerInterface Reader's options manager */ protected $optionsManager; - /** @var InternalEntityFactory $entityFactory Factory to create entities */ + /** @var InternalEntityFactory Factory to create entities */ protected $entityFactory; /** @var XMLReader The XMLReader object that will help read sheet's XML data */ diff --git a/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php b/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php index caaeed7..8850a69 100644 --- a/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php +++ b/src/Spout/Reader/XLSX/Manager/SharedStringsManager.php @@ -43,7 +43,7 @@ class SharedStringsManager /** @var InternalEntityFactory Factory to create entities */ protected $entityFactory; - /** @var HelperFactory $helperFactory Factory to create helpers */ + /** @var HelperFactory Factory to create helpers */ protected $helperFactory; /** @var CachingStrategyFactory Factory to create shared strings caching strategies */ diff --git a/src/Spout/Reader/XLSX/RowIterator.php b/src/Spout/Reader/XLSX/RowIterator.php index 4af4530..a54b8b1 100644 --- a/src/Spout/Reader/XLSX/RowIterator.php +++ b/src/Spout/Reader/XLSX/RowIterator.php @@ -35,7 +35,7 @@ class RowIterator implements IteratorInterface /** @var string Path of the XLSX file being read */ protected $filePath; - /** @var string $sheetDataXMLFilePath Path of the sheet data XML file as in [Content_Types].xml */ + /** @var string Path of the sheet data XML file as in [Content_Types].xml */ protected $sheetDataXMLFilePath; /** @var \Box\Spout\Reader\Wrapper\XMLReader The XMLReader object that will help read sheet's XML data */ diff --git a/src/Spout/Writer/Common/Manager/WorkbookManagerAbstract.php b/src/Spout/Writer/Common/Manager/WorkbookManagerAbstract.php index b513555..653778c 100644 --- a/src/Spout/Writer/Common/Manager/WorkbookManagerAbstract.php +++ b/src/Spout/Writer/Common/Manager/WorkbookManagerAbstract.php @@ -44,7 +44,7 @@ abstract class WorkbookManagerAbstract implements WorkbookManagerInterface /** @var InternalEntityFactory Factory to create entities */ protected $entityFactory; - /** @var ManagerFactoryInterface $managerFactory Factory to create managers */ + /** @var ManagerFactoryInterface Factory to create managers */ protected $managerFactory; /** @var Worksheet The worksheet where data will be written to */ diff --git a/src/Spout/Writer/ODS/Creator/ManagerFactory.php b/src/Spout/Writer/ODS/Creator/ManagerFactory.php index f38c500..a5b77ee 100644 --- a/src/Spout/Writer/ODS/Creator/ManagerFactory.php +++ b/src/Spout/Writer/ODS/Creator/ManagerFactory.php @@ -22,7 +22,7 @@ class ManagerFactory implements ManagerFactoryInterface /** @var InternalEntityFactory */ protected $entityFactory; - /** @var HelperFactory $helperFactory */ + /** @var HelperFactory */ protected $helperFactory; /** diff --git a/src/Spout/Writer/WriterAbstract.php b/src/Spout/Writer/WriterAbstract.php index d96a628..bbaa735 100644 --- a/src/Spout/Writer/WriterAbstract.php +++ b/src/Spout/Writer/WriterAbstract.php @@ -33,7 +33,7 @@ abstract class WriterAbstract implements WriterInterface /** @var GlobalFunctionsHelper Helper to work with global functions */ protected $globalFunctionsHelper; - /** @var HelperFactory $helperFactory */ + /** @var HelperFactory */ protected $helperFactory; /** @var OptionsManagerInterface Writer options manager */ diff --git a/src/Spout/Writer/XLSX/Creator/ManagerFactory.php b/src/Spout/Writer/XLSX/Creator/ManagerFactory.php index f27a2f2..aa3bcd5 100644 --- a/src/Spout/Writer/XLSX/Creator/ManagerFactory.php +++ b/src/Spout/Writer/XLSX/Creator/ManagerFactory.php @@ -24,7 +24,7 @@ class ManagerFactory implements ManagerFactoryInterface /** @var InternalEntityFactory */ protected $entityFactory; - /** @var HelperFactory $helperFactory */ + /** @var HelperFactory */ protected $helperFactory; /** diff --git a/tests/Spout/Reader/CSV/SpoutTestStream.php b/tests/Spout/Reader/CSV/SpoutTestStream.php index 3bfd06e..d66e811 100644 --- a/tests/Spout/Reader/CSV/SpoutTestStream.php +++ b/tests/Spout/Reader/CSV/SpoutTestStream.php @@ -14,10 +14,10 @@ class SpoutTestStream const PATH_TO_CSV_RESOURCES = 'tests/resources/csv/'; const CSV_EXTENSION = '.csv'; - /** @var int $position */ + /** @var int */ private $position; - /** @var resource $fileHandle */ + /** @var resource */ private $fileHandle; /** From ad913f0100aa1d994ca10fc6ed13c491e0f2e582 Mon Sep 17 00:00:00 2001 From: Oded Arbel Date: Sun, 7 Feb 2021 11:29:01 +0200 Subject: [PATCH 03/17] write boolean value according to http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#datatype-boolean instead of just "1" for true or "" for false; --- src/Spout/Writer/ODS/Manager/WorksheetManager.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 4dfe9c8..0d6942e 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -197,7 +197,8 @@ class WorksheetManager implements WorksheetManagerInterface $data .= ''; } elseif ($cell->isBoolean()) { - $data .= ' office:value-type="boolean" calcext:value-type="boolean" office:boolean-value="' . $cell->getValue() . '">'; + $value = $cell->getValue() ? 'true' : 'false'; + $data .= ' office:value-type="boolean" calcext:value-type="boolean" office:boolean-value="' . $value . '">'; $data .= '' . $cell->getValue() . ''; $data .= ''; } elseif ($cell->isNumeric()) { From 73347517f064b3ef03126db643b7d53bd40c19cf Mon Sep 17 00:00:00 2001 From: Oded Arbel Date: Mon, 8 Feb 2021 15:52:13 +0200 Subject: [PATCH 04/17] added comment with spec link, as requested --- src/Spout/Writer/ODS/Manager/WorksheetManager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 0d6942e..169e4e6 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -197,7 +197,7 @@ class WorksheetManager implements WorksheetManagerInterface $data .= ''; } elseif ($cell->isBoolean()) { - $value = $cell->getValue() ? 'true' : 'false'; + $value = $cell->getValue() ? 'true' : 'false'; // boolean-value spec: http://docs.oasis-open.org/office/v1.2/os/OpenDocument-v1.2-os-part1.html#datatype-boolean $data .= ' office:value-type="boolean" calcext:value-type="boolean" office:boolean-value="' . $value . '">'; $data .= '' . $cell->getValue() . ''; $data .= ''; From ed9322e30921070f469a66faa7f401aa9bb13de9 Mon Sep 17 00:00:00 2001 From: jmsche Date: Fri, 29 May 2020 11:43:51 +0200 Subject: [PATCH 05/17] Shorter (relevant) diff by php-cs-fixer for Travis CI --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 9babd77..365ee59 100644 --- a/.travis.yml +++ b/.travis.yml @@ -42,7 +42,7 @@ before_script: script: - | if [[ "$WITH_CS" == "true" ]]; then - vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run + vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run --diff-format=udiff fi - | if [[ "$WITH_PHPUNIT" == "true" ]]; then From 9ab0b10a0f9343bb149571a82f128abb62e4b19a Mon Sep 17 00:00:00 2001 From: jmsche Date: Fri, 29 May 2020 11:55:08 +0200 Subject: [PATCH 06/17] Contributing: added info about code style --- CONTRIBUTING.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8fa35c5..5db0f6e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -68,7 +68,21 @@ This will add your changes on top of what's already in upstream, minimizing merg Make sure that all tests are passing before submitting a pull request. -### Step 8: Send the pull request +### Step 8: Fix code style + +Run the following command to check the code style of your changes: + +``` +vendor/bin/php-cs-fixer fix --config=.php_cs.dist --verbose --diff --dry-run --diff-format=udiff +``` + +This will print a diff of proposed code style changes. To apply these suggestions, run the following command: + +``` +vendor/bin/php-cs-fixer fix --config=.php_cs.dist +``` + +### Step 9: Send the pull request Send the pull request from your feature branch to us. Be sure to include a description that lets us know what work you did. From 0f20c99a7f8b8792840ac061c34f3450641a03cc Mon Sep 17 00:00:00 2001 From: Andrii Dembitskyi Date: Fri, 2 Oct 2020 21:28:30 +0300 Subject: [PATCH 07/17] Fix constant usage in example --- .../_pages/guides/4-symfony-stream-content-large-spreadsheet.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md b/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md index fb203ae..165a4b3 100644 --- a/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md +++ b/docs/_pages/guides/4-symfony-stream-content-large-spreadsheet.md @@ -94,7 +94,7 @@ class MyStreamController extends Controller $i++; // Flushing the buffer every N rows to stream echo'ed content. - if ($i % FLUSH_THRESHOLD === 0) { + if ($i % self::FLUSH_THRESHOLD === 0) { flush(); } } From df9d96366f775c2b059d96ec9ef66767d6599942 Mon Sep 17 00:00:00 2001 From: yiranzai Date: Wed, 27 May 2020 20:24:14 +0800 Subject: [PATCH 08/17] Fixed WriterAbstract::openToBrowser meet RFC6266 --- src/Spout/Writer/WriterAbstract.php | 33 ++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/Spout/Writer/WriterAbstract.php b/src/Spout/Writer/WriterAbstract.php index bbaa735..93f9891 100644 --- a/src/Spout/Writer/WriterAbstract.php +++ b/src/Spout/Writer/WriterAbstract.php @@ -112,7 +112,7 @@ abstract class WriterAbstract implements WriterInterface * @codeCoverageIgnore * {@inheritdoc} */ - public function openToBrowser($outputFileName) + public function openToBrowser($outputFileName, array $headers = []) { $this->outputFilePath = $this->globalFunctionsHelper->basename($outputFileName); @@ -123,9 +123,26 @@ abstract class WriterAbstract implements WriterInterface // @see https://github.com/box/spout/issues/241 $this->globalFunctionsHelper->ob_end_clean(); - // Set headers + /* + * Set headers + * + * For newer browsers such as Firefox, Chrome, Opera, Safari, etc., they all support and use `filename*` + * specified by the new standard, even if they do not automatically decode filename; it does not matter; + * and for older versions of Internet Explorer, they are not recognized `filename*`, will automatically + * ignore it and use the old `filename` (the only minor flaw is that there must be an English suffix name). + * In this way, the multi-browser multi-language compatibility problem is perfectly solved, which does not + * require UA judgment and is more in line with the standard. + * + * @see https://github.com/box/spout/issues/745 + * @see https://tools.ietf.org/html/rfc6266 + * @see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Disposition + */ $this->globalFunctionsHelper->header('Content-Type: ' . static::$headerContentType); - $this->globalFunctionsHelper->header('Content-Disposition: attachment; filename="' . $this->outputFilePath . '"'); + $this->globalFunctionsHelper->header( + 'Content-Disposition: attachment; ' . + 'filename="' . rawurldecode($this->outputFilePath) . '"; ' . + 'filename*=UTF-8\'\'' . rawurldecode($this->outputFilePath) + ); /* * When forcing the download of a file over SSL,IE8 and lower browsers fail @@ -137,6 +154,16 @@ abstract class WriterAbstract implements WriterInterface $this->globalFunctionsHelper->header('Cache-Control: max-age=0'); $this->globalFunctionsHelper->header('Pragma: public'); + /* + * Set custom Headers + * Sometimes need to output or cover more headers. + * + * @see https://github.com/box/spout/issues/745 + */ + foreach ($headers as $header){ + $this->globalFunctionsHelper->header($header); + } + $this->openWriter(); $this->isWriterOpened = true; From 03e1ce438a7c4099b192240c8dec7f28ae6098cc Mon Sep 17 00:00:00 2001 From: yiranzai Date: Wed, 27 May 2020 20:41:01 +0800 Subject: [PATCH 09/17] Fixed Code Style --- src/Spout/Writer/WriterAbstract.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Spout/Writer/WriterAbstract.php b/src/Spout/Writer/WriterAbstract.php index 93f9891..6c4191a 100644 --- a/src/Spout/Writer/WriterAbstract.php +++ b/src/Spout/Writer/WriterAbstract.php @@ -160,7 +160,7 @@ abstract class WriterAbstract implements WriterInterface * * @see https://github.com/box/spout/issues/745 */ - foreach ($headers as $header){ + foreach ($headers as $header) { $this->globalFunctionsHelper->header($header); } From 91f756be0b0bf78f79f93a36ffdc240a5bf4099d Mon Sep 17 00:00:00 2001 From: yiranzai Date: Thu, 18 Mar 2021 16:13:48 +0800 Subject: [PATCH 10/17] remove custom headers --- src/Spout/Writer/WriterAbstract.php | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/src/Spout/Writer/WriterAbstract.php b/src/Spout/Writer/WriterAbstract.php index 6c4191a..36a583f 100644 --- a/src/Spout/Writer/WriterAbstract.php +++ b/src/Spout/Writer/WriterAbstract.php @@ -112,7 +112,7 @@ abstract class WriterAbstract implements WriterInterface * @codeCoverageIgnore * {@inheritdoc} */ - public function openToBrowser($outputFileName, array $headers = []) + public function openToBrowser($outputFileName) { $this->outputFilePath = $this->globalFunctionsHelper->basename($outputFileName); @@ -154,16 +154,6 @@ abstract class WriterAbstract implements WriterInterface $this->globalFunctionsHelper->header('Cache-Control: max-age=0'); $this->globalFunctionsHelper->header('Pragma: public'); - /* - * Set custom Headers - * Sometimes need to output or cover more headers. - * - * @see https://github.com/box/spout/issues/745 - */ - foreach ($headers as $header) { - $this->globalFunctionsHelper->header($header); - } - $this->openWriter(); $this->isWriterOpened = true; From 57b6e87a65cf4a8b73565139f53abb0c74e30672 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Tue, 25 Aug 2020 14:14:37 +0200 Subject: [PATCH 11/17] Begin optimize xlsx write --- src/Spout/Common/Entity/Style/EmptyStyle.php | 7 +++++++ .../Writer/Common/Manager/Style/StyleRegistry.php | 13 ++++++++++++- src/Spout/Writer/ODS/Manager/WorksheetManager.php | 11 ++++++++--- src/Spout/Writer/XLSX/Manager/WorksheetManager.php | 12 ++++++++---- 4 files changed, 35 insertions(+), 8 deletions(-) create mode 100644 src/Spout/Common/Entity/Style/EmptyStyle.php diff --git a/src/Spout/Common/Entity/Style/EmptyStyle.php b/src/Spout/Common/Entity/Style/EmptyStyle.php new file mode 100644 index 0000000..56fa034 --- /dev/null +++ b/src/Spout/Common/Entity/Style/EmptyStyle.php @@ -0,0 +1,7 @@ +serialize($style); - if (!$this->hasStyleAlreadyBeenRegistered($style)) { + if (!$this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle)) { $nextStyleId = \count($this->serializedStyleToStyleIdMappingTable); $style->setId($nextStyleId); @@ -57,6 +57,17 @@ class StyleRegistry { $serializedStyle = $this->serialize($style); + return $this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle); + } + + /** + * Returns whether the serialized style has already been registered. + * + * @param string $serializedStyle The serialized style + * @return bool + */ + protected function hasSerializedStyleAlreadyBeenRegistered(string $serializedStyle) + { // Using isset here because it is way faster than array_key_exists... return isset($this->serializedStyleToStyleIdMappingTable[$serializedStyle]); } diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 169e4e6..970cace 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -4,6 +4,7 @@ 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; @@ -157,9 +158,13 @@ class WorksheetManager implements WorksheetManagerInterface */ private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $currentCellIndex, $nextCellIndex) { - // Apply row and extra styles - $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); - $cell->setStyle($mergedCellAndRowStyle); + if ($cell->getStyle() instanceof EmptyStyle) { + $cell->setStyle($rowStyle); + } else { + $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); + $cell->setStyle($mergedCellAndRowStyle); + } + $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); $registeredStyle = $this->styleManager->registerStyle($newCellStyle); diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index 741d0aa..e49c1d8 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -4,6 +4,7 @@ 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; @@ -185,11 +186,14 @@ EOD; */ private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndexOneBased, $columnIndexZeroBased) { - // Apply row and extra styles - $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); - $cell->setStyle($mergedCellAndRowStyle); - $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + if ($cell->getStyle() instanceof EmptyStyle) { + $cell->setStyle($rowStyle); + } else { + $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); + $cell->setStyle($mergedCellAndRowStyle); + } + $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); $registeredStyle = $this->styleManager->registerStyle($newCellStyle); return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId()); From a58b340835be5f3469cca9b7060162159f8aea7c Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Tue, 25 Aug 2020 16:38:44 +0200 Subject: [PATCH 12/17] Empty style on cell --- src/Spout/Common/Entity/Cell.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Spout/Common/Entity/Cell.php b/src/Spout/Common/Entity/Cell.php index c1de389..6ffda46 100644 --- a/src/Spout/Common/Entity/Cell.php +++ b/src/Spout/Common/Entity/Cell.php @@ -2,6 +2,7 @@ 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; @@ -104,7 +105,7 @@ class Cell */ public function setStyle($style) { - $this->style = $style ?: new Style(); + $this->style = $style ?: new EmptyStyle(); } /** From 197fb9987af2b9c7dee84d63103c3505996a3c0a Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Fri, 28 Aug 2020 12:39:55 +0200 Subject: [PATCH 13/17] Register style can be skipped when already registered --- src/Spout/Common/Entity/Style/Style.php | 22 ++++++++++++++++ .../Common/Manager/Style/StyleManager.php | 18 ++++++------- .../Manager/Style/StyleManagerInterface.php | 4 +-- .../Common/Manager/Style/StyleRegistry.php | 5 ++-- .../ODS/Manager/Style/StyleRegistry.php | 4 +++ .../Writer/ODS/Manager/WorksheetManager.php | 16 +++++++++--- .../XLSX/Manager/Style/StyleRegistry.php | 4 +++ .../Writer/XLSX/Manager/WorksheetManager.php | 16 +++++++++--- .../Common/Manager/Style/StyleManagerTest.php | 26 ++++++++++++------- 9 files changed, 84 insertions(+), 31 deletions(-) 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); } } From 11d91e1740f9361ba45dc2964bceec39b43d9c28 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Mon, 22 Feb 2021 20:04:35 +0100 Subject: [PATCH 14/17] Code review changes --- src/Spout/Common/Entity/Cell.php | 3 +- src/Spout/Common/Entity/Style/EmptyStyle.php | 7 ----- src/Spout/Common/Entity/Style/Style.php | 31 ++++++++++++++++--- .../Common/Manager/Style/ManagedStyle.php | 27 ++++++++++++++++ .../Common/Manager/Style/StyleManager.php | 18 +++++------ .../Manager/Style/StyleManagerInterface.php | 4 +-- .../Common/Manager/Style/StyleRegistry.php | 3 +- .../Writer/ODS/Manager/WorksheetManager.php | 17 ++++++---- .../Writer/XLSX/Manager/WorksheetManager.php | 18 +++++++---- .../Common/Manager/OptionsManagerTest.php | 2 +- .../XLSX/Manager/SharedStringsManagerTest.php | 4 +-- tests/Spout/Writer/CSV/WriterPerfTest.php | 2 +- .../Spout/Writer/Common/Entity/SheetTest.php | 2 +- .../Common/Manager/Style/StyleManagerTest.php | 14 ++++----- .../Common/Manager/Style/StyleMergerTest.php | 2 +- .../Manager/Style/StyleRegistryTest.php | 2 +- tests/Spout/Writer/ODS/WriterPerfTest.php | 2 +- .../Spout/Writer/ODS/WriterWithStyleTest.php | 2 +- tests/Spout/Writer/XLSX/WriterPerfTest.php | 2 +- .../Spout/Writer/XLSX/WriterWithStyleTest.php | 2 +- 20 files changed, 105 insertions(+), 59 deletions(-) delete mode 100644 src/Spout/Common/Entity/Style/EmptyStyle.php create mode 100644 src/Spout/Writer/Common/Manager/Style/ManagedStyle.php diff --git a/src/Spout/Common/Entity/Cell.php b/src/Spout/Common/Entity/Cell.php index 6ffda46..c1de389 100644 --- a/src/Spout/Common/Entity/Cell.php +++ b/src/Spout/Common/Entity/Cell.php @@ -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(); } /** diff --git a/src/Spout/Common/Entity/Style/EmptyStyle.php b/src/Spout/Common/Entity/Style/EmptyStyle.php deleted file mode 100644 index 56fa034..0000000 --- a/src/Spout/Common/Entity/Style/EmptyStyle.php +++ /dev/null @@ -1,7 +0,0 @@ -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; + } } diff --git a/src/Spout/Writer/Common/Manager/Style/ManagedStyle.php b/src/Spout/Writer/Common/Manager/Style/ManagedStyle.php new file mode 100644 index 0000000..0701ac3 --- /dev/null +++ b/src/Spout/Writer/Common/Manager/Style/ManagedStyle.php @@ -0,0 +1,27 @@ +style = $style; + $this->isUpdated = $isUpdated; + } + + public function getStyle() : Style + { + return $this->style; + } + + public function isUpdated() : bool + { + return $this->isUpdated; + } +} diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManager.php b/src/Spout/Writer/Common/Manager/Style/StyleManager.php index bf87958..2f21763 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManager.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManager.php @@ -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); } } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php index 9929eeb..be77110 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|null The eventually updated style + * @return ManagedStyle|null The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell) : ?Style; + public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle; } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php index ea7b7a4..dcfa267 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php @@ -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); diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index b7c4e6e..894c82f 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -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); } diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index ae1ecbb..d68d8f3 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -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); } diff --git a/tests/Spout/Common/Manager/OptionsManagerTest.php b/tests/Spout/Common/Manager/OptionsManagerTest.php index b1cac58..d5fe5d8 100644 --- a/tests/Spout/Common/Manager/OptionsManagerTest.php +++ b/tests/Spout/Common/Manager/OptionsManagerTest.php @@ -14,7 +14,7 @@ class OptionsManagerTest extends TestCase */ protected $optionsManager; - protected function setUp(): void + protected function setUp() : void { $this->optionsManager = new class() extends OptionsManagerAbstract { protected function getSupportedOptions() diff --git a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php index 22df444..f344b35 100644 --- a/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php +++ b/tests/Spout/Reader/XLSX/Manager/SharedStringsManagerTest.php @@ -25,7 +25,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->sharedStringsManager = null; } @@ -33,7 +33,7 @@ class SharedStringsManagerTest extends TestCase /** * @return void */ - public function tearDown(): void + public function tearDown() : void { if ($this->sharedStringsManager !== null) { $this->sharedStringsManager->cleanup(); diff --git a/tests/Spout/Writer/CSV/WriterPerfTest.php b/tests/Spout/Writer/CSV/WriterPerfTest.php index 4172bf5..cbfcae0 100644 --- a/tests/Spout/Writer/CSV/WriterPerfTest.php +++ b/tests/Spout/Writer/CSV/WriterPerfTest.php @@ -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; } diff --git a/tests/Spout/Writer/Common/Entity/SheetTest.php b/tests/Spout/Writer/Common/Entity/SheetTest.php index 3c50734..e0cf8ee 100644 --- a/tests/Spout/Writer/Common/Entity/SheetTest.php +++ b/tests/Spout/Writer/Common/Entity/SheetTest.php @@ -18,7 +18,7 @@ class SheetTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->sheetManager = new SheetManager(new StringHelper()); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php index d326f9d..daf4abb 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php @@ -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()); } } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php index a91d13f..b7c1607 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleMergerTest.php @@ -18,7 +18,7 @@ class StyleMergerTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->styleMerger = new StyleMerger(); } diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php index 20a764c..ac78c9e 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleRegistryTest.php @@ -20,7 +20,7 @@ class StyleRegistryTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); $this->styleRegistry = new StyleRegistry($this->defaultStyle); diff --git a/tests/Spout/Writer/ODS/WriterPerfTest.php b/tests/Spout/Writer/ODS/WriterPerfTest.php index 40c4ee0..5917b49 100644 --- a/tests/Spout/Writer/ODS/WriterPerfTest.php +++ b/tests/Spout/Writer/ODS/WriterPerfTest.php @@ -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); diff --git a/tests/Spout/Writer/ODS/WriterWithStyleTest.php b/tests/Spout/Writer/ODS/WriterWithStyleTest.php index 669d438..75177da 100644 --- a/tests/Spout/Writer/ODS/WriterWithStyleTest.php +++ b/tests/Spout/Writer/ODS/WriterWithStyleTest.php @@ -30,7 +30,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); } diff --git a/tests/Spout/Writer/XLSX/WriterPerfTest.php b/tests/Spout/Writer/XLSX/WriterPerfTest.php index f43381f..66ff337 100644 --- a/tests/Spout/Writer/XLSX/WriterPerfTest.php +++ b/tests/Spout/Writer/XLSX/WriterPerfTest.php @@ -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); diff --git a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php index da063e8..2571df4 100644 --- a/tests/Spout/Writer/XLSX/WriterWithStyleTest.php +++ b/tests/Spout/Writer/XLSX/WriterWithStyleTest.php @@ -32,7 +32,7 @@ class WriterWithStyleTest extends TestCase /** * @return void */ - public function setUp(): void + public function setUp() : void { $this->defaultStyle = (new StyleBuilder())->build(); } From c6f596c77697adc3bd83b8f85b41c92950ab02eb Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Fri, 5 Mar 2021 14:12:20 +0100 Subject: [PATCH 15/17] New code review fixs --- src/Spout/Common/Entity/Style/Style.php | 4 ++-- .../Manager/Style/StyleManagerInterface.php | 2 +- .../Common/Manager/Style/StyleRegistry.php | 19 +++---------------- .../Writer/ODS/Manager/WorksheetManager.php | 3 ++- .../Writer/XLSX/Manager/WorksheetManager.php | 3 ++- 5 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/Spout/Common/Entity/Style/Style.php b/src/Spout/Common/Entity/Style/Style.php index f4beb97..2bacc7a 100644 --- a/src/Spout/Common/Entity/Style/Style.php +++ b/src/Spout/Common/Entity/Style/Style.php @@ -490,13 +490,13 @@ class Style return $this->isRegistered; } - public function register(int $id) : void + public function markAsRegistered(?int $id) : void { $this->setId($id); $this->isRegistered = true; } - public function unregister() : void + public function unmarkAsRegistered() : void { $this->setId(0); $this->isRegistered = false; diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php index be77110..6d01bd5 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 ManagedStyle|null The eventually updated style + * @return ManagedStyle The eventually updated style */ public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle; } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php b/src/Spout/Writer/Common/Manager/Style/StyleRegistry.php index dcfa267..6b439a7 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->register($nextStyleId); + $style->markAsRegistered($nextStyleId); $this->serializedStyleToStyleIdMappingTable[$serializedStyle] = $nextStyleId; $this->styleIdToStyleMappingTable[$nextStyleId] = $style; @@ -47,19 +47,6 @@ class StyleRegistry return $this->getStyleFromSerializedStyle($serializedStyle); } - /** - * Returns whether the given style has already been registered. - * - * @param Style $style - * @return bool - */ - protected function hasStyleAlreadyBeenRegistered(Style $style) - { - $serializedStyle = $this->serialize($style); - - return $this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle); - } - /** * Returns whether the serialized style has already been registered. * @@ -114,11 +101,11 @@ class StyleRegistry { // In order to be able to properly compare style, set static ID value and reset registration $currentId = $style->getId(); - $style->unregister(); + $style->unmarkAsRegistered(); $serializedStyle = \serialize($style); - $style->setId($currentId); + $style->markAsRegistered($currentId); return $serializedStyle; } diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 894c82f..1997e9e 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -165,7 +165,8 @@ class WorksheetManager implements WorksheetManagerInterface if ($managedStyle->isUpdated()) { $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); } else { - $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle); + $registeredStyle = $this->styleManager->registerStyle($rowStyle); + $rowStyle = $registeredStyle; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index d68d8f3..3a210d6 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -193,7 +193,8 @@ EOD; if ($managedStyle->isUpdated()) { $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); } else { - $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle); + $registeredStyle = $this->styleManager->registerStyle($rowStyle); + $rowStyle = $registeredStyle; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); From 8a17d6c71f04b4d045b564ddb89af82920e57af4 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Thu, 25 Mar 2021 08:44:08 +0100 Subject: [PATCH 16/17] Remove rowStyle reference and replace it by new RegisteredStyle class --- .../Writer/Common/Manager/RegisteredStyle.php | 34 +++++++++++++++++++ .../Writer/ODS/Manager/WorksheetManager.php | 30 ++++++++++------ .../Writer/XLSX/Manager/WorksheetManager.php | 20 ++++++----- 3 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 src/Spout/Writer/Common/Manager/RegisteredStyle.php diff --git a/src/Spout/Writer/Common/Manager/RegisteredStyle.php b/src/Spout/Writer/Common/Manager/RegisteredStyle.php new file mode 100644 index 0000000..5cf0e90 --- /dev/null +++ b/src/Spout/Writer/Common/Manager/RegisteredStyle.php @@ -0,0 +1,34 @@ +style = $style; + $this->isRowStyle = $isRowStyle; + } + + public function getStyle() : Style + { + return $this->style; + } + + public function isRowStyle() : bool + { + return $this->isRowStyle; + } +} diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 1997e9e..52f81a0 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -10,6 +10,7 @@ use Box\Spout\Common\Exception\IOException; use Box\Spout\Common\Helper\Escaper\ODS as ODSEscaper; use Box\Spout\Common\Helper\StringHelper; use Box\Spout\Writer\Common\Entity\Worksheet; +use Box\Spout\Writer\Common\Manager\RegisteredStyle; use Box\Spout\Writer\Common\Manager\Style\StyleMerger; use Box\Spout\Writer\Common\Manager\WorksheetManagerInterface; use Box\Spout\Writer\ODS\Manager\Style\StyleManager; @@ -93,7 +94,7 @@ class WorksheetManager implements WorksheetManagerInterface $escapedSheetName = $this->stringsEscaper->escape($externalSheet->getName()); $tableStyleName = 'ta' . ($externalSheet->getIndex() + 1); - $tableElement = ''; + $tableElement = ''; $tableElement .= ''; return $tableElement; @@ -104,8 +105,8 @@ class WorksheetManager implements WorksheetManagerInterface * * @param Worksheet $worksheet The worksheet to add the row to * @param Row $row The row to be added - * @throws IOException If the data cannot be written * @throws InvalidArgumentException If a cell value's type is not supported + * @throws IOException If the data cannot be written * @return void */ public function addRow(Worksheet $worksheet, Row $row) @@ -125,7 +126,13 @@ class WorksheetManager implements WorksheetManagerInterface $nextCell = isset($cells[$nextCellIndex]) ? $cells[$nextCellIndex] : null; if ($nextCell === null || $cell->getValue() !== $nextCell->getValue()) { - $data .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $currentCellIndex, $nextCellIndex); + $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); + $cellStyle = $registeredStyle->getStyle(); + if ($registeredStyle->isRowStyle()) { + $rowStyle = $cellStyle; + } + + $data .= $this->getCellXMLWithStyle($cell, $cellStyle, $currentCellIndex, $nextCellIndex); $currentCellIndex = $nextCellIndex; } @@ -146,17 +153,15 @@ class WorksheetManager implements WorksheetManagerInterface /** * Applies styles to the given style, merging the cell's style with its row's style - * Then builds and returns xml for the cell. * * @param Cell $cell * @param Style $rowStyle - * @param int $currentCellIndex - * @param int $nextCellIndex * @throws InvalidArgumentException If a cell value's type is not supported - * @return string + * @return RegisteredStyle */ - private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $currentCellIndex, $nextCellIndex) + private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { + $isRowStyle = false; if ($cell->getStyle()->isEmpty()) { $cell->setStyle($rowStyle); @@ -166,7 +171,7 @@ class WorksheetManager implements WorksheetManagerInterface $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); } else { $registeredStyle = $this->styleManager->registerStyle($rowStyle); - $rowStyle = $registeredStyle; + $isRowStyle = true; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); @@ -182,7 +187,12 @@ class WorksheetManager implements WorksheetManagerInterface $registeredStyle = $this->styleManager->registerStyle($newCellStyle); } - $styleIndex = $registeredStyle->getId() + 1; // 1-based + return new RegisteredStyle($registeredStyle, $isRowStyle); + } + + private function getCellXMLWithStyle(Cell $cell, Style $style, int $currentCellIndex, int $nextCellIndex) : string + { + $styleIndex = $style->getId() + 1; // 1-based $numTimesValueRepeated = ($nextCellIndex - $currentCellIndex); diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index 3a210d6..2108dc1 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -14,6 +14,7 @@ use Box\Spout\Writer\Common\Creator\InternalEntityFactory; use Box\Spout\Writer\Common\Entity\Options; use Box\Spout\Writer\Common\Entity\Worksheet; use Box\Spout\Writer\Common\Helper\CellHelper; +use Box\Spout\Writer\Common\Manager\RegisteredStyle; use Box\Spout\Writer\Common\Manager\RowManager; use Box\Spout\Writer\Common\Manager\Style\StyleMerger; use Box\Spout\Writer\Common\Manager\WorksheetManagerInterface; @@ -160,7 +161,12 @@ EOD; $rowXML = ''; foreach ($row->getCells() as $columnIndexZeroBased => $cell) { - $rowXML .= $this->applyStyleAndGetCellXML($cell, $rowStyle, $rowIndexOneBased, $columnIndexZeroBased); + $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); + $cellStyle = $registeredStyle->getStyle(); + if ($registeredStyle->isRowStyle()) { + $rowStyle = $cellStyle; + } + $rowXML .= $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $cellStyle->getId()); } $rowXML .= ''; @@ -173,18 +179,16 @@ EOD; /** * Applies styles to the given style, merging the cell's style with its row's style - * Then builds and returns xml for the cell. * * @param Cell $cell * @param Style $rowStyle - * @param int $rowIndexOneBased - * @param int $columnIndexZeroBased * * @throws InvalidArgumentException If the given value cannot be processed - * @return string + * @return RegisteredStyle */ - private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $rowIndexOneBased, $columnIndexZeroBased) + private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { + $isRowStyle = false; if ($cell->getStyle()->isEmpty()) { $cell->setStyle($rowStyle); @@ -194,7 +198,7 @@ EOD; $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); } else { $registeredStyle = $this->styleManager->registerStyle($rowStyle); - $rowStyle = $registeredStyle; + $isRowStyle = true; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); @@ -211,7 +215,7 @@ EOD; $registeredStyle = $this->styleManager->registerStyle($newCellStyle); } - return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId()); + return new RegisteredStyle($registeredStyle, $isRowStyle); } /** From eb84ec9364fee1442c7d26833d406420e3657d14 Mon Sep 17 00:00:00 2001 From: Antoine Lamirault Date: Tue, 30 Mar 2021 14:04:40 +0200 Subject: [PATCH 17/17] Rename ManagedStyle to PossiblyUpdatedStyle and add documentation --- .../Writer/Common/Manager/RegisteredStyle.php | 14 +++++++----- ...agedStyle.php => PossiblyUpdatedStyle.php} | 7 +++++- .../Common/Manager/Style/StyleManager.php | 12 +++++----- .../Manager/Style/StyleManagerInterface.php | 4 ++-- .../Writer/ODS/Manager/WorksheetManager.php | 22 +++++++++---------- .../Writer/XLSX/Manager/WorksheetManager.php | 22 +++++++++---------- .../Common/Manager/Style/StyleManagerTest.php | 14 ++++++------ 7 files changed, 52 insertions(+), 43 deletions(-) rename src/Spout/Writer/Common/Manager/Style/{ManagedStyle.php => PossiblyUpdatedStyle.php} (74%) diff --git a/src/Spout/Writer/Common/Manager/RegisteredStyle.php b/src/Spout/Writer/Common/Manager/RegisteredStyle.php index 5cf0e90..734c2b6 100644 --- a/src/Spout/Writer/Common/Manager/RegisteredStyle.php +++ b/src/Spout/Writer/Common/Manager/RegisteredStyle.php @@ -4,6 +4,10 @@ namespace Box\Spout\Writer\Common\Manager; use Box\Spout\Common\Entity\Style\Style; +/** + * Class RegisteredStyle + * Allow to know if this style must replace actual row style. + */ class RegisteredStyle { /** @@ -14,12 +18,12 @@ class RegisteredStyle /** * @var bool */ - private $isRowStyle; + private $isMatchingRowStyle; - public function __construct(Style $style, bool $isRowStyle) + public function __construct(Style $style, bool $isMatchingRowStyle) { $this->style = $style; - $this->isRowStyle = $isRowStyle; + $this->isMatchingRowStyle = $isMatchingRowStyle; } public function getStyle() : Style @@ -27,8 +31,8 @@ class RegisteredStyle return $this->style; } - public function isRowStyle() : bool + public function isMatchingRowStyle() : bool { - return $this->isRowStyle; + return $this->isMatchingRowStyle; } } diff --git a/src/Spout/Writer/Common/Manager/Style/ManagedStyle.php b/src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php similarity index 74% rename from src/Spout/Writer/Common/Manager/Style/ManagedStyle.php rename to src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php index 0701ac3..6ccaa29 100644 --- a/src/Spout/Writer/Common/Manager/Style/ManagedStyle.php +++ b/src/Spout/Writer/Common/Manager/Style/PossiblyUpdatedStyle.php @@ -4,7 +4,12 @@ namespace Box\Spout\Writer\Common\Manager\Style; use Box\Spout\Common\Entity\Style\Style; -class ManagedStyle +/** + * Class PossiblyUpdatedStyle + * Indicates if style is updated. + * It allow to know if style registration must be done. + */ +class PossiblyUpdatedStyle { private $style; private $isUpdated; diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManager.php b/src/Spout/Writer/Common/Manager/Style/StyleManager.php index 2f21763..e2b5ebd 100644 --- a/src/Spout/Writer/Common/Manager/Style/StyleManager.php +++ b/src/Spout/Writer/Common/Manager/Style/StyleManager.php @@ -50,9 +50,9 @@ class StyleManager implements StyleManagerInterface * Typically, set "wrap text" if a cell contains a new line. * * @param Cell $cell - * @return ManagedStyle The eventually updated style + * @return PossiblyUpdatedStyle The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle + public function applyExtraStylesIfNeeded(Cell $cell) : PossiblyUpdatedStyle { return $this->applyWrapTextIfCellContainsNewLine($cell); } @@ -67,9 +67,9 @@ class StyleManager implements StyleManagerInterface * on the Windows version of Excel... * * @param Cell $cell The cell the style should be applied to - * @return ManagedStyle The eventually updated style + * @return PossiblyUpdatedStyle The eventually updated style */ - protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : ManagedStyle + protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : PossiblyUpdatedStyle { $cellStyle = $cell->getStyle(); @@ -77,9 +77,9 @@ class StyleManager implements StyleManagerInterface if (!$cellStyle->hasSetWrapText() && $cell->isString() && \strpos($cell->getValue(), "\n") !== false) { $cellStyle->setShouldWrapText(); - return new ManagedStyle($cellStyle, true); + return new PossiblyUpdatedStyle($cellStyle, true); } - return new ManagedStyle($cellStyle, false); + return new PossiblyUpdatedStyle($cellStyle, false); } } diff --git a/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php b/src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php index 6d01bd5..6b320b1 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 ManagedStyle The eventually updated style + * @return PossiblyUpdatedStyle The eventually updated style */ - public function applyExtraStylesIfNeeded(Cell $cell) : ManagedStyle; + public function applyExtraStylesIfNeeded(Cell $cell) : PossiblyUpdatedStyle; } diff --git a/src/Spout/Writer/ODS/Manager/WorksheetManager.php b/src/Spout/Writer/ODS/Manager/WorksheetManager.php index 52f81a0..e0366d1 100644 --- a/src/Spout/Writer/ODS/Manager/WorksheetManager.php +++ b/src/Spout/Writer/ODS/Manager/WorksheetManager.php @@ -128,8 +128,8 @@ class WorksheetManager implements WorksheetManagerInterface if ($nextCell === null || $cell->getValue() !== $nextCell->getValue()) { $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); $cellStyle = $registeredStyle->getStyle(); - if ($registeredStyle->isRowStyle()) { - $rowStyle = $cellStyle; + if ($registeredStyle->isMatchingRowStyle()) { + $rowStyle = $cellStyle; // Replace actual rowStyle (possibly with null id) by registered style (with id) } $data .= $this->getCellXMLWithStyle($cell, $cellStyle, $currentCellIndex, $nextCellIndex); @@ -161,25 +161,25 @@ class WorksheetManager implements WorksheetManagerInterface */ private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { - $isRowStyle = false; + $isMatchingRowStyle = false; if ($cell->getStyle()->isEmpty()) { $cell->setStyle($rowStyle); - $managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - if ($managedStyle->isUpdated()) { - $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); + if ($possiblyUpdatedStyle->isUpdated()) { + $registeredStyle = $this->styleManager->registerStyle($possiblyUpdatedStyle->getStyle()); } else { $registeredStyle = $this->styleManager->registerStyle($rowStyle); - $isRowStyle = true; + $isMatchingRowStyle = true; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); $cell->setStyle($mergedCellAndRowStyle); - $managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - if ($managedStyle->isUpdated()) { - $newCellStyle = $managedStyle->getStyle(); + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + if ($possiblyUpdatedStyle->isUpdated()) { + $newCellStyle = $possiblyUpdatedStyle->getStyle(); } else { $newCellStyle = $mergedCellAndRowStyle; } @@ -187,7 +187,7 @@ class WorksheetManager implements WorksheetManagerInterface $registeredStyle = $this->styleManager->registerStyle($newCellStyle); } - return new RegisteredStyle($registeredStyle, $isRowStyle); + return new RegisteredStyle($registeredStyle, $isMatchingRowStyle); } private function getCellXMLWithStyle(Cell $cell, Style $style, int $currentCellIndex, int $nextCellIndex) : string diff --git a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php index 2108dc1..028bdef 100644 --- a/src/Spout/Writer/XLSX/Manager/WorksheetManager.php +++ b/src/Spout/Writer/XLSX/Manager/WorksheetManager.php @@ -163,8 +163,8 @@ EOD; foreach ($row->getCells() as $columnIndexZeroBased => $cell) { $registeredStyle = $this->applyStyleAndRegister($cell, $rowStyle); $cellStyle = $registeredStyle->getStyle(); - if ($registeredStyle->isRowStyle()) { - $rowStyle = $cellStyle; + if ($registeredStyle->isMatchingRowStyle()) { + $rowStyle = $cellStyle; // Replace actual rowStyle (possibly with null id) by registered style (with id) } $rowXML .= $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $cellStyle->getId()); } @@ -188,26 +188,26 @@ EOD; */ private function applyStyleAndRegister(Cell $cell, Style $rowStyle) : RegisteredStyle { - $isRowStyle = false; + $isMatchingRowStyle = false; if ($cell->getStyle()->isEmpty()) { $cell->setStyle($rowStyle); - $managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - if ($managedStyle->isUpdated()) { - $registeredStyle = $this->styleManager->registerStyle($managedStyle->getStyle()); + if ($possiblyUpdatedStyle->isUpdated()) { + $registeredStyle = $this->styleManager->registerStyle($possiblyUpdatedStyle->getStyle()); } else { $registeredStyle = $this->styleManager->registerStyle($rowStyle); - $isRowStyle = true; + $isMatchingRowStyle = true; } } else { $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle); $cell->setStyle($mergedCellAndRowStyle); - $managedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); + $possiblyUpdatedStyle = $this->styleManager->applyExtraStylesIfNeeded($cell); - if ($managedStyle->isUpdated()) { - $newCellStyle = $managedStyle->getStyle(); + if ($possiblyUpdatedStyle->isUpdated()) { + $newCellStyle = $possiblyUpdatedStyle->getStyle(); } else { $newCellStyle = $mergedCellAndRowStyle; } @@ -215,7 +215,7 @@ EOD; $registeredStyle = $this->styleManager->registerStyle($newCellStyle); } - return new RegisteredStyle($registeredStyle, $isRowStyle); + return new RegisteredStyle($registeredStyle, $isMatchingRowStyle); } /** diff --git a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php index daf4abb..5bbbe5c 100644 --- a/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php +++ b/tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php @@ -28,10 +28,10 @@ class StyleManagerTest extends TestCase $this->assertFalse($style->shouldWrapText()); $styleManager = $this->getStyleManager(); - $managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); - $this->assertTrue($managedStyle->isUpdated()); - $this->assertTrue($managedStyle->getStyle()->shouldWrapText()); + $this->assertTrue($possiblyUpdatedStyle->isUpdated()); + $this->assertTrue($possiblyUpdatedStyle->getStyle()->shouldWrapText()); } public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextNotNeeded() : void @@ -40,9 +40,9 @@ class StyleManagerTest extends TestCase $this->assertFalse($style->shouldWrapText()); $styleManager = $this->getStyleManager(); - $managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style)); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style)); - $this->assertFalse($managedStyle->isUpdated()); + $this->assertFalse($possiblyUpdatedStyle->isUpdated()); } public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextAlreadyApplied() : void @@ -51,8 +51,8 @@ class StyleManagerTest extends TestCase $this->assertTrue($style->shouldWrapText()); $styleManager = $this->getStyleManager(); - $managedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); + $possiblyUpdatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style)); - $this->assertFalse($managedStyle->isUpdated()); + $this->assertFalse($possiblyUpdatedStyle->isUpdated()); } }