Bug 44893 - [PATCH] autoSizeColumn doesn't consider merged regions or indenting
Summary: [PATCH] autoSizeColumn doesn't consider merged regions or indenting
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: HSSF (show other bugs)
Version: 3.0-FINAL
Hardware: All All
: P2 normal with 1 vote (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-04-28 11:46 UTC by Michael Hall
Modified: 2008-05-13 06:01 UTC (History)
0 users



Attachments
Patch to HSSFSheet.java (3.77 KB, patch)
2008-04-28 11:47 UTC, Michael Hall
Details | Diff
Added boolean flag to turn on use of merged columns in autosize (5.28 KB, patch)
2008-05-06 06:50 UTC, Michael Hall
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Michael Hall 2008-04-28 11:46:30 UTC
The HSSFSheet method autoSizeColumn() explicitly ignored text in merged cells, and doesn't take indentation into consideration when calculating the preferred width of a column.  The included patch does the following:

When it encounters a merged region in the column it will use the text of the left-most cell in that row of the merged region as the base for calculating width.  

On merged regions that do not span columns, this should always set the correct column width.  

On regions that span columns, the total width needed for the text of the merged region is divided by the number of columns the region spans, and that fraction of the width is used for the width of the current column.  The width of other columns in the merged region are not effected, so the total width of all the columns would not be properly set unless you called autoSizeColumn on every column in the merged region.  This approach can sometimes result in the combined widths of the spanned columns being greater that what is necessary to display the text in the merged region, but it is the only solution that I could think of that would provide consistent results.

If the HSSFStyle of the cell contains indentation, the width of the indentation is added to the calculated width necessary to display the text of the cell.
Comment 1 Michael Hall 2008-04-28 11:47:13 UTC
Created attachment 21871 [details]
Patch to HSSFSheet.java
Comment 2 Michael Hall 2008-05-05 06:11:54 UTC
What else do I need to do to get this patch added to the POI trunk?
Comment 3 Yegor Kozlov 2008-05-05 06:58:08 UTC
Thanks for the patch. However, Excel does ignore merged cells when auto-sizing columns. See a similar bug report #43902.
If you have a test case demonstrating it is not so, please re-open and attach a sample xls and test code.

I applied only the fix with indentation. 

Yegor
	

Comment 4 Michael Hall 2008-05-05 07:13:42 UTC
I did notice that Excel seemed to ignore merged cells also, I didn't realize this would be a barrier for POI.  Is there a possibility of implementing this feature in some way that clearly denotes that it is not the same way Excel does it?  I can't be the only one who's needed to do this with POI.

Thanks for including the indentation bits anyway, its better than nothing.
Comment 5 Yegor Kozlov 2008-05-05 07:33:36 UTC
(In reply to comment #4)
> I did notice that Excel seemed to ignore merged cells also, I didn't realize
> this would be a barrier for POI.  Is there a possibility of implementing this
> feature in some way that clearly denotes that it is not the same way Excel does
> it?  I can't be the only one who's needed to do this with POI.
> 
> Thanks for including the indentation bits anyway, its better than nothing.
> 

Well, it's not a barrier for POI. I wrote autoSizeColumn to mimic Excel and this is why I didn't apply the first part of your patch.

I think we can extend it and add autoSizeColumn(boolean useMergedCells). Default behavior is to ignore merged cells. 

This way all will be happy :)

Regards,
Yegor  
Comment 6 Michael Hall 2008-05-06 06:50:06 UTC
Created attachment 21926 [details]
Added boolean flag to turn on use of merged columns in autosize

Changed POI to create extra method signature: HSSFSheet#autoSizeColumn(short column, boolean useMergedCells)

So that the default behavior can be to ignore them, to preserve backwards compatibility, while giving developers the option to use them when desirable.
Comment 7 Michael Hall 2008-05-06 06:50:55 UTC
Reopened for new patch which provides an alternate method to auto-size using the content of merged cells.
Comment 8 Michael Hall 2008-05-09 12:12:12 UTC
Yegor,
    Will this second patch be applied to POI?  I don't want to commit my application to using a modified POI if it won't be included in future versions.
Comment 9 Yegor Kozlov 2008-05-11 02:04:17 UTC
Thanks for the patch. I applied it.

Yegor
Comment 10 Michael Hall 2008-05-13 05:36:27 UTC
Thanks Yegor.  Will this make it into the final 3.1 release, or will it have to wait until the one after?
Comment 11 Yegor Kozlov 2008-05-13 06:01:01 UTC
(In reply to comment #10)
> Thanks Yegor.  Will this make it into the final 3.1 release, or will it have to
> wait until the one after?
> 

Yes, it will go in 3.1-final.