Removing rows or columns that include range edges #1449
Comments
|
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. |
|
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. |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
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 |
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.
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.
The text was updated successfully, but these errors were encountered: