Skip to content

feature: add an afterSheetDispose method to the SheetWriteHandler #413

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
delei merged 11 commits into from
Aug 26, 2025
Merged

feature: add an afterSheetDispose method to the SheetWriteHandler #413

delei merged 11 commits into from
Aug 26, 2025
+105 −0

Conversation

@wangmiscoding
Copy link
Contributor

@wangmiscoding wangmiscoding commented Jul 16, 2025 •
edited
Loading

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

  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

@mymde
Copy link

mymde commented Jul 17, 2025

Maybe you can call the afterSheetDispose method within the WriteContextImpl#initSheet method

@wangmiscoding
Copy link
Contributor Author

wangmiscoding commented Jul 22, 2025

Maybe you can call the afterSheetDispose method within the WriteContextImpl#initSheet method

The WriteContextImpl#currentSheet method has not yet started writing data to the sheet

@mymde
Copy link

mymde commented Jul 28, 2025

You are right. What if it is written in the finish() method?

wangmeng added 2 commits August 1, 2025 15:02
Merge branch 'main' of https://github.com/wangmiscoding/fastexcel int…
b373dba 
…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
refactor(fastexcel): Adjust the code according to Spotless
bebc600
wangmiscoding
wangmiscoding commented Aug 1, 2025
View reviewed changes
fastexcel-test/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
Copy link
Contributor Author

@wangmiscoding wangmiscoding Jul 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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

Copy link
Member

@delei delei Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😊 It's fine. We have recently added the spotless plugin. Please use mvn spotless:apply to automate code formatting.

Copy link
Contributor Author

@wangmiscoding wangmiscoding Aug 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😊 It's fine. We have recently added the spotless plugin. Please use mvn spotless:apply to automate code formatting.

ok，I've dealt with it

@delei
Copy link
Member

delei commented Aug 11, 2025

Hi, @wangmiscoding
Could you please resolve the conflict? If no one else has any questions, we plan to merge it into version 1.3.0.

@wangmiscoding
Copy link
Contributor Author

wangmiscoding commented Aug 11, 2025

Hi, @wangmiscoding Could you please resolve the conflict? If no one else has any questions, we plan to merge it into version 1.3.0.

Okay, I’ll handle it later

@wangmiscoding
Copy link
Contributor Author

wangmiscoding commented Aug 11, 2025

Hi,   你好@wangmiscoding Could you please resolve the conflict? If no one else has any questions, we plan to merge it into version 1.3.0.你能解决冲突吗？如果没有其他人有任何问题，我们计划将其合并到 1.3.0 版本中。

I have resolved the conflict

@psxjoy psxjoy requested a review from Copilot August 15, 2025 03:36
Copilot AI reviewed Aug 15, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 afterSheetDispose method to SheetWriteHandler interface
  • Integrated the new method into the execution chain and utility classes
  • Added method calls in ExcelBuilderImpl for 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.

fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java Outdated
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import org.apache.poi.ss.usermodel.*;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using wildcard imports is discouraged as it makes dependencies unclear and can lead to naming conflicts. Import specific classes instead of using '*'.

Suggested change
import org.apache.poi.ss.usermodel.*;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
import java.util.List;
import java.util.Set;
import org.apache.poi.ss.usermodel.*;
import org.apache.poi.ss.usermodel.Cell;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is redundant since Cell is already imported via the wildcard import on line 43. Remove this duplicate import.

Suggested change
import org.apache.poi.ss.usermodel.Cell;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
import java.util.Set;
import org.apache.poi.ss.usermodel.*;
import org.apache.poi.ss.usermodel.Cell;
import org.apache.poi.ss.usermodel.CellStyle;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is redundant since CellStyle is already imported via the wildcard import on line 43. Remove this duplicate import.

Suggested change
import org.apache.poi.ss.usermodel.CellStyle;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
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;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is redundant since FillPatternType is already imported via the wildcard import on line 43. Remove this duplicate import.

Suggested change
import org.apache.poi.ss.usermodel.FillPatternType;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
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;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is redundant since IndexedColors is already imported via the wildcard import on line 43. Remove this duplicate import.

Suggested change
import org.apache.poi.ss.usermodel.IndexedColors;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
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;
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This import is redundant since Workbook is already imported via the wildcard import on line 43. Remove this duplicate import.

Suggested change
import org.apache.poi.ss.usermodel.Workbook;

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/write/WriteTest.java
public void afterSheetDispose(SheetWriteHandlerContext context) {
Sheet sheet = context.getWriteSheetHolder().getSheet();
// 合并区域单元格
sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 10, 2, 2));
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic numbers (1, 10, 2, 2) should be replaced with named constants or variables to improve code readability and maintainability.

Suggested change
sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 10, 2, 2));
sheet.addMergedRegionUnsafe(new CellRangeAddress(MERGE_FIRST_ROW, MERGE_LAST_ROW, MERGE_FIRST_COL, MERGE_LAST_COL));

Copilot uses AI. Check for mistakes.
fastexcel/src/test/java/cn/idev/excel/test/demo/fill/FillTest.java
@Override
public void afterSheetDispose(SheetWriteHandlerContext context) {
Sheet sheet = context.getWriteSheetHolder().getSheet();
sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 2, 0, 1));
Copy link

Copilot AI Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic numbers (1, 2, 0, 1) should be replaced with named constants or variables to improve code readability and maintainability.

Suggested change
sheet.addMergedRegionUnsafe(new CellRangeAddress(1, 2, 0, 1));
sheet.addMergedRegionUnsafe(
new CellRangeAddress(
MERGE_FIRST_ROW,
MERGE_LAST_ROW,
MERGE_FIRST_COL,
MERGE_LAST_COL
)
);

Copilot uses AI. Check for mistakes.
psxjoy
psxjoy approved these changes Aug 15, 2025
View reviewed changes
Copy link
Member

@psxjoy psxjoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@psxjoy
Copy link
Member

psxjoy commented Aug 15, 2025

If no one has other questions, this PR will merge in 24hours.

delei
delei approved these changes Aug 26, 2025
View reviewed changes
Copy link
Member

@delei delei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

This feature will be released in the next version ：)

@delei
Copy link
Member

delei commented Aug 26, 2025 •
edited
Loading

Hi, @wangmiscoding
Please fix the issue in CI, and then we will merge this PR.

@wangmiscoding
Copy link
Contributor Author

wangmiscoding commented Aug 26, 2025

Hi, @wangmiscoding Please fix the issue in CI, and then we will merge this PR.

Sure, I'll handle it later.

wangmeng added 3 commits August 26, 2025 13:11
Merge branch 'main' of https://github.com/wangmiscoding/fastexcel int…
a996782 
…o feature/sheet-dispose
Merge remote-tracking branch 'origin/feature/sheet-dispose' into feat…
25e593d 
…ure/sheet-dispose
Update Excel writing method in test code
c6979d4 
Replace EasyExcel.write with FastExcel.write

Modify two files related to template filling and regular writing tests
@delei delei merged commit c06fa23 into apache:main Aug 26, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@psxjoy psxjoy psxjoy approved these changes

@delei delei delei approved these changes

@jipengfei-jpf jipengfei-jpf Awaiting requested review from jipengfei-jpf

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

add an afterSheetDispose method to the SheetWriteHandler

4 participants

@wangmiscoding @mymde @delei @psxjoy