Skip to content

feat: introduce header merge strategy for Excel writing - #674

Merged
delei merged 17 commits into
apache:mainfrom
liugddx:feat-666
Dec 9, 2025
Merged

feat: introduce header merge strategy for Excel writing#674
delei merged 17 commits into
apache:mainfrom
liugddx:feat-666

Conversation

@liugddx

@liugddx liugddx commented Nov 3, 2025

Copy link
Copy Markdown
Member

Related to #666

@liugddx

liugddx commented Nov 4, 2025

Copy link
Copy Markdown
Member Author

@delei PTAL

@delei
delei requested a review from Copilot November 4, 2025 12:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 introduces a new HeaderMergeStrategy enum to provide fine-grained control over how Excel headers are merged during writing. The enhancement maintains backward compatibility with the existing automaticMergeHead parameter while offering more nuanced merge behaviors.

  • Adds HeaderMergeStrategy enum with five options: NONE, HORIZONTAL_ONLY, VERTICAL_ONLY, FULL_RECTANGLE, and AUTO
  • Refactors the header merge logic in ExcelWriteHeadProperty to support different merge strategies
  • Updates documentation in both English and Chinese to explain the new parameter and its usage

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
fesod/src/main/java/org/apache/fesod/excel/enums/HeaderMergeStrategy.java New enum defining five merge strategies for header cells
fesod/src/main/java/org/apache/fesod/excel/write/property/ExcelWriteHeadProperty.java Refactored merge logic to support different strategies, added helper methods for validation
fesod/src/main/java/org/apache/fesod/excel/write/metadata/WriteBasicParameter.java Added headerMergeStrategy field to parameter class
fesod/src/main/java/org/apache/fesod/excel/write/metadata/holder/AbstractWriteHolder.java Added headerMergeStrategy field and backward compatibility logic
fesod/src/main/java/org/apache/fesod/excel/write/metadata/holder/WriteHolder.java Added headerMergeStrategy() method to interface
fesod/src/main/java/org/apache/fesod/excel/write/builder/AbstractExcelWriterParameterBuilder.java Added headerMergeStrategy() builder method
fesod/src/main/java/org/apache/fesod/excel/context/WriteContextImpl.java Updated to use new merge strategy parameter when initializing headers
fesod/src/test/java/org/apache/fesod/excel/head/HeaderMergeStrategyTest.java Comprehensive test coverage for all merge strategies
website/docs/write/head.md English documentation for the new feature
website/i18n/zh-cn/docusaurus-plugin-content-docs/current/write/head.md Chinese documentation for the new feature
website/docs/help/parameter.md Updated parameter reference in English
website/i18n/zh-cn/docusaurus-plugin-content-docs/current/help/parameter.md Updated parameter reference in Chinese
Comments suppressed due to low confidence (1)

fesod/src/main/java/org/apache/fesod/excel/write/property/ExcelWriteHeadProperty.java:237

  • The parameter 'cellName' is never used.
            List<Head> headList, int row1, int row2, int startCol, int endCol, String cellName) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread fesod/src/test/java/org/apache/fesod/excel/head/HeaderMergeStrategyTest.java Outdated
Comment thread fesod/src/test/java/org/apache/fesod/excel/head/HeaderMergeStrategyTest.java Outdated
delei and others added 4 commits November 4, 2025 20:45
@psxjoy

psxjoy commented Nov 5, 2025

Copy link
Copy Markdown
Member

Very interesting PR,thanks @liugddx
I will handle in this week.

@psxjoy
psxjoy self-requested a review November 5, 2025 07:14
@psxjoy psxjoy added the PR: reviewing Currently under active review. label Nov 5, 2025
@psxjoy

psxjoy commented Nov 12, 2025

Copy link
Copy Markdown
Member

Hi @liugddx , we have updated some package-name. Your PR is very useful,could you mind resolve these conflicts?

# Conflicts:
#	fesod/src/main/java/org/apache/fesod/sheet/context/WriteContextImpl.java
#	fesod/src/main/java/org/apache/fesod/sheet/enums/HeaderMergeStrategy.java
#	fesod/src/main/java/org/apache/fesod/sheet/write/builder/AbstractExcelWriterParameterBuilder.java
#	fesod/src/main/java/org/apache/fesod/sheet/write/metadata/WriteBasicParameter.java
#	fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/AbstractWriteHolder.java
#	fesod/src/main/java/org/apache/fesod/sheet/write/metadata/holder/WriteHolder.java
#	fesod/src/main/java/org/apache/fesod/sheet/write/property/ExcelWriteHeadProperty.java
#	fesod/src/test/java/org/apache/fesod/sheet/head/HeaderMergeStrategyTest.java
@liugddx

liugddx commented Nov 12, 2025

Copy link
Copy Markdown
Member Author

Hi @liugddx , we have updated some package-name. Your PR is very useful,could you mind resolve these conflicts?

Done.

psxjoy
psxjoy previously approved these changes Nov 17, 2025

@psxjoy psxjoy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@psxjoy

psxjoy commented Nov 17, 2025

Copy link
Copy Markdown
Member

If no one has any other views.I will merge this PR in 24 hours.

如果没人对此有其他的看法,我将在 24 小时内合并此 PR。感谢 @liugddx 的贡献:)

@psxjoy

psxjoy commented Nov 18, 2025

Copy link
Copy Markdown
Member

@liugddx I have submitted a PR(liugddx#1) to fix conflicts .
Please make sure If no any changes, merge that PR ,and we can carry on :)

Hi, 我发现还有些冲突,这不是你的问题。我已经在你的分支上提交了相关的PR。等冲突的PR解决后,我将合并这个PR :)

# Conflicts:
#	website/docs/sheet/help/parameter.md
#	website/i18n/zh-cn/docusaurus-plugin-content-docs/current/sheet/help/parameter.md
@liugddx

liugddx commented Nov 22, 2025

Copy link
Copy Markdown
Member Author

@liugddx I have submitted a PR(liugddx#1) to fix conflicts . Please make sure If no any changes, merge that PR ,and we can carry on :)

Hi, 我发现还有些冲突,这不是你的问题。我已经在你的分支上提交了相关的PR。等冲突的PR解决后,我将合并这个PR :)

PTAL

@netlify

netlify Bot commented Nov 23, 2025

Copy link
Copy Markdown

Deploy Preview for fesod failed.

Name Link
🔨 Latest commit d9a60bb
🔍 Latest deploy log https://app.netlify.com/projects/fesod/deploys/69231855ff30520008af3384

@liugddx
liugddx requested a review from psxjoy November 28, 2025 01:53
@delei

delei commented Dec 8, 2025

Copy link
Copy Markdown
Member

Hi, @liugddx
We have introduced markdownlint-cli2 to format .md files. When you have time, please run pnpm run md-lint-fix in the website directory to automatically fix the issues.

After the processing is completed, we will merge this PR ASAP.

@liugddx

liugddx commented Dec 9, 2025

Copy link
Copy Markdown
Member Author

pnpm run md-lint-fix

Done.

@delei delei left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@delei
delei merged commit a501139 into apache:main Dec 9, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: reviewing Currently under active review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants