feature: add an afterSheetDispose method to the SheetWriteHandler#413
feature: add an afterSheetDispose method to the SheetWriteHandler#413delei merged 11 commits intoapache:mainfrom
Conversation
|
Maybe you can call the afterSheetDispose method within the WriteContextImpl#initSheet method |
…o feature/sheet-dispose
The WriteContextImpl#currentSheet method has not yet started writing data to the sheet |
|
You are right. What if it is written in the finish() method? |
…o feature/sheet-dispose # Conflicts: # fastexcel-test/src/test/java/cn/idev/excel/test/core/handler/WriteHandler.java # fastexcel-test/src/test/java/cn/idev/excel/test/demo/fill/FillTest.java # fastexcel-test/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java # fastexcel/src/main/java/cn/idev/excel/util/WriteHandlerUtils.java # fastexcel/src/main/java/cn/idev/excel/write/ExcelBuilderImpl.java
There was a problem hiding this comment.
@delei When I edit this file, even though I haven't formatted it, many lines in this class are modified, and I don't know how to resolve this issue
There was a problem hiding this comment.
😊 It's fine. We have recently added the spotless plugin. Please use mvn spotless:apply to automate code formatting.
There was a problem hiding this comment.
😊 It's fine. We have recently added the
spotlessplugin. Please usemvn spotless:applyto automate code formatting.
ok，I've dealt with it
|
Hi, @wangmiscoding |
Okay, I’ll handle it later |
…o feature/sheet-dispose
I have resolved the conflict |
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new afterSheetDispose method to the SheetWriteHandler interface, allowing developers to execute custom logic after all sheet operations are completed. The feature addresses issue #401 by providing a lifecycle hook for sheet cleanup and post-processing tasks.
Key changes:
- Added
afterSheetDisposemethod toSheetWriteHandlerinterface - Integrated the new method into the execution chain and utility classes
- Added method calls in
ExcelBuilderImplfor both write and fill operations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| SheetWriteHandler.java | Adds the new afterSheetDispose default method to the interface |
| SheetHandlerExecutionChain.java | Implements chain execution for the new method |
| WriteHandlerUtils.java | Adds utility method to trigger sheet disposal handlers |
| ExcelBuilderImpl.java | Integrates calls to afterSheetDispose in addContent and fill methods |
| WriteHandler.java | Updates test handler with new method implementation and assertion updates |
| WriteTest.java | Adds test case demonstrating the new functionality |
| FillTest.java | Adds test case for the new functionality in fill operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
| import org.apache.poi.ss.usermodel.*; |
There was a problem hiding this comment.
Using wildcard imports is discouraged as it makes dependencies unclear and can lead to naming conflicts. Import specific classes instead of using '*'.
| import org.apache.poi.ss.usermodel.*; |
| import java.util.List; | ||
| import java.util.Set; | ||
| import org.apache.poi.ss.usermodel.*; | ||
| import org.apache.poi.ss.usermodel.Cell; |
There was a problem hiding this comment.
This import is redundant since Cell is already imported via the wildcard import on line 43. Remove this duplicate import.
| import org.apache.poi.ss.usermodel.Cell; |
| import java.util.Set; | ||
| import org.apache.poi.ss.usermodel.*; | ||
| import org.apache.poi.ss.usermodel.Cell; | ||
| import org.apache.poi.ss.usermodel.CellStyle; |
There was a problem hiding this comment.
This import is redundant since CellStyle is already imported via the wildcard import on line 43. Remove this duplicate import.
| import org.apache.poi.ss.usermodel.CellStyle; |
| import org.apache.poi.ss.usermodel.*; | ||
| import org.apache.poi.ss.usermodel.Cell; | ||
| import org.apache.poi.ss.usermodel.CellStyle; | ||
| import org.apache.poi.ss.usermodel.FillPatternType; |
There was a problem hiding this comment.
This import is redundant since FillPatternType is already imported via the wildcard import on line 43. Remove this duplicate import.
| import org.apache.poi.ss.usermodel.FillPatternType; |
| import org.apache.poi.ss.usermodel.Cell; | ||
| import org.apache.poi.ss.usermodel.CellStyle; | ||
| import org.apache.poi.ss.usermodel.FillPatternType; | ||
| import org.apache.poi.ss.usermodel.IndexedColors; |
There was a problem hiding this comment.
This import is redundant since IndexedColors is already imported via the wildcard import on line 43. Remove this duplicate import.
| import org.apache.poi.ss.usermodel.IndexedColors; |
| import org.apache.poi.ss.usermodel.CellStyle; | ||
| import org.apache.poi.ss.usermodel.FillPatternType; | ||
| import org.apache.poi.ss.usermodel.IndexedColors; | ||
| import org.apache.poi.ss.usermodel.Workbook; |
There was a problem hiding this comment.
This import is redundant since Workbook is already imported via the wildcard import on line 43. Remove this duplicate import.
| import org.apache.poi.ss.usermodel.Workbook; |
| public void afterSheetDispose(SheetWriteHandlerContext context) { | ||
| Sheet sheet = context.getWriteSheetHolder().getSheet(); | ||
| // 合并区域单元格 | ||
| sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 10, 2, 2)); |
There was a problem hiding this comment.
The magic numbers (1, 10, 2, 2) should be replaced with named constants or variables to improve code readability and maintainability.
| sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 10, 2, 2)); | |
| sheet.addMergedRegionUnsafe(new CellRangeAddress(MERGE_FIRST_ROW, MERGE_LAST_ROW, MERGE_FIRST_COL, MERGE_LAST_COL)); |
| @Override | ||
| public void afterSheetDispose(SheetWriteHandlerContext context) { | ||
| Sheet sheet = context.getWriteSheetHolder().getSheet(); | ||
| sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 2, 0, 1)); |
There was a problem hiding this comment.
The magic numbers (1, 2, 0, 1) should be replaced with named constants or variables to improve code readability and maintainability.
| sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 2, 0, 1)); | |
| sheet.addMergedRegionUnsafe( | |
| new CellRangeAddress( | |
| MERGE_FIRST_ROW, | |
| MERGE_LAST_ROW, | |
| MERGE_FIRST_COL, | |
| MERGE_LAST_COL | |
| ) | |
| ); |
|
If no one has other questions, this PR will merge in 24hours. |
delei
left a comment
delei
left a comment
There was a problem hiding this comment.
LGTM!
This feature will be released in the next version ：)
|
Hi, @wangmiscoding |
Sure, I'll handle it later. |
…o feature/sheet-dispose
…ure/sheet-dispose
Replace EasyExcel.write with FastExcel.write Modify two files related to template filling and regular writing tests
Purpose of the pull request
Closed: #401
What's changed?
Add an afterSheetDispose method in SheetWriteHandler, and call it in ExcelBuilderImpl#addContent and ExcelBuilderImpl#fill.
Checklist