Plain text: a captioned figure keeps its caption on its own line - #271
Merged
Conversation
PlainTextRenderer had no arm for Figure and none for Caption, so both fell through to the default child-concatenation. A captioned image rendered as `aPanel caption`: the alt text glued straight onto the caption, no separator, no trailing blank line, and nothing to tell a reader where the figure ended. Every other block in this renderer terminates with a blank line, and the ANSI renderer has carried a figure and caption arm all along. Plain text now matches: each child on its own line, the caption last, one blank line after the figure. The table path is untouched. Its caption goes through renderTable(), which reads the caption off the node rather than dispatching it, so the new Caption arm cannot reach it; the existing table-caption test pins that output unchanged.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #271 +/- ##
=========================================
Coverage 92.41% 92.41%
- Complexity 3674 3682 +8
=========================================
Files 109 109
Lines 10424 10440 +16
=========================================
+ Hits 9633 9648 +15
- Misses 791 792 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Follow-up to #270, but a standalone bug that predates it: found while checking how a composite figure degrades to plain text, and reproducible on master with an ordinary captioned image.
PlainTextRenderer::renderNode()has no arm forFigureand none forCaption, so both fall through todefault => $this->renderChildren($node).Input:
Plain text on master:
The alt text is glued straight onto the caption, with no separator and no trailing blank line, so nothing marks where the figure ends. Every other block in this renderer terminates with a blank line, and
AnsiRendererhas hadrenderFigure()andrenderCaption()all along - plain text was the only target missing them.After:
Shape
renderFigure()renders each child on its own line, caption last, and terminates the block with one blank line like every neighbouring block method.renderCaption()returns the trimmed caption text with no terminator, so the caller owns the separators; the standalone arm only exists for a caption that somehow appears outside a figure.The table path is untouched
A table caption never dispatches through
renderNode()-renderTable()reads it off the node withgetCaption()- so the newCaptionarm cannot reach it. The existing table-caption test pins that output unchanged, which is what makes the claim checkable rather than merely stated.Interaction with the composite-figure branch
#270 has a degradation test that asserts the current glued plain-text string. Whichever of the two lands second needs that assertion updated to the fixed output; #270 is a draft, so the simplest order is this one first and then a rebase there.