Skip to content

add an option for RangeCopier.copyRange() also clone styles#179

Closed
xjlin0 wants to merge 4 commits intoapache:trunkfrom
xjlin0:range_copier_style_copy
Closed

add an option for RangeCopier.copyRange() also clone styles#179
xjlin0 wants to merge 4 commits intoapache:trunkfrom
xjlin0:range_copier_style_copy

Conversation

@xjlin0
Copy link

@xjlin0 xjlin0 commented Apr 30, 2020

Found out that currently RangeCopier.copyRange() only copy values and formulas, however it always pass hardcoded null styleMap to RangeCopier.cloneCellContent(), therefore no styles were copied. Not so sure why, so I added a boolean option for RangeCopier.copyRange() to copy styles.

The MR provides an additional argument on the current RangeCopier.copyRange(), while passing the boolean argument is optional. So it will be compatible with all existing codes using RangeCopier.copyRange().

myRangeCopier.copyRange(originalDataRange, destinationDataRange, true);
// the additional true enables style copying

myRangeCopier.copyRange(originalDataRange, destinationDataRange);
// works as before, copy range without copying styles

Tested in my company and the cell styles can be copied by copyRange(). Somehow we also need to include compile("org.apache.commons:commons-math3:3.0") for it to copy range with styles, probably due to local jar files including.

@centic9
Copy link
Member

centic9 commented May 1, 2020

Can you add a test-case which verifies the new option?

@xjlin0
Copy link
Author

xjlin0 commented May 1, 2020

Sure, I updated a test case. Here is the passing results: Tests passed: 15 of 15 tests - 2s 304ms.

BUILD SUCCESSFUL in 6s
22 actionable tasks: 13 executed, 9 up-to-date
3:38:39 PM: Task execution finished ' :main:test --tests "org.apache.poi.hssf.usermodel.TestHSSFRangeCopier" :ooxml:test --tests "org.apache.poi.ss.usermodel.TestXSSFRangeCopier" --continue'.

One follow up question for you @centic9 , I found out that the RangeCopier.copyRange() not only cannot copy styles but also cannot copy merged cells. So I am adding options to that function. Do you think the following signature looks good? Where could I also update the document? Thanks!

copyRange(
   CellRangeAddress tilePatternRange,
   CellRangeAddress tileDestRange,
   boolean copyStyles,
   boolean copyMergedRanges
)

Or should we just forget about the boolean option, make it always copy styles & merged ranges?

@asfgit asfgit closed this in 9bddb87 May 15, 2020
@pjfanning
Copy link
Member

Thanks - merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants