Repository navigation
fix(sheet): apply AUTO header merge context on the right axis - #1202
Open
Sean-Walker0 wants to merge 1 commit into
Open
Sean-Walker0 wants to merge 1 commit into
Sean-Walker0 wants to merge 1 commit into
Conversation
Sean-Walker0
force-pushed
the
fix/auto-header-merge-context
branch
2 times, most recently
from
October 14, 2026 12:13
87ea95b to
2f07984
Compare
The AUTO merge strategy in ExcelWriteHeadProperty validates merge context on the wrong axis, with two visible effects: - Horizontal runs still join same-name cells that live under different parent groups. This is what issue apache#666 reported for a dynamic head (head30/head31 next to head40/head31): the reported C2:D2 merge still happens on current main. - The AUTO-only canMergeVertically check compares the cell above a vertical run with a cell inside the run. A vertical run starting at row >= 1 only exists when the cell above differs from the run (an equal cell above would have consumed the run from the earlier row), so the comparison is provably false there and AUTO never merges equal cells below the first header row anymore - a regression against both the pre-strategy automatic merge and VERTICAL_ONLY. Fix both by validating context where it belongs: under AUTO, a horizontal run may only join cells whose parent labels (the cell above each column) are equal, and vertical runs no longer carry the wrong-axis check, so equal cells under the same parent merge again. Verified with AutoHeaderMergeContextTest (both cases red before the change, green after) plus the full fesod-sheet suite (950 tests) and spotless:check.
Sean-Walker0
force-pushed
the
fix/auto-header-merge-context
branch
from
October 15, 2026 06:45
2f07984 to
ea07bde
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the pull request
The AUTO header merge strategy validates merge context on the wrong axis in
ExcelWriteHeadProperty.headCellRangeList, producing two wrong merges:C2:D2merged region it complains about: same-name cells (head31/head31) sitting in columns under different parents (head30vshead40) are joined horizontally, because the horizontal run has no context check at all. feat: introduce header merge strategy for Excel writing #674 addedcanMergeVerticallyfor this report, but that only guards the vertical path.canMergeVerticallycompares the cell above a vertical run with a cell inside the run (row1-1vsrow2-1whererow2-1is a run member). A vertical run starting at row >= 1 can only exist when the cell above differs from it (otherwise the run was already consumed from the earlier row), so the comparison is provably always false there. AUTO consequently never merges equal cells under the same parent below row 0 — a regression against the pre-strategy automatic merge and againstVERTICAL_ONLY, which both merge them.Found by code audit of the merge algorithm; both effects verified end-to-end before this change (issue #666's head yields
C2:D2; head columns[G,A,A] [G,B,y] [G,B,z]yield noA2:A3).What's changed?
j-1) are equal — this is the context check the horizontal path was missing, and it fixes the [Bug] Merge error in dynamic header #666 case (head30vshead40differ, no merge) while keeping same-parent runs merged (B/Bunder a mergedGrun still merge).canMergeVerticallycheck is removed from the vertical path (the method became dead code), restoring automatic vertical merging of equal cells under the same parent (A2:A3merges again).VERTICAL_ONLY,HORIZONTAL_ONLYandFULL_RECTANGLEare untouched.Checklist
Verifying this change
AutoHeaderMergeContextTest(both directions):autoDoesNotMergeSameNameCellsUnderDifferentParents— the [Bug] Merge error in dynamic header #666 head; red before (same-name cells under different parents must not merge ==> expected: <false> but was: <true>), green after; also pins that first-row runs (head40/head40) still merge;autoMergesVerticalRunsBelowTheFirstHeaderRow— red before (equal cells under the same parent must merge vertically ==> expected: <true> but was: <false>), green after; also pins same-parent horizontal merges (B/BunderG).Existing
HeaderMergeStrategyTest: all 5 cases still pass../mvnw clean package -B -Dmaven.test.skip=false -pl fesod-common,fesod-shaded,fesod-sheet: 950 tests, 0 failures, 0 errors../mvnw -pl fesod-sheet spotless:check: clean.Notes
ExcelWriteHeadPropertyonly in the annotation-processing constructor hunks, and refactor: Improve the basic javadoc content #1181 only javadoc in the enum — no overlap withheadCellRangeList.initHeadRowNumberpads ragged head columns by repeating their last value, so a one-level column in a two-level head renders merged with itself (A1:A2in the [Bug] Merge error in dynamic header #666 file); that normalization predates the merge strategies and is left as is.