From 762300da4748836e133a9377cbff9812439ebd9c Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sat, 12 Mar 2022 20:54:48 +0100 Subject: [PATCH 1/6] Ensure that some CF style is displayed in Xls samples, even while background colour is still a problem, so that it's easy to verify that the rule is beng written correctly --- .../01_Basic_Comparisons.php | 46 ++++++++++++++++++- .../02_Text_Comparisons.php | 3 ++ .../03_Blank_Comparisons.php | 2 + .../04_Error_Comparisons.php | 2 + .../05_Date_Comparisons.php | 3 +- .../06_Duplicate_Comparisons.php | 10 ++-- .../07_Expression_Comparisons.php | 10 ++-- 7 files changed, 65 insertions(+), 11 deletions(-) diff --git a/samples/ConditionalFormatting/01_Basic_Comparisons.php b/samples/ConditionalFormatting/01_Basic_Comparisons.php index b201005b..5c5381ae 100644 --- a/samples/ConditionalFormatting/01_Basic_Comparisons.php +++ b/samples/ConditionalFormatting/01_Basic_Comparisons.php @@ -30,7 +30,8 @@ $spreadsheet->getActiveSheet() ->setCellValue('A1', 'Literal Value Comparison') ->setCellValue('A9', 'Value Comparison with Absolute Cell Reference $H$9') ->setCellValue('A17', 'Value Comparison with Relative Cell References') - ->setCellValue('A23', 'Value Comparison with Formula based on AVERAGE() ± STDEV()'); + ->setCellValue('A23', 'Value Comparison with Formula based on AVERAGE() ± STDEV()') + ->setCellValue('A30', 'Literal String Value Comparison'); $dataArray = [ [-2, -1, 0, 1, 2], @@ -45,11 +46,18 @@ $betweenDataArray = [ [4, 3, 8], ]; +$stringArray = [ + ['I'], + ['Love'], + ['PHP'], +]; + $spreadsheet->getActiveSheet() ->fromArray($dataArray, null, 'A2', true) ->fromArray($dataArray, null, 'A10', true) ->fromArray($betweenDataArray, null, 'A18', true) ->fromArray($dataArray, null, 'A24', true) + ->fromArray($stringArray, null, 'A31', true) ->setCellValue('H9', 1); // Set title row bold @@ -58,21 +66,31 @@ $spreadsheet->getActiveSheet()->getStyle('A1:E1')->getFont()->setBold(true); $spreadsheet->getActiveSheet()->getStyle('A9:E9')->getFont()->setBold(true); $spreadsheet->getActiveSheet()->getStyle('A17:E17')->getFont()->setBold(true); $spreadsheet->getActiveSheet()->getStyle('A23:E23')->getFont()->setBold(true); +$spreadsheet->getActiveSheet()->getStyle('A30:E30')->getFont()->setBold(true); // Define some styles for our Conditionals $helper->log('Define some styles for our Conditionals'); $yellowStyle = new Style(false, true); $yellowStyle->getFill() ->setFillType(Fill::FILL_SOLID) + ->getStartColor()->setARGB(Color::COLOR_YELLOW); +$yellowStyle->getFill() ->getEndColor()->setARGB(Color::COLOR_YELLOW); +$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE)); $greenStyle = new Style(false, true); $greenStyle->getFill() ->setFillType(Fill::FILL_SOLID) + ->getStartColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFill() ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); $redStyle = new Style(false, true); $redStyle->getFill() ->setFillType(Fill::FILL_SOLID) + ->getStartColor()->setARGB(Color::COLOR_RED); +$redStyle->getFill() ->getEndColor()->setARGB(Color::COLOR_RED); +$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); @@ -166,6 +184,32 @@ $cellWizard->lessThan('AVERAGE(' . $formulaRange . ')-STDEV(' . $formulaRange . ->setStyle($redStyle); $conditionalStyles[] = $cellWizard->getConditional(); +$spreadsheet->getActiveSheet() + ->getStyle($cellWizard->getCellRange()) + ->setConditionalStyles($conditionalStyles); + +// Set rules for Value Comparison with String Literal +$cellRange = 'A31:A33'; +$formulaRange = implode( + ':', + array_map( + [Coordinate::class, 'absoluteCoordinate'], + Coordinate::splitRange($cellRange)[0] + ) +); +$conditionalStyles = []; +$wizardFactory = new Wizard($cellRange); +/** @var Wizard\CellValue $cellWizard */ +$cellWizard = $wizardFactory->newRule(Wizard::CELL_VALUE); + +$cellWizard->equals('LOVE') + ->setStyle($redStyle); +$conditionalStyles[] = $cellWizard->getConditional(); + +$cellWizard->equals('PHP') + ->setStyle($greenStyle); +$conditionalStyles[] = $cellWizard->getConditional(); + $spreadsheet->getActiveSheet() ->getStyle($cellWizard->getCellRange()) ->setConditionalStyles($conditionalStyles); diff --git a/samples/ConditionalFormatting/02_Text_Comparisons.php b/samples/ConditionalFormatting/02_Text_Comparisons.php index 53aa84ad..10cd25b8 100644 --- a/samples/ConditionalFormatting/02_Text_Comparisons.php +++ b/samples/ConditionalFormatting/02_Text_Comparisons.php @@ -74,14 +74,17 @@ $yellowStyle = new Style(false, true); $yellowStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_YELLOW); +$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE)); $greenStyle = new Style(false, true); $greenStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); $redStyle = new Style(false, true); $redStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_RED); +$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); diff --git a/samples/ConditionalFormatting/03_Blank_Comparisons.php b/samples/ConditionalFormatting/03_Blank_Comparisons.php index c8584c3a..33b17ef6 100644 --- a/samples/ConditionalFormatting/03_Blank_Comparisons.php +++ b/samples/ConditionalFormatting/03_Blank_Comparisons.php @@ -46,10 +46,12 @@ $greenStyle = new Style(false, true); $greenStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); $redStyle = new Style(false, true); $redStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_RED); +$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); diff --git a/samples/ConditionalFormatting/04_Error_Comparisons.php b/samples/ConditionalFormatting/04_Error_Comparisons.php index 957569cf..1c9c7b58 100644 --- a/samples/ConditionalFormatting/04_Error_Comparisons.php +++ b/samples/ConditionalFormatting/04_Error_Comparisons.php @@ -49,10 +49,12 @@ $greenStyle = new Style(false, true); $greenStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); $redStyle = new Style(false, true); $redStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_RED); +$redStyle->getFont()->setColor(new Color(Color::COLOR_GREEN)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); diff --git a/samples/ConditionalFormatting/05_Date_Comparisons.php b/samples/ConditionalFormatting/05_Date_Comparisons.php index ef9ad405..0834939d 100644 --- a/samples/ConditionalFormatting/05_Date_Comparisons.php +++ b/samples/ConditionalFormatting/05_Date_Comparisons.php @@ -108,12 +108,11 @@ $spreadsheet->getActiveSheet()->getStyle('B1:K1')->getAlignment()->setHorizontal // Define some styles for our Conditionals $helper->log('Define some styles for our Conditionals'); - $yellowStyle = new Style(false, true); $yellowStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_YELLOW); -$yellowStyle->getNumberFormat()->setFormatCode('ddd dd-mmm-yyyy'); +$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); diff --git a/samples/ConditionalFormatting/06_Duplicate_Comparisons.php b/samples/ConditionalFormatting/06_Duplicate_Comparisons.php index 0b94bd47..cbed0eb2 100644 --- a/samples/ConditionalFormatting/06_Duplicate_Comparisons.php +++ b/samples/ConditionalFormatting/06_Duplicate_Comparisons.php @@ -51,14 +51,16 @@ $spreadsheet->getActiveSheet()->getStyle('A1:C1')->getFont()->setBold(true); // Define some styles for our Conditionals $helper->log('Define some styles for our Conditionals'); -$greenStyle = new Style(false, true); -$greenStyle->getFill() - ->setFillType(Fill::FILL_SOLID) - ->getEndColor()->setARGB(Color::COLOR_GREEN); $yellowStyle = new Style(false, true); $yellowStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_YELLOW); +$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE)); +$greenStyle = new Style(false, true); +$greenStyle->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); // Set conditional formatting rules and styles $helper->log('Define conditional formatting and set styles'); diff --git a/samples/ConditionalFormatting/07_Expression_Comparisons.php b/samples/ConditionalFormatting/07_Expression_Comparisons.php index 6ea16bcc..88d79b23 100644 --- a/samples/ConditionalFormatting/07_Expression_Comparisons.php +++ b/samples/ConditionalFormatting/07_Expression_Comparisons.php @@ -69,14 +69,16 @@ $spreadsheet->getActiveSheet()->getStyle('A25:D26')->getFont()->setBold(true); // Define some styles for our Conditionals $helper->log('Define some styles for our Conditionals'); -$greenStyle = new Style(false, true); -$greenStyle->getFill() - ->setFillType(Fill::FILL_SOLID) - ->getEndColor()->setARGB(Color::COLOR_GREEN); $yellowStyle = new Style(false, true); $yellowStyle->getFill() ->setFillType(Fill::FILL_SOLID) ->getEndColor()->setARGB(Color::COLOR_YELLOW); +$yellowStyle->getFont()->setColor(new Color(Color::COLOR_BLUE)); +$greenStyle = new Style(false, true); +$greenStyle->getFill() + ->setFillType(Fill::FILL_SOLID) + ->getEndColor()->setARGB(Color::COLOR_GREEN); +$greenStyle->getFont()->setColor(new Color(Color::COLOR_DARKRED)); $greenStyleMoney = clone $greenStyle; $greenStyleMoney->getNumberFormat()->setFormatCode(NumberFormat::FORMAT_ACCOUNTING_USD); From 7e89d3397e0435a773ef81ea21f9b137ea7f5098 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sat, 12 Mar 2022 20:55:37 +0100 Subject: [PATCH 2/6] Resolve problems with writing the CF Header for each group of rules --- src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 76 ++++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index fcf7789a..c02e3420 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -539,25 +539,32 @@ class Worksheet extends BIFFwriter $this->writeSheetProtection(); $this->writeRangeProtection(); - $arrConditionalStyles = $phpSheet->getConditionalStylesCollection(); + // Write Conditional Formatting Rules and Styles + $this->writeConditionalFormatting(); + + $this->storeEof(); + } + + private function writeConditionalFormatting(): void + { + $arrConditionalStyles = $this->phpSheet->getConditionalStylesCollection(); if (!empty($arrConditionalStyles)) { $arrConditional = []; - $cfHeaderWritten = false; // Write ConditionalFormattingTable records foreach ($arrConditionalStyles as $cellCoordinate => $conditionalStyles) { + $cfHeaderWritten = false; foreach ($conditionalStyles as $conditional) { /** @var Conditional $conditional */ if ( - $conditional->getConditionType() == Conditional::CONDITION_EXPRESSION || - $conditional->getConditionType() == Conditional::CONDITION_CELLIS + $conditional->getConditionType() === Conditional::CONDITION_EXPRESSION || + $conditional->getConditionType() === Conditional::CONDITION_CELLIS ) { // Write CFHEADER record (only if there are Conditional Styles that we are able to write) if ($cfHeaderWritten === false) { - $this->writeCFHeader(); - $cfHeaderWritten = true; + $cfHeaderWritten = $this->writeCFHeader($cellCoordinate, $conditionalStyles); } - if (!isset($arrConditional[$conditional->getHashCode()])) { + if ($cfHeaderWritten === true && !isset($arrConditional[$conditional->getHashCode()])) { // This hash code has been handled $arrConditional[$conditional->getHashCode()] = true; @@ -568,8 +575,6 @@ class Worksheet extends BIFFwriter } } } - - $this->storeEof(); } /** @@ -3127,8 +3132,10 @@ class Worksheet extends BIFFwriter /** * Write CFHeader record. + * + * @param Conditional[] $conditionalStyles */ - private function writeCFHeader(): void + private function writeCFHeader(string $cellCoordinate, array $conditionalStyles): bool { $record = 0x01B0; // Record identifier $length = 0x0016; // Bytes to follow @@ -3137,33 +3144,32 @@ class Worksheet extends BIFFwriter $numColumnMax = null; $numRowMin = null; $numRowMax = null; + $arrConditional = []; - foreach ($this->phpSheet->getConditionalStylesCollection() as $cellCoordinate => $conditionalStyles) { - foreach ($conditionalStyles as $conditional) { - if ( - $conditional->getConditionType() == Conditional::CONDITION_EXPRESSION || - $conditional->getConditionType() == Conditional::CONDITION_CELLIS - ) { - if (!in_array($conditional->getHashCode(), $arrConditional)) { - $arrConditional[] = $conditional->getHashCode(); - } - // Cells - $rangeCoordinates = Coordinate::rangeBoundaries($cellCoordinate); - if ($numColumnMin === null || ($numColumnMin > $rangeCoordinates[0][0])) { - $numColumnMin = $rangeCoordinates[0][0]; - } - if ($numColumnMax === null || ($numColumnMax < $rangeCoordinates[1][0])) { - $numColumnMax = $rangeCoordinates[1][0]; - } - if ($numRowMin === null || ($numRowMin > $rangeCoordinates[0][1])) { - $numRowMin = (int) $rangeCoordinates[0][1]; - } - if ($numRowMax === null || ($numRowMax < $rangeCoordinates[1][1])) { - $numRowMax = (int) $rangeCoordinates[1][1]; - } - } + foreach ($conditionalStyles as $conditional) { + if (!in_array($conditional->getHashCode(), $arrConditional)) { + $arrConditional[] = $conditional->getHashCode(); + } + // Cells + $rangeCoordinates = Coordinate::rangeBoundaries($cellCoordinate); + if ($numColumnMin === null || ($numColumnMin > $rangeCoordinates[0][0])) { + $numColumnMin = $rangeCoordinates[0][0]; + } + if ($numColumnMax === null || ($numColumnMax < $rangeCoordinates[1][0])) { + $numColumnMax = $rangeCoordinates[1][0]; + } + if ($numRowMin === null || ($numRowMin > $rangeCoordinates[0][1])) { + $numRowMin = (int) $rangeCoordinates[0][1]; + } + if ($numRowMax === null || ($numRowMax < $rangeCoordinates[1][1])) { + $numRowMax = (int) $rangeCoordinates[1][1]; } } + + if (count($arrConditional) === 0) { + return false; + } + $needRedraw = 1; $cellRange = pack('vvvv', $numRowMin - 1, $numRowMax - 1, $numColumnMin - 1, $numColumnMax - 1); @@ -3173,6 +3179,8 @@ class Worksheet extends BIFFwriter $data .= pack('v', 0x0001); $data .= $cellRange; $this->append($header . $data); + + return true; } private function getDataBlockProtection(Conditional $conditional): int From 9ca9d741fecd8545c39a0c3b3dfaaa00432855c5 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sun, 13 Mar 2022 03:04:37 +0100 Subject: [PATCH 3/6] Resolve saving cell references, string literals and formula as values for conditional formatting rules in the Xls file save The code is ugly as sin; but it works... I'll do some refactoring and cleaning later (once I've had some sleep, because I'm stupidly still working on this at 3am) The main remaining issue is formulae that can't be parsed in BIFF8 files; e.g. formulae that use functions that aren't available. In this case, all CF in that worksheet is corrupted, and the file errors when opened, so it is a serious issue. The ISODD()/ISEVEN example in 07_Expression_Comparisons.php uses these, so I've temporarily commented out setting that CF range until I've solved that problem. There are other limitations listed in the BIFF documentation; but they're harder to detect. I've also left a couple of debug statements in the code to display these formula errors: I'll remove them once I've resolved the issue. --- .../07_Expression_Comparisons.php | 7 +- .../Wizard/WizardAbstract.php | 2 +- src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 69 +++++++++++++++---- 3 files changed, 59 insertions(+), 19 deletions(-) diff --git a/samples/ConditionalFormatting/07_Expression_Comparisons.php b/samples/ConditionalFormatting/07_Expression_Comparisons.php index 88d79b23..43d6fe56 100644 --- a/samples/ConditionalFormatting/07_Expression_Comparisons.php +++ b/samples/ConditionalFormatting/07_Expression_Comparisons.php @@ -28,6 +28,7 @@ $helper->log('Add data'); $spreadsheet->setActiveSheetIndex(0); $spreadsheet->getActiveSheet() ->setCellValue('A1', 'Odd/Even Expression Comparison') + ->setCellValue('A4', 'Note that these functions are not available for Xls files') ->setCellValue('A15', 'Sales Grid Expression Comparison') ->setCellValue('A25', 'Sales Grid Multiple Expression Comparison'); @@ -101,9 +102,9 @@ $expressionWizard->expression('ISEVEN(A1)') ->setStyle($yellowStyle); $conditionalStyles[] = $expressionWizard->getConditional(); -$spreadsheet->getActiveSheet() - ->getStyle($expressionWizard->getCellRange()) - ->setConditionalStyles($conditionalStyles); +//$spreadsheet->getActiveSheet() +// ->getStyle($expressionWizard->getCellRange()) +// ->setConditionalStyles($conditionalStyles); // Set rules for Sales Grid Row match against Country Comparison $cellRange = 'A17:D22'; diff --git a/src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/WizardAbstract.php b/src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/WizardAbstract.php index 75d6856e..df9daab3 100644 --- a/src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/WizardAbstract.php +++ b/src/PhpSpreadsheet/Style/ConditionalFormatting/Wizard/WizardAbstract.php @@ -122,7 +122,7 @@ abstract class WizardAbstract return "{$worksheet}{$column}{$row}"; } - protected static function reverseAdjustCellRef(string $condition, string $cellRange): string + public static function reverseAdjustCellRef(string $condition, string $cellRange): string { $conditionalRange = Coordinate::splitRange(str_replace('$', '', strtoupper($cellRange))); [$referenceCell] = $conditionalRange[0]; diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index c02e3420..ecfc1a6a 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -13,6 +13,7 @@ use PhpOffice\PhpSpreadsheet\Shared\Xls; use PhpOffice\PhpSpreadsheet\Style\Border; use PhpOffice\PhpSpreadsheet\Style\Color; use PhpOffice\PhpSpreadsheet\Style\Conditional; +use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard; use PhpOffice\PhpSpreadsheet\Style\Protection; use PhpOffice\PhpSpreadsheet\Worksheet\PageSetup; use PhpOffice\PhpSpreadsheet\Worksheet\SheetView; @@ -569,7 +570,7 @@ class Worksheet extends BIFFwriter $arrConditional[$conditional->getHashCode()] = true; // Write CFRULE record - $this->writeCFRule($conditional); + $this->writeCFRule($conditional, $cellCoordinate); } } } @@ -2779,7 +2780,7 @@ class Worksheet extends BIFFwriter /** * Write CFRule Record. */ - private function writeCFRule(Conditional $conditional): void + private function writeCFRule(Conditional $conditional, string $cellRange): void { $record = 0x01B1; // Record identifier $type = null; // Type of the CF @@ -2832,21 +2833,59 @@ class Worksheet extends BIFFwriter // $szValue2 : size of the formula data for second value or formula $arrConditions = $conditional->getConditions(); $numConditions = count($arrConditions); + + $szValue1 = 0x0000; + $szValue2 = 0x0000; + $operand1 = null; + $operand2 = null; + if ($numConditions == 1) { - $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); - $szValue2 = 0x0000; - $operand1 = pack('Cv', 0x1E, $arrConditions[0]); - $operand2 = null; + if (is_int($arrConditions[0]) || is_float($arrConditions[0])) { + $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); + $operand1 = pack('Cv', 0x1E, $arrConditions[0]); + } else { + try { + $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange); + $this->parser->parse($formula1); + $formula1 = $this->parser->toReversePolish(); + $szValue1 = strlen($formula1); + } catch (PhpSpreadsheetException $e) { + var_dump("PARSER EXCEPTION: {$e->getMessage()}"); + $formula1 = null; + } + $operand1 = $formula1; + } } elseif ($numConditions == 2 && ($conditional->getOperatorType() == Conditional::OPERATOR_BETWEEN)) { - $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); - $szValue2 = ($arrConditions[1] <= 65535 ? 3 : 0x0000); - $operand1 = pack('Cv', 0x1E, $arrConditions[0]); - $operand2 = pack('Cv', 0x1E, $arrConditions[1]); - } else { - $szValue1 = 0x0000; - $szValue2 = 0x0000; - $operand1 = null; - $operand2 = null; + if (is_int($arrConditions[0]) || is_float($arrConditions[0])) { + $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); + $operand1 = pack('Cv', 0x1E, $arrConditions[0]); + } else { + try { + $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange); + $this->parser->parse($formula1); + $formula1 = $this->parser->toReversePolish(); + $szValue1 = strlen($formula1); + } catch (PhpSpreadsheetException $e) { + var_dump("PARSER EXCEPTION: {$e->getMessage()}"); + $formula1 = null; + } + $operand1 = $formula1; + } + if (is_int($arrConditions[1]) || is_float($arrConditions[1])) { + $szValue2 = ($arrConditions[1] <= 65535 ? 3 : 0x0000); + $operand2 = pack('Cv', 0x1E, $arrConditions[1]); + } else { + try { + $formula2 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[1], $cellRange); + $this->parser->parse($formula2); + $formula2 = $this->parser->toReversePolish(); + $szValue2 = strlen($formula2); + } catch (PhpSpreadsheetException $e) { + var_dump("PARSER EXCEPTION: {$e->getMessage()}"); + $formula2 = null; + } + $operand2 = $formula2; + } } // $flags : Option flags From 83161de91e24570930eebb7e266ebcccd2aff717 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sun, 13 Mar 2022 04:02:05 +0100 Subject: [PATCH 4/6] Some refactoring to extract the logic for calculating the CF operand tokens and size, to reduce duplicated code --- .../Writer/Xls/ConditionalHelper.php | 76 +++++++++++++++++++ src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 70 +++++------------ 2 files changed, 95 insertions(+), 51 deletions(-) create mode 100644 src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php diff --git a/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php b/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php new file mode 100644 index 00000000..f408692a --- /dev/null +++ b/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php @@ -0,0 +1,76 @@ +parser = $parser; + } + + /** + * @param mixed $condition + */ + public function processCondition($condition, string $cellRange): void + { + $this->condition = $condition; + $this->cellRange = $cellRange; + + if (is_int($condition) || is_float($condition)) { + $this->size = ($condition <= 65535 ? 3 : 0x0000); + $this->tokens = pack('Cv', 0x1E, $condition); + } else { + try { + $formula = Wizard\WizardAbstract::reverseAdjustCellRef((string) $condition, $cellRange); + $this->parser->parse($formula); + $this->tokens = $this->parser->toReversePolish(); + $this->size = strlen($this->tokens); + } catch (PhpSpreadsheetException $e) { + var_dump("PARSER EXCEPTION: {$e->getMessage()}"); + $this->tokens = null; + $this->size = 0; + } + } + } + + public function tokens(): ?string + { + return $this->tokens; + } + + public function size(): int + { + return $this->size; + } +} diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index ecfc1a6a..b77719bd 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -13,7 +13,6 @@ use PhpOffice\PhpSpreadsheet\Shared\Xls; use PhpOffice\PhpSpreadsheet\Style\Border; use PhpOffice\PhpSpreadsheet\Style\Color; use PhpOffice\PhpSpreadsheet\Style\Conditional; -use PhpOffice\PhpSpreadsheet\Style\ConditionalFormatting\Wizard; use PhpOffice\PhpSpreadsheet\Style\Protection; use PhpOffice\PhpSpreadsheet\Worksheet\PageSetup; use PhpOffice\PhpSpreadsheet\Worksheet\SheetView; @@ -548,6 +547,8 @@ class Worksheet extends BIFFwriter private function writeConditionalFormatting(): void { + $conditionalFormulaHelper = new ConditionalHelper($this->parser); + $arrConditionalStyles = $this->phpSheet->getConditionalStylesCollection(); if (!empty($arrConditionalStyles)) { $arrConditional = []; @@ -570,7 +571,7 @@ class Worksheet extends BIFFwriter $arrConditional[$conditional->getHashCode()] = true; // Write CFRULE record - $this->writeCFRule($conditional, $cellCoordinate); + $this->writeCFRule($conditionalFormulaHelper, $conditional, $cellCoordinate); } } } @@ -2780,8 +2781,11 @@ class Worksheet extends BIFFwriter /** * Write CFRule Record. */ - private function writeCFRule(Conditional $conditional, string $cellRange): void - { + private function writeCFRule( + ConditionalHelper $conditionalFormulaHelper, + Conditional $conditional, + string $cellRange + ): void { $record = 0x01B1; // Record identifier $type = null; // Type of the CF $operatorType = null; // Comparison operator @@ -2839,53 +2843,17 @@ class Worksheet extends BIFFwriter $operand1 = null; $operand2 = null; - if ($numConditions == 1) { - if (is_int($arrConditions[0]) || is_float($arrConditions[0])) { - $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); - $operand1 = pack('Cv', 0x1E, $arrConditions[0]); - } else { - try { - $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange); - $this->parser->parse($formula1); - $formula1 = $this->parser->toReversePolish(); - $szValue1 = strlen($formula1); - } catch (PhpSpreadsheetException $e) { - var_dump("PARSER EXCEPTION: {$e->getMessage()}"); - $formula1 = null; - } - $operand1 = $formula1; - } - } elseif ($numConditions == 2 && ($conditional->getOperatorType() == Conditional::OPERATOR_BETWEEN)) { - if (is_int($arrConditions[0]) || is_float($arrConditions[0])) { - $szValue1 = ($arrConditions[0] <= 65535 ? 3 : 0x0000); - $operand1 = pack('Cv', 0x1E, $arrConditions[0]); - } else { - try { - $formula1 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[0], $cellRange); - $this->parser->parse($formula1); - $formula1 = $this->parser->toReversePolish(); - $szValue1 = strlen($formula1); - } catch (PhpSpreadsheetException $e) { - var_dump("PARSER EXCEPTION: {$e->getMessage()}"); - $formula1 = null; - } - $operand1 = $formula1; - } - if (is_int($arrConditions[1]) || is_float($arrConditions[1])) { - $szValue2 = ($arrConditions[1] <= 65535 ? 3 : 0x0000); - $operand2 = pack('Cv', 0x1E, $arrConditions[1]); - } else { - try { - $formula2 = Wizard\WizardAbstract::reverseAdjustCellRef((string) $arrConditions[1], $cellRange); - $this->parser->parse($formula2); - $formula2 = $this->parser->toReversePolish(); - $szValue2 = strlen($formula2); - } catch (PhpSpreadsheetException $e) { - var_dump("PARSER EXCEPTION: {$e->getMessage()}"); - $formula2 = null; - } - $operand2 = $formula2; - } + if ($numConditions === 1) { + $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange); + $szValue1 = $conditionalFormulaHelper->size(); + $operand1 = $conditionalFormulaHelper->tokens(); + } elseif ($numConditions === 2 && ($conditional->getOperatorType() === Conditional::OPERATOR_BETWEEN)) { + $conditionalFormulaHelper->processCondition($arrConditions[0], $cellRange); + $szValue1 = $conditionalFormulaHelper->size(); + $operand1 = $conditionalFormulaHelper->tokens(); + $conditionalFormulaHelper->processCondition($arrConditions[1], $cellRange); + $szValue2 = $conditionalFormulaHelper->size(); + $operand2 = $conditionalFormulaHelper->tokens(); } // $flags : Option flags From a6e792082b440b073ac2d1d8c6929dd372a690bd Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sun, 13 Mar 2022 10:53:07 +0100 Subject: [PATCH 5/6] Handle the case of an invalid formula by defaulting to ptgInt + 0, which will avoid breaking the file --- .../ConditionalFormatting/07_Expression_Comparisons.php | 6 +++--- src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php | 8 ++++---- src/PhpSpreadsheet/Writer/Xls/Worksheet.php | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/samples/ConditionalFormatting/07_Expression_Comparisons.php b/samples/ConditionalFormatting/07_Expression_Comparisons.php index 43d6fe56..0f2b1a84 100644 --- a/samples/ConditionalFormatting/07_Expression_Comparisons.php +++ b/samples/ConditionalFormatting/07_Expression_Comparisons.php @@ -102,9 +102,9 @@ $expressionWizard->expression('ISEVEN(A1)') ->setStyle($yellowStyle); $conditionalStyles[] = $expressionWizard->getConditional(); -//$spreadsheet->getActiveSheet() -// ->getStyle($expressionWizard->getCellRange()) -// ->setConditionalStyles($conditionalStyles); +$spreadsheet->getActiveSheet() + ->getStyle($expressionWizard->getCellRange()) + ->setConditionalStyles($conditionalStyles); // Set rules for Sales Grid Row match against Country Comparison $cellRange = 'A17:D22'; diff --git a/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php b/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php index f408692a..0f78b8c5 100644 --- a/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php +++ b/src/PhpSpreadsheet/Writer/Xls/ConditionalHelper.php @@ -55,11 +55,11 @@ class ConditionalHelper $formula = Wizard\WizardAbstract::reverseAdjustCellRef((string) $condition, $cellRange); $this->parser->parse($formula); $this->tokens = $this->parser->toReversePolish(); - $this->size = strlen($this->tokens); + $this->size = strlen($this->tokens ?? ''); } catch (PhpSpreadsheetException $e) { - var_dump("PARSER EXCEPTION: {$e->getMessage()}"); - $this->tokens = null; - $this->size = 0; + // In the event of a parser error with a formula value, we set the expression to ptgInt + 0 + $this->tokens = pack('Cv', 0x1E, 0); + $this->size = 3; } } } diff --git a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php index b77719bd..16059e28 100644 --- a/src/PhpSpreadsheet/Writer/Xls/Worksheet.php +++ b/src/PhpSpreadsheet/Writer/Xls/Worksheet.php @@ -2787,8 +2787,8 @@ class Worksheet extends BIFFwriter string $cellRange ): void { $record = 0x01B1; // Record identifier - $type = null; // Type of the CF - $operatorType = null; // Comparison operator + $type = null; // Type of the CF + $operatorType = null; // Comparison operator if ($conditional->getConditionType() == Conditional::CONDITION_EXPRESSION) { $type = 0x02; From 7101da80a52735b11d8ae11c5339426d53fb6b42 Mon Sep 17 00:00:00 2001 From: MarkBaker Date: Sun, 13 Mar 2022 11:12:59 +0100 Subject: [PATCH 6/6] Update Change Log --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ff894ab..df24ba1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed +- Fix bug in Conditional Formatting in the Xls Writer that resulted in a broken file when there were multiple conditional ranges in a worksheet. +- Fix Conditional Formatting in the Xls Writer to work with rules that contain string literals, cell references and formulae. - Fix for setting Active Sheet to the first loaded worksheet when bookViews element isn't defined [Issue #2666](https://github.com/PHPOffice/PhpSpreadsheet/issues/2666) [PR #2669](https://github.com/PHPOffice/PhpSpreadsheet/pull/2669) - Fixed behaviour of XLSX font style vertical align settings. - Resolved formula translations to handle separators (row and column) for array functions as well as for function argument separators; and cleanly handle nesting levels.