From 8654e805f4e1aebf0aff6feb2df68472dc9bba62 Mon Sep 17 00:00:00 2001 From: oleibman <10341515+oleibman@users.noreply.github.com> Date: Thu, 5 Dec 2024 20:14:12 -0800 Subject: [PATCH] Ods Writer Eliminate Padding at End of Row Ods Writer currently will write something like `` at the end of each row. This is not necessary. In eliminating that, I have made the code a bit more efficient and (hopefully) more readable. --- src/PhpSpreadsheet/Writer/Ods/Content.php | 59 +++++++------------ .../Ods/RepeatEmptyCellsAndRowsTest.php | 48 +++++++++++++++ tests/data/Writer/Ods/content-arrays.xml | 45 +++++++++++++- tests/data/Writer/Ods/content-empty.xml | 4 -- .../Writer/Ods/content-hidden-worksheet.xml | 2 - tests/data/Writer/Ods/content-with-data.xml | 6 -- 6 files changed, 113 insertions(+), 51 deletions(-) create mode 100644 tests/PhpSpreadsheetTests/Reader/Ods/RepeatEmptyCellsAndRowsTest.php diff --git a/src/PhpSpreadsheet/Writer/Ods/Content.php b/src/PhpSpreadsheet/Writer/Ods/Content.php index 91ac5d1d2..3b9cc8245 100644 --- a/src/PhpSpreadsheet/Writer/Ods/Content.php +++ b/src/PhpSpreadsheet/Writer/Ods/Content.php @@ -20,9 +20,6 @@ use PhpOffice\PhpSpreadsheet\Writer\Ods\Cell\Style; */ class Content extends WriterPart { - const NUMBER_COLS_REPEATED_MAX = 1024; - const NUMBER_ROWS_REPEATED_MAX = 1048576; - private Formula $formulaConvertor; /** @@ -142,7 +139,6 @@ class Content extends WriterPart sprintf('%s_%d_%d', Style::COLUMN_STYLE_PREFIX, $sheetIndex, $columnDimension->getColumnNumeric()) ); $objWriter->writeAttribute('table:default-cell-style-name', 'ce0'); -// $objWriter->writeAttribute('table:number-columns-repeated', self::NUMBER_COLS_REPEATED_MAX); $objWriter->endElement(); } $this->writeRows($objWriter, $spreadsheet->getSheet($sheetIndex), $sheetIndex); @@ -155,34 +151,33 @@ class Content extends WriterPart */ private function writeRows(XMLWriter $objWriter, Worksheet $sheet, int $sheetIndex): void { - $numberRowsRepeated = self::NUMBER_ROWS_REPEATED_MAX; - $span_row = 0; + $spanRow = 0; $rows = $sheet->getRowIterator(); foreach ($rows as $row) { - $cellIterator = $row->getCellIterator(); - --$numberRowsRepeated; - if ($cellIterator->valid()) { - $objWriter->startElement('table:table-row'); - if ($span_row) { - if ($span_row > 1) { - $objWriter->writeAttribute('table:number-rows-repeated', (string) $span_row); - } - $objWriter->startElement('table:table-cell'); - $objWriter->writeAttribute('table:number-columns-repeated', (string) self::NUMBER_COLS_REPEATED_MAX); + $cellIterator = $row->getCellIterator(iterateOnlyExistingCells: true); + $cellIterator->rewind(); + $rowStyleExists = $sheet->rowDimensionExists($row->getRowIndex()) && $sheet->getRowDimension($row->getRowIndex())->getRowHeight() > 0; + if ($cellIterator->valid() || $rowStyleExists) { + if ($spanRow) { + $objWriter->startElement('table:table-row'); + $objWriter->writeAttribute( + 'table:number-rows-repeated', + (string) $spanRow + ); $objWriter->endElement(); - $span_row = 0; - } else { - if ($sheet->rowDimensionExists($row->getRowIndex()) && $sheet->getRowDimension($row->getRowIndex())->getRowHeight() > 0) { - $objWriter->writeAttribute( - 'table:style-name', - sprintf('%s_%d_%d', Style::ROW_STYLE_PREFIX, $sheetIndex, $row->getRowIndex()) - ); - } - $this->writeCells($objWriter, $cellIterator); + $spanRow = 0; } + $objWriter->startElement('table:table-row'); + if ($rowStyleExists) { + $objWriter->writeAttribute( + 'table:style-name', + sprintf('%s_%d_%d', Style::ROW_STYLE_PREFIX, $sheetIndex, $row->getRowIndex()) + ); + } + $this->writeCells($objWriter, $cellIterator); $objWriter->endElement(); } else { - ++$span_row; + ++$spanRow; } } } @@ -192,7 +187,6 @@ class Content extends WriterPart */ private function writeCells(XMLWriter $objWriter, RowCellIterator $cells): void { - $numberColsRepeated = self::NUMBER_COLS_REPEATED_MAX; $prevColumn = -1; foreach ($cells as $cell) { /** @var Cell $cell */ @@ -293,17 +287,6 @@ class Content extends WriterPart $objWriter->endElement(); $prevColumn = $column; } - - $numberColsRepeated = $numberColsRepeated - $prevColumn - 1; - if ($numberColsRepeated > 0) { - if ($numberColsRepeated > 1) { - $objWriter->startElement('table:table-cell'); - $objWriter->writeAttribute('table:number-columns-repeated', (string) $numberColsRepeated); - $objWriter->endElement(); - } else { - $objWriter->writeElement('table:table-cell'); - } - } } /** diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/RepeatEmptyCellsAndRowsTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/RepeatEmptyCellsAndRowsTest.php new file mode 100644 index 000000000..b64f8fd1d --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/RepeatEmptyCellsAndRowsTest.php @@ -0,0 +1,48 @@ +getActiveSheet(); + $oldSheet->setCellValue('C1', 'xx'); + $oldSheet->setCellValue('G1', 'aa'); + $oldSheet->setCellValue('BB1', 'bb'); + $oldSheet->setCellValue('A6', 'aaa'); + $oldSheet->setCellValue('B7', 'bbb'); + $oldSheet->getRowDimension(10)->setRowHeight(12); + $oldSheet->setCellValue('A12', 'this is A12'); + $style = $oldSheet->getStyle('B14:D14'); + $style->getFont()->setBold(true); + $oldSheet->getCell('E15')->setValue('X'); + $oldSheet->mergeCells('E15:G16'); + $oldSheet->getCell('J15')->setValue('j15'); + $oldSheet->getCell('J16')->setValue('j16'); + $oldSheet->getCell('A19')->setValue('lastrow'); + $spreadsheet = $this->writeAndReload($spreadsheetOld, 'Ods'); + $spreadsheetOld->disconnectWorksheets(); + + $sheet = $spreadsheet->getActiveSheet(); + self::assertSame('xx', $sheet->getCell('C1')->getValue()); + self::assertSame('aa', $sheet->getCell('G1')->getValue()); + self::assertSame('bb', $sheet->getCell('BB1')->getValue()); + self::assertSame('aaa', $sheet->getCell('A6')->getValue()); + self::assertSame('bbb', $sheet->getCell('B7')->getValue()); + self::assertSame('this is A12', $sheet->getCell('A12')->getValue()); + // Read styles, including row height, not yet implemented for ODS + self::assertSame('j15', $sheet->getCell('J15')->getValue()); + self::assertSame('j16', $sheet->getCell('J16')->getValue()); + self::assertSame(['E15:G16' => 'E15:G16'], $sheet->getMergeCells()); + self::assertSame('lastrow', $sheet->getCell('A19')->getValue()); + + $spreadsheet->disconnectWorksheets(); + } +} diff --git a/tests/data/Writer/Ods/content-arrays.xml b/tests/data/Writer/Ods/content-arrays.xml index a33b7dbfc..939dbea8c 100644 --- a/tests/data/Writer/Ods/content-arrays.xml +++ b/tests/data/Writer/Ods/content-arrays.xml @@ -1,2 +1,45 @@ -11133 \ No newline at end of file + + + + + + + + + + + + + + + + + + + + +1 + + +1 + + + + +1 + + +3 + + + + +3 + + + + + + + \ No newline at end of file diff --git a/tests/data/Writer/Ods/content-empty.xml b/tests/data/Writer/Ods/content-empty.xml index 84f4c2397..f8efeb542 100644 --- a/tests/data/Writer/Ods/content-empty.xml +++ b/tests/data/Writer/Ods/content-empty.xml @@ -17,10 +17,6 @@ - - - - diff --git a/tests/data/Writer/Ods/content-hidden-worksheet.xml b/tests/data/Writer/Ods/content-hidden-worksheet.xml index 88a53257a..8f34a1963 100644 --- a/tests/data/Writer/Ods/content-hidden-worksheet.xml +++ b/tests/data/Writer/Ods/content-hidden-worksheet.xml @@ -24,7 +24,6 @@ 1 - @@ -33,7 +32,6 @@ 2 - diff --git a/tests/data/Writer/Ods/content-with-data.xml b/tests/data/Writer/Ods/content-with-data.xml index db7d75a74..911566ae0 100644 --- a/tests/data/Writer/Ods/content-with-data.xml +++ b/tests/data/Writer/Ods/content-with-data.xml @@ -92,7 +92,6 @@ Lorem ipsum - @@ -107,10 +106,6 @@ 42798.572060185 - - - - @@ -119,7 +114,6 @@ 2 -