Merge pull request #4925 from oleibman/issue943

Consistent HighestRow/Column After Row/Column Delete
This commit is contained in:
oleibman
2026-06-29 03:33:55 +00:00
committed by GitHub
3 changed files with 123 additions and 9 deletions
@@ -2443,6 +2443,18 @@ class Worksheet
if ($row < 1) {
throw new Exception('Rows to be deleted should at least start from row 1.');
}
if ($numberOfRows === 0) {
return $this;
}
if ($numberOfRows < 0) {
$newRow = max(1, $row + $numberOfRows + 1);
$numberOfRows = $row - $newRow + 1;
$row = $newRow;
}
$newHighestRow = $this->cachedHighestRow;
if ($newHighestRow >= $row) {
$newHighestRow = max($row - 1, $this->cachedHighestRow - $numberOfRows);
}
$startRow = $row;
$endRow = $startRow + $numberOfRows - 1;
$removeKeys = [];
@@ -2499,6 +2511,7 @@ class Worksheet
}
$this->rowDimensions = $holdRowDimensions;
$this->cachedHighestRow = $newHighestRow;
return $this;
}
@@ -2537,6 +2550,19 @@ class Worksheet
throw new Exception('Column references should not be numeric.');
}
$startColumnInt = Coordinate::columnIndexFromString($column);
if ($numberOfColumns === 0) {
return $this;
}
if ($numberOfColumns < 0) {
$newStartColumnInt = max(1, $startColumnInt + $numberOfColumns + 1);
$numberOfColumns = $startColumnInt - $newStartColumnInt + 1;
$startColumnInt = $newStartColumnInt;
$column = Coordinate::stringFromColumnIndex($startColumnInt);
}
$newHighestColumn = $this->cachedHighestColumn;
if ($newHighestColumn >= $startColumnInt) {
$newHighestColumn = max($startColumnInt - 1, $this->cachedHighestColumn - $numberOfColumns);
}
$endColumnInt = $startColumnInt + $numberOfColumns - 1;
$removeKeys = [];
$addKeys = [];
@@ -2587,6 +2613,8 @@ class Worksheet
$this->columnDimensions = $holdColumnDimensions;
if ($pColumnIndex > $highestColumnIndex) {
$this->cachedHighestColumn = $newHighestColumn;
return $this;
}
@@ -2596,6 +2624,7 @@ class Worksheet
$this->cellCollection->removeColumn($highestColumn);
$highestColumn = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($highestColumn) - 1);
}
$this->cachedHighestColumn = $newHighestColumn;
$this->garbageCollect();
@@ -28,12 +28,24 @@ class InsertTest extends TestCase
$sheet->insertNewRowBefore($currentRow, 1);
self::assertSame(1001, $sheet->getHighestRow());
self::assertSame(6, $sheet->getHighestDataRow());
self::assertTrue($sheet->getStyle('C3')->getFont()->getBold());
self::assertTrue(
$sheet->getStyle('C3')->getFont()->getBold()
);
self::assertSame(11, $sheet->getCell('C3')->getValue());
self::assertTrue($sheet->getStyle('C4')->getFont()->getBold());
self::assertTrue(
$sheet->getStyle('C4')->getFont()->getBold()
);
self::assertNull($sheet->getCell('C4')->getValue());
self::assertFalse($sheet->getRowDimension(1001)->getVisible());
self::assertTrue($sheet->getRowDimension(1000)->getVisible());
self::assertFalse(
$sheet->getRowDimension(1001)->getVisible()
);
self::assertTrue(
$sheet->getRowDimension(1000)->getVisible()
);
$sheet->removeRow(15, 10);
self::assertSame(991, $sheet->getHighestRow(), 'highest row decreases by 10');
$sheet->removeRow(985, 10);
self::assertSame(984, $sheet->getHighestRow(), 'delete range overlaps highest row so highest is now row before delete');
$spreadsheet->disconnectWorksheets();
}
@@ -56,12 +68,24 @@ class InsertTest extends TestCase
$sheet->insertNewColumnBefore($currentColumn, 1);
self::assertSame('ZZ', $sheet->getHighestColumn());
self::assertSame('E', $sheet->getHighestDataColumn());
self::assertTrue($sheet->getStyle('C3')->getFont()->getBold());
self::assertTrue(
$sheet->getStyle('C3')->getFont()->getBold()
);
self::assertSame(11, $sheet->getCell('C3')->getValue());
self::assertTrue($sheet->getStyle('D3')->getFont()->getBold());
self::assertTrue(
$sheet->getStyle('D3')->getFont()->getBold()
);
self::assertNull($sheet->getCell('D3')->getValue());
self::assertFalse($sheet->getColumnDimension('ZZ')->getVisible());
self::assertTrue($sheet->getColumnDimension('ZY')->getVisible());
self::assertFalse(
$sheet->getColumnDimension('ZZ')->getVisible()
);
self::assertTrue(
$sheet->getColumnDimension('ZY')->getVisible()
);
$sheet->removeColumn('G', 5);
self::assertSame('ZU', $sheet->getHighestColumn(), 'ZZ moved over 5 columns');
$sheet->removeColumn('ZR', 5);
self::assertSame('ZQ', $sheet->getHighestColumn(), 'delete range overlaps highest column so new highest is one before deleted columns');
$spreadsheet->disconnectWorksheets();
}
@@ -97,7 +121,9 @@ class InsertTest extends TestCase
]);
$sheet->getCell('XFD1')->setValue('lastcol');
$sheet->insertNewColumnBefore('D', 4);
self::assertFalse($sheet->getCellCollection()->has('XFH1'));
self::assertFalse(
$sheet->getCellCollection()->has('XFH1')
);
$spreadsheet->disconnectWorksheets();
}
}
@@ -8,6 +8,7 @@ use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
use PhpOffice\PhpSpreadsheet\Style\Color;
use PhpOffice\PhpSpreadsheet\Style\Fill;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\TestCase;
class RemoveTest extends TestCase
@@ -86,4 +87,62 @@ class RemoveTest extends TestCase
$spreadsheet->disconnectWorksheets();
}
/**
* @param array<array<int, int>> $expectedArray
*/
#[DataProvider('providerColumnEdgeCases')]
public function testColumnEdgeCases(string $start, int $num, array $expectedArray, string $expectedHighestColumn): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->fromArray([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
$sheet->removeColumn($start, $num);
self::assertSame($expectedArray, $sheet->toArray(formatData: false));
self::assertSame($expectedHighestColumn, $sheet->getHighestColumn());
$spreadsheet->disconnectWorksheets();
}
/**
* @return array<string, array{string, int, int[][], string}>
*/
public static function providerColumnEdgeCases(): array
{
return [
'remove positive cols' => ['E', 2, [[1, 2, 3, 4, 7, 8, 9, 10]], 'H'],
'remove negative cols' => ['E', -2, [[1, 2, 3, 6, 7, 8, 9, 10]], 'H'],
'remove zero cols' => ['E', 0, [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]], 'J'],
'remove cols above highest' => ['T', 2, [[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]], 'J'],
];
}
/**
* @param array<int, list<int>> $expectedArray
*/
#[DataProvider('providerRowEdgeCases')]
public function testRowEdgeCases(int $start, int $num, array $expectedArray, int $expectedHighestRow): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->fromArray([[1], [2], [3], [4], [5], [6], [7], [8], [9], [10]]);
$sheet->removeRow($start, $num);
self::assertSame($expectedArray, $sheet->toArray(formatData: false));
self::assertSame($expectedHighestRow, $sheet->getHighestRow());
$spreadsheet->disconnectWorksheets();
}
/**
* @return array<string, array{int, int, array<int, list<int>>, int}>
*/
public static function providerRowEdgeCases(): array
{
return [
'remove positive rows' => [5, 2, [[1], [2], [3], [4], [7], [8], [9], [10]], 8],
'remove negative rows' => [5, -2, [[1], [2], [3], [6], [7], [8], [9], [10]], 8],
'remove zero rows' => [5, 0, [[1], [2], [3], [4], [5], [6], [7], [8], [9], [10]], 10],
'remove rows above highest' => [20, 2, [[1], [2], [3], [4], [5], [6], [7], [8], [9], [10]], 10],
];
}
}