Bug 57151 - [PATCH] Document CellRangeAddress
Summary: [PATCH] Document CellRangeAddress
Alias: None
Product: POI
Classification: Unclassified
Component: XSSF (show other bugs)
Version: 3.11-dev
Hardware: All All
: P2 minor (vote)
Target Milestone: ---
Assignee: POI Developers List
Depends on:
Reported: 2014-10-27 12:58 UTC by Viliam Anirud
Modified: 2014-10-30 07:51 UTC (History)
0 users

The patch (generated from SVN on the current revision) (1.34 KB, text/plain)
2014-10-27 12:58 UTC, Viliam Anirud

Note You need to log in before you can comment on or make changes to this bug.
Description Viliam Anirud 2014-10-27 12:58:01 UTC
Created attachment 32150 [details]
The patch (generated from SVN on the current revision)

In the current version of POI it is easy to generate merged cells in an XLSX document, that excel fails to read, if merged regions overlap or are of zero or negative size.

My patch is very simple, but helpful. It documents the constructor of CellRangeAddress, because to Java developers it is not obvious, that the end-index is included in the range. The common practice in Java API is that the end index is the index of the first not-included element.

Secondly, it adds the check, that lastRow (lastCol) cannot be less than firstRow (firstCol). This check may cause minor backward incompatibility, but only in cases, when the generated output was already unreadable by Excel. Similar check was already implemented for the HSSF format, in InternalSheet.java, line 447 (addMergedRegion method). So it will not cause incompatibility here, the error will just be reported earlier.

It could be helpful to add overlap check, but this will be an more in-depth patch. HSSF format, unlike XSSF, seems to forgive this error: http://apache-poi.1045710.n5.nabble.com/XSSFSheet-addMergedRegion-giving-excel-error-td3389764.html
Comment 1 Dominik Stadler 2014-10-30 07:51:31 UTC
Thanks for the patch, applied via r1635389.

I also added some more unit tests to verify the changes, would be nice if you can provide an accompanying unit-test as part of future patches.