Skip to content

Commit 02abb41

Browse files
author
MarkBaker
committed
Merge branch 'master' into 2.0-Development
2 parents 53a6ab9 + 7169648 commit 02abb41

File tree

5 files changed

+104
-91
lines changed

5 files changed

+104
-91
lines changed

‎src/PhpSpreadsheet/Calculation/Calculation.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5236,8 +5236,6 @@ public function extractCellRange(&$range = 'A1', ?Worksheet $worksheet = null, $
52365236
$aReferences = Coordinate::extractAllCellReferencesInRange($range);
52375237
$range = "'" . $worksheetName . "'" . '!' . $range;
52385238
if (!isset($aReferences[1])) {
5239-
$currentCol = '';
5240-
$currentRow = 0;
52415239
// Single cell in range
52425240
sscanf($aReferences[0], '%[A-Z]%d', $currentCol, $currentRow);
52435241
if ($worksheet->cellExists($aReferences[0])) {
@@ -5248,8 +5246,6 @@ public function extractCellRange(&$range = 'A1', ?Worksheet $worksheet = null, $
52485246
} else {
52495247
// Extract cell data for all cells in the range
52505248
foreach ($aReferences as $reference) {
5251-
$currentCol = '';
5252-
$currentRow = 0;
52535249
// Extract range
52545250
sscanf($reference, '%[A-Z]%d', $currentCol, $currentRow);
52555251
if ($worksheet->cellExists($reference)) {

‎src/PhpSpreadsheet/Cell/Coordinate.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -353,9 +353,8 @@ public static function extractAllCellReferencesInRange($cellRange): array
353353
}
354354

355355
$cellList = array_merge(...$cells);
356-
$cellList = self::sortCellReferenceArray($cellList);
357356

358-
return $cellList;
357+
return self::sortCellReferenceArray($cellList);
359358
}
360359

361360
private static function processRangeSetOperators(array $operators, array $cells): array
@@ -382,9 +381,10 @@ private static function sortCellReferenceArray(array $cellList): array
382381
{
383382
// Sort the result by column and row
384383
$sortKeys = [];
385-
foreach ($cellList as $coord) {
386-
sscanf($coord, '%[A-Z]%d', $column, $row);
387-
$sortKeys[sprintf('%3s%09d', $column, $row)] = $coord;
384+
foreach ($cellList as $coordinate) {
385+
sscanf($coordinate, '%[A-Z]%d', $column, $row);
386+
$key = (--$row * 16384) + self::columnIndexFromString($column);
387+
$sortKeys[$key] = $coordinate;
388388
}
389389
ksort($sortKeys);
390390

‎src/PhpSpreadsheet/Collection/Cells.php

Lines changed: 44 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Generator;
66
use PhpOffice\PhpSpreadsheet\Cell\Cell;
7+
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
78
use PhpOffice\PhpSpreadsheet\Exception as PhpSpreadsheetException;
89
use PhpOffice\PhpSpreadsheet\Settings;
910
use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
@@ -150,41 +151,16 @@ public function getCoordinates()
150151
public function getSortedCoordinates()
151152
{
152153
$sortKeys = [];
153-
foreach ($this->getCoordinates() as $coord) {
154-
sscanf($coord, '%[A-Z]%d', $column, $row);
155-
$sortKeys[sprintf('%09d%3s', $row, $column)] = $coord;
154+
foreach ($this->getCoordinates() as $coordinate) {
155+
sscanf($coordinate, '%[A-Z]%d', $column, $row);
156+
$key = (--$row * 16384) + Coordinate::columnIndexFromString($column);
157+
$sortKeys[$key] = $coordinate;
156158
}
157159
ksort($sortKeys);
158160

159161
return array_values($sortKeys);
160162
}
161163

162-
/**
163-
* Get highest worksheet column and highest row that have cell records.
164-
*
165-
* @return array Highest column name and highest row number
166-
*/
167-
public function getHighestRowAndColumn()
168-
{
169-
// Lookup highest column and highest row
170-
$col = ['A' => '1A'];
171-
$row = [1];
172-
foreach ($this->getCoordinates() as $coord) {
173-
sscanf($coord, '%[A-Z]%d', $c, $r);
174-
$row[$r] = $r;
175-
$col[$c] = strlen($c) . $c;
176-
}
177-
178-
// Determine highest column and row
179-
$highestRow = max($row);
180-
$highestColumn = substr((string) @max($col), 1);
181-
182-
return [
183-
'row' => $highestRow,
184-
'column' => $highestColumn,
185-
];
186-
}
187-
188164
/**
189165
* Return the cell coordinate of the currently active cell object.
190166
*
@@ -219,6 +195,32 @@ public function getCurrentRow()
219195
return (int) $row;
220196
}
221197

198+
/**
199+
* Get highest worksheet column and highest row that have cell records.
200+
*
201+
* @return array Highest column name and highest row number
202+
*/
203+
public function getHighestRowAndColumn()
204+
{
205+
// Lookup highest column and highest row
206+
$columns = ['1A'];
207+
$rows = [1];
208+
foreach ($this->getCoordinates() as $coordinate) {
209+
sscanf($coordinate, '%[A-Z]%d', $column, $row);
210+
$rows[$row] = $rows[$row] ?? $row;
211+
$columns[$column] = $columns[$column] ?? strlen($column) . $column;
212+
}
213+
214+
// Determine highest column and row
215+
$highestRow = max($rows);
216+
$highestColumn = substr((string) max($columns), 1);
217+
218+
return [
219+
'row' => $highestRow,
220+
'column' => $highestColumn,
221+
];
222+
}
223+
222224
/**
223225
* Get highest worksheet column.
224226
*
@@ -239,7 +241,8 @@ public function getHighestColumn($row = null)
239241
if ($r != $row) {
240242
continue;
241243
}
242-
$maxColumn = max($maxColumn, strlen($c) . $c);
244+
$sortableColum = strlen($c) . $c;
245+
$maxColumn = $maxColumn > $sortableColum ? $maxColumn : $sortableColum;
243246
}
244247

245248
return substr($maxColumn, 1);
@@ -265,7 +268,7 @@ public function getHighestRow($column = null)
265268
if ($c != $column) {
266269
continue;
267270
}
268-
$maxRow = max($maxRow, $r);
271+
$maxRow = ($maxRow > $r) ? $maxRow : $r;
269272
}
270273

271274
return $maxRow;
@@ -314,7 +317,9 @@ public function cloneCellCollection(Worksheet $worksheet)
314317

315318
// Store new values
316319
$stored = $newCollection->cache->setMultiple($newValues);
317-
$this->destructIfNeeded($stored, $newCollection, 'Failed to copy cells in cache');
320+
if ($stored === false) {
321+
$this->destructIfNeeded($newCollection, 'Failed to copy cells in cache');
322+
}
318323

319324
return $newCollection;
320325
}
@@ -359,21 +364,21 @@ private function storeCurrentCell(): void
359364
$this->currentCell->detach();
360365

361366
$stored = $this->cache->set($this->cachePrefix . $this->currentCoordinate, $this->currentCell);
362-
$this->destructIfNeeded($stored, $this, "Failed to store cell {$this->currentCoordinate} in cache");
367+
if ($stored === false) {
368+
$this->destructIfNeeded($this, "Failed to store cell {$this->currentCoordinate} in cache");
369+
}
363370
$this->currentCellIsDirty = false;
364371
}
365372

366373
$this->currentCoordinate = null;
367374
$this->currentCell = null;
368375
}
369376

370-
private function destructIfNeeded(bool $stored, self $cells, string $message): void
377+
private function destructIfNeeded(self $cells, string $message): void
371378
{
372-
if (!$stored) {
373-
$cells->__destruct();
379+
$cells->__destruct();
374380

375-
throw new PhpSpreadsheetException($message);
376-
}
381+
throw new PhpSpreadsheetException($message);
377382
}
378383

379384
/**
@@ -413,7 +418,7 @@ public function get($cellCoordinate)
413418
$this->storeCurrentCell();
414419

415420
// Return null if requested entry doesn't exist in collection
416-
if (!$this->has($cellCoordinate)) {
421+
if ($this->has($cellCoordinate) === false) {
417422
return null;
418423
}
419424

‎tests/PhpSpreadsheetTests/Collection/CellsTest.php

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,16 +29,28 @@ public function testCollectionCell(): void
2929
self::assertSame($cell1, $collection->add('B2', $cell1), 'adding a cell should return the cell');
3030

3131
// Assert cell presence
32-
self::assertEquals(['B2'], $collection->getCoordinates(), 'cell list should contains the cell');
33-
self::assertEquals(['B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the cell');
32+
self::assertEquals(['B2'], $collection->getCoordinates(), 'cell list should contains the B2 cell');
33+
self::assertEquals(['B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the B2 cell');
3434
self::assertSame($cell1, $collection->get('B2'), 'should get exact same object');
35-
self::assertTrue($collection->has('B2'), 'cell should exists');
35+
self::assertTrue($collection->has('B2'), 'B2 cell should exists');
3636

3737
// Add a second cell
3838
$cell2 = $sheet->getCell('A1');
3939
self::assertSame($cell2, $collection->add('A1', $cell2), 'adding a second cell should return the cell');
40-
self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the cell');
41-
self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the cell');
40+
self::assertEquals(['B2', 'A1'], $collection->getCoordinates(), 'cell list should contains the second cell');
41+
self::assertEquals(['A1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the second cell');
42+
43+
// Add a third cell
44+
$cell3 = $sheet->getCell('AA1');
45+
self::assertSame($cell3, $collection->add('AA1', $cell3), 'adding a third cell should return the cell');
46+
self::assertEquals(['B2', 'A1', 'AA1'], $collection->getCoordinates(), 'cell list should contains the third cell');
47+
self::assertEquals(['A1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the third cell');
48+
49+
// Add a fourth cell
50+
$cell4 = $sheet->getCell('Z1');
51+
self::assertSame($cell4, $collection->add('Z1', $cell4), 'adding a fourth cell should return the cell');
52+
self::assertEquals(['B2', 'A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should contains the fourth cell');
53+
self::assertEquals(['A1', 'Z1', 'AA1', 'B2'], $collection->getSortedCoordinates(), 'sorted cell list contains the fourth cell');
4254

4355
// Assert collection copy
4456
$sheet2 = $spreadsheet->createSheet();
@@ -53,16 +65,16 @@ public function testCollectionCell(): void
5365
// Assert deletion
5466
$collection->delete('B2');
5567
self::assertFalse($collection->has('B2'), 'cell should have been deleted');
56-
self::assertEquals(['A1'], $collection->getCoordinates(), 'cell list should contains the cell');
68+
self::assertEquals(['A1', 'AA1', 'Z1'], $collection->getCoordinates(), 'cell list should still contains the A1,Z1 and A11 cells');
5769

5870
// Assert update
5971
$cell2 = $sheet->getCell('A1');
6072
self::assertSame($sheet->getCellCollection(), $collection);
6173
self::assertSame($cell2, $collection->update($cell2), 'should update existing cell');
6274

6375
$cell3 = $sheet->getCell('C3');
64-
self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing cell');
65-
self::assertEquals(['A1', 'C3'], $collection->getCoordinates(), 'cell list should contains the cell');
76+
self::assertSame($cell3, $collection->update($cell3), 'should silently add non-existing C3 cell');
77+
self::assertEquals(['A1', 'AA1', 'Z1', 'C3'], $collection->getCoordinates(), 'cell list should contains the C3 cell');
6678
}
6779

6880
public function testCacheLastCell(): void

‎tests/data/CellExtractAllCellReferencesInRange.php

Lines changed: 35 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@
1212
[
1313
[
1414
'B4',
15-
'B5',
16-
'B6',
1715
'D4',
16+
'B5',
1817
'D5',
18+
'B6',
1919
'D6',
2020
],
2121
'B4:B6,D4:D6',
@@ -28,89 +28,89 @@
2828
[
2929
[
3030
'B4',
31-
'B5',
32-
'B6',
3331
'C4',
34-
'C5',
35-
'C6',
3632
'D4',
33+
'B5',
34+
'C5',
3735
'D5',
36+
'B6',
37+
'C6',
3838
'D6',
3939
],
4040
'B4:D6',
4141
],
4242
[
4343
[
4444
'B4',
45-
'B5',
46-
'B6',
4745
'C4',
48-
'C5',
49-
'C6',
50-
'C7',
5146
'D4',
47+
'B5',
48+
'C5',
5249
'D5',
53-
'D6',
54-
'D7',
5550
'E5',
51+
'B6',
52+
'C6',
53+
'D6',
5654
'E6',
55+
'C7',
56+
'D7',
5757
'E7',
5858
],
5959
'B4:D6,C5:E7',
6060
],
6161
[
6262
[
6363
'C5',
64-
'C6',
6564
'D5',
65+
'C6',
6666
'D6',
6767
],
6868
'B4:D6 C5:E7',
6969
],
7070
[
7171
[
7272
'B2',
73-
'B3',
74-
'B4',
7573
'C2',
76-
'C3',
77-
'C4',
78-
'C5',
7974
'D2',
75+
'B3',
76+
'C3',
8077
'D3',
81-
'D4',
82-
'D5',
83-
'D6',
8478
'E3',
79+
'B4',
80+
'C4',
81+
'D4',
8582
'E4',
86-
'E5',
87-
'E6',
8883
'F4',
84+
'C5',
85+
'D5',
86+
'E5',
8987
'F5',
88+
'D6',
89+
'E6',
9090
'F6',
9191
],
9292
'B2:D4,C5:D5,E3:E5,D6:E6,F4:F6',
9393
],
9494
[
9595
[
9696
'B2',
97-
'B3',
98-
'B4',
9997
'C2',
100-
'C3',
101-
'C4',
102-
'C5',
10398
'D2',
99+
'B3',
100+
'C3',
104101
'D3',
105-
'D4',
106-
'D5',
107-
'D6',
108102
'E3',
103+
'B4',
104+
'C4',
105+
'D4',
109106
'E4',
110-
'E5',
111-
'E6',
112107
'F4',
108+
'C5',
109+
'D5',
110+
'E5',
113111
'F5',
112+
'D6',
113+
'E6',
114114
'F6',
115115
],
116116
'B2:D4,C3:E5,D4:F6',
@@ -124,8 +124,8 @@
124124
[
125125
[
126126
'Z2',
127-
'Z3',
128127
'AA2',
128+
'Z3',
129129
'AA3',
130130
],
131131
'Z2:AA3',

0 commit comments

Comments
 (0)