Skip to content

Commit

Permalink
WIP Removing Rows or Columns that Include Edge Ranges
Browse files Browse the repository at this point in the history 
This PR will probably remain in draft status for some time. It needs updates, and I need more time to understand exactly what it is doing. It is a replacement for PR PHPOffice#2096, intended as a fix for issue PHPOffice#1449. That PR has been stuck, with a good deal of acrimony, for over 2 years. In the interim, all the parameters for the functions involved were renamed, the functions were refactored to have fewer parameters with new class properties replacing the missing parameters, special code was added for absolute references, and code was refactored putting some of the methods involved in a new class under a new method name. In addition, the original PR was not Php8-compliant (one of the sticking points regarding implementing it), and there were a great many formatting problems (another sticking point). This has made it very difficult to figure out exactly what was intended in the original PR.

This PR (a) addresses some of the issues associated with the original issue (deleting columns/rows at the end of a range passed to a formula *on the same sheet*), and (b) does not cause any existing tests to fail. Many more unit tests are needed (another sticking point); in particular, there are changes involving data validations, comments, defined names, and other elements which are not adequately tested.

Aside from those, PhpSpreadsheet does not adjust formulas on a different sheet which involve deleted rows/columns, and it should, as Excel does.

Similarly, it appears that Excel doesn't do anything extraordinary when the right/bottom part of the range is deleted, but when the left/top part is deleted, it converts the range to #REF! at least some of the time. This needs more research; if confirmed, it needs to be added to PhpSpreadsheet.
  • Loading branch information
@oleibman
oleibman committed Apr 20, 2023
1 parent aab1614 commit 99d4955
Show file tree
Hide file tree
Showing 3 changed files with 168 additions and 37 deletions.
56 changes: 53 additions & 3 deletions src/PhpSpreadsheet/CellReferenceHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ class CellReferenceHelper
*/
protected $beforeColumn;

/** @var string */
protected $beforeColumnString;

/** @var bool */
protected $beforeColumnAbsolute = false;

/** @var bool */
protected $beforeRowAbsolute = false;

/**
* @var int
*/
Expand All @@ -34,12 +43,15 @@ class CellReferenceHelper

public function __construct(string $beforeCellAddress = 'A1', int $numberOfColumns = 0, int $numberOfRows = 0)
{
$this->beforeColumnAbsolute = $beforeCellAddress[0] === '$';
$this->beforeRowAbsolute = strpos($beforeCellAddress, '$', 1) !== false;
$this->beforeCellAddress = str_replace('$', '', $beforeCellAddress);
$this->numberOfColumns = $numberOfColumns;
$this->numberOfRows = $numberOfRows;

// Get coordinate of $beforeCellAddress
[$beforeColumn, $beforeRow] = Coordinate::coordinateFromString($beforeCellAddress);
$this->beforeColumnString = $beforeColumn;
$this->beforeColumn = (int) Coordinate::columnIndexFromString($beforeColumn);
$this->beforeRow = (int) $beforeRow;
}
Expand All @@ -56,7 +68,7 @@ public function refreshRequired(string $beforeCellAddress, int $numberOfColumns,
$this->numberOfRows !== $numberOfRows;
}

public function updateCellReference(string $cellReference = 'A1', bool $includeAbsoluteReferences = false): string
public function updateCellReference(string $cellReference = 'A1', bool $includeAbsoluteReferences = false, ?bool $topLeft = null): string
{
if (Coordinate::coordinateIsRange($cellReference)) {
throw new Exception('Only single cell references may be passed to this method.');
Expand All @@ -74,8 +86,46 @@ public function updateCellReference(string $cellReference = 'A1', bool $includeA
$updateColumn = (($absoluteColumn !== '$') && $newColumnIndex >= $this->beforeColumn);
$updateRow = (($absoluteRow !== '$') && $newRowIndex >= $this->beforeRow);
} else {
$updateColumn = ($newColumnIndex >= $this->beforeColumn);
$updateRow = ($newRowIndex >= $this->beforeRow);
// A special case is removing the left/top or bottom/right edge of a range
// $topLeft is null if we aren't adjusting a range at all.
if (
$topLeft !== null
&& $this->numberOfColumns < 0
&& $newColumnIndex >= $this->beforeColumn + $this->numberOfColumns
&& $newColumnIndex <= $this->beforeColumn - 1
) {
if ($topLeft) {
$newColumnIndex = $this->beforeColumn + $this->numberOfColumns;
$newColumn = Coordinate::stringFromColumnIndex($newColumnIndex);
} else {
$newColumnIndex = $this->beforeColumn + $this->numberOfColumns - 1;
}
} elseif ($newColumnIndex >= $this->beforeColumn) {
// Create new column reference
$newColumnIndex += $this->numberOfColumns;
}
$newColumn = $absoluteColumn . Coordinate::stringFromColumnIndex($newColumnIndex);
//$updateColumn = ($newColumnIndex >= $this->beforeColumn);
$updateColumn = false;
// A special case is removing the left/top or bottom/right edge of a range
// $topLeft is null if we aren't adjusting a range at all.
if (
$topLeft !== null
&& $this->numberOfRows < 0
&& $newRowIndex >= $this->beforeRow + $this->numberOfRows
&& $newRowIndex <= $this->beforeRow - 1
) {
if ($topLeft) {
$newRowIndex = $this->beforeRow + $this->numberOfRows;
} else {
$newRowIndex = $this->beforeRow + $this->numberOfRows - 1;
}
} elseif ($newRowIndex >= $this->beforeRow) {
$newRowIndex = $newRowIndex + $this->numberOfRows;
}
$newRow = $absoluteRow . $newRowIndex;
//$updateRow = ($newRowIndex >= $this->beforeRow);
$updateRow = false;
}

// Create new column reference
Expand Down
77 changes: 43 additions & 34 deletions src/PhpSpreadsheet/ReferenceHelper.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ protected function adjustPageBreaks(Worksheet $worksheet, int $numberOfColumns,
} else {
// Otherwise update any affected breaks by inserting a new break at the appropriate point
// and removing the old affected break
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);
if ($cellAddress !== $newReference) {
$worksheet->setBreak($newReference, $value)
->setBreak($cellAddress, Worksheet::BREAK_NONE);
Expand All @@ -177,7 +177,7 @@ protected function adjustComments(Worksheet $worksheet): void
// Any comments inside a deleted range will be ignored
if ($this->cellReferenceHelper->cellAddressInDeleteRange($cellAddress) === false) {
// Otherwise build a new array of comments indexed by the adjusted cell reference
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);
$aNewComments[$newReference] = $value;
}
}
Expand All @@ -200,12 +200,14 @@ protected function adjustHyperlinks(Worksheet $worksheet, int $numberOfColumns,
: uksort($aHyperlinkCollection, [self::class, 'cellSort']);

foreach ($aHyperlinkCollection as $cellAddress => $value) {
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);
if ($this->cellReferenceHelper->cellAddressInDeleteRange($cellAddress) === true) {
$worksheet->setHyperlink($cellAddress, null);
} elseif ($cellAddress !== $newReference) {
$worksheet->setHyperlink($newReference, $value);
$worksheet->setHyperlink($cellAddress, null);
if ($newReference) {
$worksheet->setHyperlink($newReference, $value);
}
}
}
}
Expand All @@ -226,7 +228,7 @@ protected function adjustConditionalFormatting(Worksheet $worksheet, int $number

foreach ($aStyles as $cellAddress => $cfRules) {
$worksheet->removeConditionalStyles($cellAddress);
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);

foreach ($cfRules as &$cfRule) {
/** @var Conditional $cfRule */
Expand Down Expand Up @@ -264,11 +266,13 @@ protected function adjustDataValidations(Worksheet $worksheet, int $numberOfColu
: uksort($aDataValidationCollection, [self::class, 'cellSort']);

foreach ($aDataValidationCollection as $cellAddress => $dataValidation) {
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);
if ($cellAddress !== $newReference) {
$dataValidation->setSqref($newReference);
$worksheet->setDataValidation($newReference, $dataValidation);
$worksheet->setDataValidation($cellAddress, null);
if ($newReference) {
$worksheet->setDataValidation($newReference, $dataValidation);
}
}
}
}
Expand All @@ -283,8 +287,10 @@ protected function adjustMergeCells(Worksheet $worksheet): void
$aMergeCells = $worksheet->getMergeCells();
$aNewMergeCells = []; // the new array of all merge cells
foreach ($aMergeCells as $cellAddress => &$value) {
$newReference = $this->updateCellReference($cellAddress);
$aNewMergeCells[$newReference] = $newReference;
$newReference = $this->updateCellReference($cellAddress, false, null);
if ($newReference) {
$aNewMergeCells[$newReference] = $newReference;
}
}
$worksheet->setMergeCells($aNewMergeCells); // replace the merge cells array
}
Expand All @@ -303,10 +309,12 @@ protected function adjustProtectedCells(Worksheet $worksheet, int $numberOfColum
? uksort($aProtectedCells, [self::class, 'cellReverseSort'])
: uksort($aProtectedCells, [self::class, 'cellSort']);
foreach ($aProtectedCells as $cellAddress => $value) {
$newReference = $this->updateCellReference($cellAddress);
$newReference = $this->updateCellReference($cellAddress, false, null);
if ($cellAddress !== $newReference) {
$worksheet->protectCells($newReference, $value, true);
$worksheet->unprotectCells($cellAddress);
if ($newReference) {
$worksheet->protectCells($newReference, $value, true);
}
}
}
}
Expand All @@ -321,7 +329,7 @@ protected function adjustColumnDimensions(Worksheet $worksheet): void
$aColumnDimensions = array_reverse($worksheet->getColumnDimensions(), true);
if (!empty($aColumnDimensions)) {
foreach ($aColumnDimensions as $objColumnDimension) {
$newReference = $this->updateCellReference($objColumnDimension->getColumnIndex() . '1');
$newReference = $this->updateCellReference($objColumnDimension->getColumnIndex() . '1', false, null);
[$newReference] = Coordinate::coordinateFromString($newReference);
if ($objColumnDimension->getColumnIndex() !== $newReference) {
$objColumnDimension->setColumnIndex($newReference);
Expand All @@ -344,7 +352,7 @@ protected function adjustRowDimensions(Worksheet $worksheet, $beforeRow, $number
$aRowDimensions = array_reverse($worksheet->getRowDimensions(), true);
if (!empty($aRowDimensions)) {
foreach ($aRowDimensions as $objRowDimension) {
$newReference = $this->updateCellReference('A' . $objRowDimension->getRowIndex());
$newReference = $this->updateCellReference('A' . $objRowDimension->getRowIndex(), false, null);
[, $newReference] = Coordinate::coordinateFromString($newReference);
$newRoweference = (int) $newReference;
if ($objRowDimension->getRowIndex() !== $newRoweference) {
Expand Down Expand Up @@ -434,7 +442,8 @@ function ($coordinate) use ($cellCollection) {
$cell = $worksheet->getCell($coordinate);
$cellIndex = Coordinate::columnIndexFromString($cell->getColumn());

if ($cellIndex - 1 + $numberOfColumns < 0) {
// Don't update cells that are being removed
if ($numberOfColumns < 0 && $cellIndex >= $beforeColumn + $numberOfColumns && $cellIndex < $beforeColumn) {
continue;
}

Expand Down Expand Up @@ -518,28 +527,28 @@ function ($coordinate) use ($cellCollection) {
$splitCell = $worksheet->getFreezePane();
$topLeftCell = $worksheet->getTopLeftCell() ?? '';

$splitCell = $this->updateCellReference($splitCell);
$topLeftCell = $this->updateCellReference($topLeftCell);
$splitCell = $this->updateCellReference($splitCell, false, null);
$topLeftCell = $this->updateCellReference($topLeftCell, false, null);

$worksheet->freezePane($splitCell, $topLeftCell);
}

// Page setup
if ($worksheet->getPageSetup()->isPrintAreaSet()) {
$worksheet->getPageSetup()->setPrintArea(
$this->updateCellReference($worksheet->getPageSetup()->getPrintArea())
$this->updateCellReference($worksheet->getPageSetup()->getPrintArea(), false, null)
);
}

// Update worksheet: drawings
$aDrawings = $worksheet->getDrawingCollection();
foreach ($aDrawings as $objDrawing) {
$newReference = $this->updateCellReference($objDrawing->getCoordinates());
$newReference = $this->updateCellReference($objDrawing->getCoordinates(), false, null);
if ($objDrawing->getCoordinates() != $newReference) {
$objDrawing->setCoordinates($newReference);
}
if ($objDrawing->getCoordinates2() !== '') {
$newReference = $this->updateCellReference($objDrawing->getCoordinates2());
$newReference = $this->updateCellReference($objDrawing->getCoordinates2(), false, null);
if ($objDrawing->getCoordinates2() != $newReference) {
$objDrawing->setCoordinates2($newReference);
}
Expand Down Expand Up @@ -596,8 +605,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences), 2);
$modified3 = substr($this->updateCellReference('$A' . $match[3], $includeAbsoluteReferences, true), 2);
$modified4 = substr($this->updateCellReference('$A' . $match[4], $includeAbsoluteReferences, false), 2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -621,8 +630,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences), 0, -2);
$modified3 = substr($this->updateCellReference($match[3] . '$1', $includeAbsoluteReferences, true), 0, -2);
$modified4 = substr($this->updateCellReference($match[4] . '$1', $includeAbsoluteReferences, false), 0, -2);

if ($match[3] . ':' . $match[4] !== $modified3 . ':' . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -646,8 +655,8 @@ public function updateFormulaReferences(
foreach ($matches as $match) {
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3] . ':' . $match[4];
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
$modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences, true);
$modified4 = $this->updateCellReference($match[4], $includeAbsoluteReferences, false);

if ($match[3] . $match[4] !== $modified3 . $modified4) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
Expand All @@ -674,7 +683,7 @@ public function updateFormulaReferences(
$fromString = ($match[2] > '') ? $match[2] . '!' : '';
$fromString .= $match[3];

$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences);
$modified3 = $this->updateCellReference($match[3], $includeAbsoluteReferences, null);
if ($match[3] !== $modified3) {
if (($match[2] == '') || (trim($match[2], "'") == $worksheetName)) {
$toString = ($match[2] > '') ? $match[2] . '!' : '';
Expand Down Expand Up @@ -854,7 +863,7 @@ private function updateRowRangesAllWorksheets(string $formula, int $numberOfRows
*
* @return string Updated cell range
*/
private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false)
private function updateCellReference($cellReference = 'A1', bool $includeAbsoluteReferences = false, ?bool $topLeft = null)
{
// Is it in another worksheet? Will not have to update anything.
if (strpos($cellReference, '!') !== false) {
Expand All @@ -863,7 +872,7 @@ private function updateCellReference($cellReference = 'A1', bool $includeAbsolut
// Is it a range or a single cell?
if (!Coordinate::coordinateIsRange($cellReference)) {
// Single cell
return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences);
return $this->cellReferenceHelper->updateCellReference($cellReference, $includeAbsoluteReferences, $topLeft);
}

// Range
Expand Down Expand Up @@ -925,7 +934,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
$formula = $this->updateFormulaReferences($cellAddress, $beforeCellAddress, $numberOfColumns, $numberOfRows, $worksheet->getTitle(), true);
$definedName->setValue($formula);
} else {
$definedName->setValue($this->updateCellReference(ltrim($cellAddress, '='), true));
$definedName->setValue($this->updateCellReference(ltrim($cellAddress, '='), true, null));
}
}
}
Expand Down Expand Up @@ -967,14 +976,14 @@ private function updateCellRange(string $cellRange = 'A1:A1', bool $includeAbsol
for ($j = 0; $j < $jc; ++$j) {
if (ctype_alpha($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences)
$this->cellReferenceHelper->updateCellReference($range[$i][$j] . '1', $includeAbsoluteReferences, null)
)[0];
} elseif (ctype_digit($range[$i][$j])) {
$range[$i][$j] = Coordinate::coordinateFromString(
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences)
$this->cellReferenceHelper->updateCellReference('A' . $range[$i][$j], $includeAbsoluteReferences, null)
)[1];
} else {
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences);
$range[$i][$j] = $this->cellReferenceHelper->updateCellReference($range[$i][$j], $includeAbsoluteReferences, null);
}
}
}
Expand Down Expand Up @@ -1052,7 +1061,7 @@ private function adjustAutoFilter(Worksheet $worksheet, string $beforeCellAddres
}

$worksheet->setAutoFilter(
$this->updateCellReference($autoFilterRange)
$this->updateCellReference($autoFilterRange, false, null)
);
}
}
Expand Down Expand Up @@ -1131,7 +1140,7 @@ private function adjustTable(Worksheet $worksheet, string $beforeCellAddress, in
}
}

$table->setRange($this->updateCellReference($tableRange));
$table->setRange($this->updateCellReference($tableRange, false, null));
}
}
}
Expand Down
Loading

0 comments on commit 99d4955

Please sign in to comment.