Removing rows or columns that include range edges

[jabouillei]

This is a bug report. I have a fix for PHPExcel and confirmed that the fix is not present in PhpSpreadsheet.
PHPOffice/PHPExcel@1.8...jabouillei:patch-1

What is the expected behavior?

Removing rows or columns that overlap the edge of a range should adjust the edges of the range.
Also, formulas in column A should be adjusted even if only 5 columns are being removed starting with column G. (A formula in column A may refer to a range that includes column G.)

What is the current behavior?

The critical function for adjusting references only considers the case of adding rows or columns, not removing them. It also works if the removed rows or columns do not overlap the edge of the range.
There is also an uncommented "if" that "continue"s the loop that updates formulas in the initial columns of the sheet, so for example, formulas in column A do not get updated.

What are the steps to reproduce?

I provided a fix and have already put more time into this that I was supposed to, so I'm not providing a complete example, but here is what I've been using...

setIncludeCharts(true); $content = $objReader->load('/tmp/testremovecolumn.xlsx'); foreach ($content->getWorksheetIterator() as $worksheet) { //$worksheet->removeColumn('F',2); $worksheet->removeRow(3,3); } $objWriter = PHPExcel_IOFactory::createWriter($content, "Excel2007"); $objWriter->setPreCalculateFormulas(false); $objWriter->setIncludeCharts(true); $objWriter->save('/tmp/testremovecolumn2.xlsx'); ?>

That just requires a test file that has, for example, =SUM(C4:F7) in cell A1 and a bunch of numbers in the range C4:F7.

Which versions of PhpSpreadsheet and PHP are affected?

I'm using PHPExcel 1.8 on php 7.2.12 and php 5.6.16. I looked at the source code for PhpSpreadsheet master and see that neither problem has been addressed yet.
Someday, we'll get all our servers on php 7 and we'll probably move to PhpSpreadsheet. If the issue hasn't been addressed by then, I'll post a fix for PhpSpreadsheet master instead of just PHPExcel, but that might be a couple of years.

[jabouillei]

The problems with removing rows or columns in PHPExcel/PhpSpreadsheet are more extensive than I recognized with my previous post/patch. For example, removing all of the columns associated with a merged range of cells results in there continuing to be a merged range of cells that doesn't belong. I've updated the ReferenceHelper to return an empty string for the updated range if the range has been eliminated by a removal and updated the callers to the best of my understanding. For example, in the case of merged ranges, receiving an empty string means the merged range should be discarded. In the case of recomputing the freezePane, and empty string is the perfect value to pass to the worksheet.

I also recognized why an "if (...) { continue; }" was present that I removed in my previous patch. That "if" block does cause problems for the leading columns in the case of a removal, but it should be fixed, not removed. I've updated it to only skip the columns that are being removed.

PHPExcel-1.8.1_remove.txt

[jabouillei]

I've updated patch-1 on PHPExcel accordingly. I don't know if it is better to use 1.8 or 1.8.2 as a foundation for changes. My first commit on patch-1 was against 1.8. My update is against 1.8.2, hoping that is better.
PHPOffice/PHPExcel@1.8.2...jabouillei:patch-1

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If this is still an issue for you, please try to help by debugging it further and sharing your results.
Thank you for your contributions.

[jabouillei]

It's been over a year and I needed to update so I can do some work on number formatting. I see that the fix still hasn't been incorporated, so I updated my patch, applied it, and re-ran my unit test. The fix still works and nothing went wrong, so I'm submitting it again. It is pull request #2096

[oleibman]

PR #3528 has now been installed, addressing many of the issues in this ticket, including everything that had been proposed in closed PR #2096. Other issues in this area will now be tracked in issue #4379. Closing this ticket.