fix(docs): markdown empty-header tables leak last row after the table#611
Open
chrischall wants to merge 2 commits into
Open
fix(docs): markdown empty-header tables leak last row after the table#611chrischall wants to merge 2 commits into
chrischall wants to merge 2 commits into
Conversation
Closes openclaw#609. `isTableSeparator` accepted rows of empty pipe cells like `| | |` as separators because the per-segment loop hit `continue` for empty segments and never tripped the "must have at least one dash" check. The parser then consumed the empty header AND the actual `|---|---|` separator, advanced the outer loop too far, and re-parsed the last data row as a literal pipe paragraph after the table. Track a `sawDashSegment` flag and require at least one non-empty segment that actually contains dashes. `||` is still rejected by the existing `len(segments) > 1` guard. Tests: - TestIsTableSeparator_EmptyPipeRowRejected — table-driven cases including the empty-cell forms from the issue - TestParseMarkdown_EmptyHeaderTableKeepsAllDataRows — full ParseMarkdown regression covering the exact repro Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes #609.
Root cause
isTableSeparatoraccepted rows of empty pipe cells like| | |as separators because the per-segment loop hitcontinuefor empty segments and never tripped the "must have at least one dash" check at the end of the loop body. Concretely:With input
parseMarkdownTablethen skipped both row 0 (false separator) and row 1, returning only 2 data rows. The outer parser advancedi += len(tableCells)= 2, putting the next iteration on row 3, where| Label B | Value B |was re-parsed as a literal pipe paragraph.Fix
Track
sawDashSegmentinisTableSeparatorand require at least one non-empty, dash-containing segment.||is still rejected by the existinglen(segments) > 1guard, so this change is strictly additive.Tests
TestIsTableSeparator_EmptyPipeRowRejected— table-driven cases covering empty-cell forms, normal separators, and alignment variants.TestParseMarkdown_EmptyHeaderTableKeepsAllDataRows— fullParseMarkdownregression that exercises the exact repro from the issue and asserts there is exactly oneMDTableelement with all three rows intact.go test ./internal/cmd/ -count=1→ ok 23.5s.