Skip to content

Commit b31d42c

Browse files
author
MarkBaker
committed
Some Refactoring of the Ods Reader, moving all formula and address translation from Ods to Excel into a separate class to eliminate code duplication
1 parent 99f488e commit b31d42c

File tree

6 files changed

+104
-83
lines changed

6 files changed

+104
-83
lines changed

‎CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
1515

1616
- Gnumeric Reader now loads number formatting for cells.
1717
- Gnumeric Reader now correctly identifies selected worksheet.
18+
- Some Refactoring of the Ods Reader, moving all formula and address translation from Ods to Excel into a separate class to eliminate code duplication and ensure consistency.
1819

1920
### Deprecated
2021

@@ -27,6 +28,12 @@ and this project adheres to [Semantic Versioning](https://semver.org).
2728
### Fixed
2829

2930
- Fixed behaviour of XLSX font style vertical align settings
31+
- Resolved formula translations to handle separators (row and column) for array functions as well as for function argument separators; and cleanly handle nesting levels.
32+
33+
Note that this method is used when translating Excel functions between en and other locale languages, as well as when converting formulae between different spreadsheet formats (e.g. Ods to Excel).
34+
35+
Nor is this a perfect solution, as there may still be issues when function calls have array arguments that themselves contain function calls; but it's still better than the current logic.
36+
3037

3138
## 1.22.0 - 2022-02-18
3239

‎src/PhpSpreadsheet/Reader/Ods.php

Lines changed: 34 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@
66
use DOMDocument;
77
use DOMElement;
88
use DOMNode;
9-
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
109
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
1110
use PhpOffice\PhpSpreadsheet\Cell\DataType;
1211
use PhpOffice\PhpSpreadsheet\Reader\Ods\AutoFilter;
1312
use PhpOffice\PhpSpreadsheet\Reader\Ods\DefinedNames;
13+
use PhpOffice\PhpSpreadsheet\Reader\Ods\FormulaTranslator;
1414
use PhpOffice\PhpSpreadsheet\Reader\Ods\PageSettings;
1515
use PhpOffice\PhpSpreadsheet\Reader\Ods\Properties as DocumentProperties;
1616
use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner;
@@ -512,7 +512,7 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
512512
if ($hasCalculatedValue) {
513513
$type = DataType::TYPE_FORMULA;
514514
$cellDataFormula = substr($cellDataFormula, strpos($cellDataFormula, ':=') + 1);
515-
$cellDataFormula = $this->convertToExcelFormulaValue($cellDataFormula);
515+
$cellDataFormula = FormulaTranslator::convertToExcelFormulaValue($cellDataFormula);
516516
}
517517

518518
if ($cellData->hasAttributeNS($tableNs, 'number-columns-repeated')) {
@@ -568,31 +568,7 @@ public function loadIntoExisting($filename, Spreadsheet $spreadsheet)
568568
}
569569

570570
// Merged cells
571-
if (
572-
$cellData->hasAttributeNS($tableNs, 'number-columns-spanned')
573-
|| $cellData->hasAttributeNS($tableNs, 'number-rows-spanned')
574-
) {
575-
if (($type !== DataType::TYPE_NULL) || (!$this->readDataOnly)) {
576-
$columnTo = $columnID;
577-
578-
if ($cellData->hasAttributeNS($tableNs, 'number-columns-spanned')) {
579-
$columnIndex = Coordinate::columnIndexFromString($columnID);
580-
$columnIndex += (int) $cellData->getAttributeNS($tableNs, 'number-columns-spanned');
581-
$columnIndex -= 2;
582-
583-
$columnTo = Coordinate::stringFromColumnIndex($columnIndex + 1);
584-
}
585-
586-
$rowTo = $rowID;
587-
588-
if ($cellData->hasAttributeNS($tableNs, 'number-rows-spanned')) {
589-
$rowTo = $rowTo + (int) $cellData->getAttributeNS($tableNs, 'number-rows-spanned') - 1;
590-
}
591-
592-
$cellRange = $columnID . $rowID . ':' . $columnTo . $rowTo;
593-
$spreadsheet->getActiveSheet()->mergeCells($cellRange);
594-
}
595-
}
571+
$this->processMergedCells($cellData, $tableNs, $type, $columnID, $rowID, $spreadsheet);
596572

597573
++$columnID;
598574
}
@@ -735,31 +711,38 @@ private function parseRichText($is)
735711
return $value;
736712
}
737713

738-
private function convertToExcelFormulaValue(string $openOfficeFormula): string
739-
{
740-
$temp = explode('"', $openOfficeFormula);
741-
$tKey = false;
742-
foreach ($temp as &$value) {
743-
// Only replace in alternate array entries (i.e. non-quoted blocks)
744-
if ($tKey = !$tKey) {
745-
// Cell range reference in another sheet
746-
$value = preg_replace('/\[\$?([^\.]+)\.([^\.]+):\.([^\.]+)\]/miu', '$1!$2:$3', $value);
747-
// Cell reference in another sheet
748-
$value = preg_replace('/\[\$?([^\.]+)\.([^\.]+)\]/miu', '$1!$2', $value ?? '');
749-
// Cell range reference
750-
$value = preg_replace('/\[\.([^\.]+):\.([^\.]+)\]/miu', '$1:$2', $value ?? '');
751-
// Simple cell reference
752-
$value = preg_replace('/\[\.([^\.]+)\]/miu', '$1', $value ?? '');
753-
// Convert references to defined names/formulae
754-
$value = str_replace('$$', '', $value ?? '');
755-
756-
$value = Calculation::translateSeparator(';', ',', $value, $inBraces);
757-
}
758-
}
714+
private function processMergedCells(
715+
DOMElement $cellData,
716+
string $tableNs,
717+
string $type,
718+
string $columnID,
719+
int $rowID,
720+
Spreadsheet $spreadsheet
721+
): void {
722+
if (
723+
$cellData->hasAttributeNS($tableNs, 'number-columns-spanned')
724+
|| $cellData->hasAttributeNS($tableNs, 'number-rows-spanned')
725+
) {
726+
if (($type !== DataType::TYPE_NULL) || ($this->readDataOnly === false)) {
727+
$columnTo = $columnID;
728+
729+
if ($cellData->hasAttributeNS($tableNs, 'number-columns-spanned')) {
730+
$columnIndex = Coordinate::columnIndexFromString($columnID);
731+
$columnIndex += (int) $cellData->getAttributeNS($tableNs, 'number-columns-spanned');
732+
$columnIndex -= 2;
733+
734+
$columnTo = Coordinate::stringFromColumnIndex($columnIndex + 1);
735+
}
759736

760-
// Then rebuild the formula string
761-
$excelFormula = implode('"', $temp);
737+
$rowTo = $rowID;
762738

763-
return $excelFormula;
739+
if ($cellData->hasAttributeNS($tableNs, 'number-rows-spanned')) {
740+
$rowTo = $rowTo + (int) $cellData->getAttributeNS($tableNs, 'number-rows-spanned') - 1;
741+
}
742+
743+
$cellRange = $columnID . $rowID . ':' . $columnTo . $rowTo;
744+
$spreadsheet->getActiveSheet()->mergeCells($cellRange);
745+
}
746+
}
764747
}
765748
}

‎src/PhpSpreadsheet/Reader/Ods/AutoFilter.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
use DOMElement;
66
use DOMNode;
77

8-
class AutoFilter extends BaseReader
8+
class AutoFilter extends BaseLoader
99
{
1010
public function read(DOMElement $workbookData): void
1111
{
@@ -20,7 +20,7 @@ protected function readAutoFilters(DOMElement $workbookData): void
2020
foreach ($autofilters->childNodes as $autofilter) {
2121
$autofilterRange = $this->getAttributeValue($autofilter, 'target-range-address');
2222
if ($autofilterRange !== null) {
23-
$baseAddress = $this->convertToExcelAddressValue($autofilterRange);
23+
$baseAddress = FormulaTranslator::convertToExcelAddressValue($autofilterRange);
2424
$this->spreadsheet->getActiveSheet()->setAutoFilter($baseAddress);
2525
}
2626
}

‎src/PhpSpreadsheet/Reader/Ods/BaseLoader.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
namespace PhpOffice\PhpSpreadsheet\Reader\Ods;
4+
5+
use DOMElement;
6+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
7+
8+
abstract class BaseLoader
9+
{
10+
/**
11+
* @var Spreadsheet
12+
*/
13+
protected $spreadsheet;
14+
15+
/**
16+
* @var string
17+
*/
18+
protected $tableNs;
19+
20+
public function __construct(Spreadsheet $spreadsheet, string $tableNs)
21+
{
22+
$this->spreadsheet = $spreadsheet;
23+
$this->tableNs = $tableNs;
24+
}
25+
26+
abstract public function read(DOMElement $workbookData): void;
27+
}

‎src/PhpSpreadsheet/Reader/Ods/DefinedNames.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use PhpOffice\PhpSpreadsheet\DefinedName;
77
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
88

9-
class DefinedNames extends BaseReader
9+
class DefinedNames extends BaseLoader
1010
{
1111
public function read(DOMElement $workbookData): void
1212
{
@@ -25,8 +25,8 @@ protected function readDefinedRanges(DOMElement $workbookData): void
2525
$baseAddress = $definedNameElement->getAttributeNS($this->tableNs, 'base-cell-address');
2626
$range = $definedNameElement->getAttributeNS($this->tableNs, 'cell-range-address');
2727

28-
$baseAddress = $this->convertToExcelAddressValue($baseAddress);
29-
$range = $this->convertToExcelAddressValue($range);
28+
$baseAddress = FormulaTranslator::convertToExcelAddressValue($baseAddress);
29+
$range = FormulaTranslator::convertToExcelAddressValue($range);
3030

3131
$this->addDefinedName($baseAddress, $definedName, $range);
3232
}
@@ -43,9 +43,9 @@ protected function readDefinedExpressions(DOMElement $workbookData): void
4343
$baseAddress = $definedNameElement->getAttributeNS($this->tableNs, 'base-cell-address');
4444
$expression = $definedNameElement->getAttributeNS($this->tableNs, 'expression');
4545

46-
$baseAddress = $this->convertToExcelAddressValue($baseAddress);
46+
$baseAddress = FormulaTranslator::convertToExcelAddressValue($baseAddress);
4747
$expression = substr($expression, strpos($expression, ':=') + 1);
48-
$expression = $this->convertToExcelFormulaValue($expression);
48+
$expression = FormulaTranslator::convertToExcelFormulaValue($expression);
4949

5050
$this->addDefinedName($baseAddress, $definedName, $expression);
5151
}

‎src/PhpSpreadsheet/Reader/Ods/BaseReader.php renamed to ‎src/PhpSpreadsheet/Reader/Ods/FormulaTranslator.php

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,11 @@
22

33
namespace PhpOffice\PhpSpreadsheet\Reader\Ods;
44

5-
use DOMElement;
65
use PhpOffice\PhpSpreadsheet\Calculation\Calculation;
7-
use PhpOffice\PhpSpreadsheet\Spreadsheet;
86

9-
abstract class BaseReader
7+
class FormulaTranslator
108
{
11-
/**
12-
* @var Spreadsheet
13-
*/
14-
protected $spreadsheet;
15-
16-
/**
17-
* @var string
18-
*/
19-
protected $tableNs;
20-
21-
public function __construct(Spreadsheet $spreadsheet, string $tableNs)
22-
{
23-
$this->spreadsheet = $spreadsheet;
24-
$this->tableNs = $tableNs;
25-
}
26-
27-
abstract public function read(DOMElement $workbookData): void;
28-
29-
protected function convertToExcelAddressValue(string $openOfficeAddress): string
9+
public static function convertToExcelAddressValue(string $openOfficeAddress): string
3010
{
3111
$excelAddress = $openOfficeAddress;
3212

@@ -46,13 +26,16 @@ protected function convertToExcelAddressValue(string $openOfficeAddress): string
4626
return $excelAddress ?? '';
4727
}
4828

49-
protected function convertToExcelFormulaValue(string $openOfficeFormula): string
29+
public static function convertToExcelFormulaValue(string $openOfficeFormula): string
5030
{
51-
$temp = explode('"', $openOfficeFormula);
31+
$temp = explode(Calculation::FORMULA_STRING_QUOTE, $openOfficeFormula);
5232
$tKey = false;
33+
$inMatrixBracesLevel = 0;
34+
$inFunctionBracesLevel = 0;
5335
foreach ($temp as &$value) {
5436
// @var string $value
5537
// Only replace in alternate array entries (i.e. non-quoted blocks)
38+
// so that conversion isn't done in string values
5639
if ($tKey = !$tKey) {
5740
// Cell range reference in another sheet
5841
$value = preg_replace('/\[\$?([^\.]+)\.([^\.]+):\.([^\.]+)\]/miu', '$1!$2:$3', $value);
@@ -65,7 +48,28 @@ protected function convertToExcelFormulaValue(string $openOfficeFormula): string
6548
// Convert references to defined names/formulae
6649
$value = str_replace('$$', '', $value ?? '');
6750

68-
$value = Calculation::translateSeparator(';', ',', $value, $inBraces);
51+
// Convert ODS function argument separators to Excel function argument separators
52+
$value = Calculation::translateSeparator(';', ',', $value, $inFunctionBracesLevel);
53+
54+
// Convert ODS matrix separators to Excel matrix separators
55+
$value = Calculation::translateSeparator(
56+
';',
57+
',',
58+
$value,
59+
$inMatrixBracesLevel,
60+
Calculation::FORMULA_OPEN_MATRIX_BRACE,
61+
Calculation::FORMULA_CLOSE_MATRIX_BRACE
62+
);
63+
$value = Calculation::translateSeparator(
64+
'|',
65+
';',
66+
$value,
67+
$inMatrixBracesLevel,
68+
Calculation::FORMULA_OPEN_MATRIX_BRACE,
69+
Calculation::FORMULA_CLOSE_MATRIX_BRACE
70+
);
71+
72+
$value = preg_replace('/COM\.MICROSOFT\./ui', '', $value);
6973
}
7074
}
7175

0 commit comments

Comments
 (0)