Add support for the Marpit color directive#146
Conversation
There was a problem hiding this comment.
Pull request overview
Adds Marpit color directive support across parsing, slide style modeling, and PPTX rendering so authors can override default slide text color (including headings) via front matter, local directives, and spot directives.
Changes:
- Extend parser +
SlideStyleto recognize and carrycolor(global/local/spot) and include it in slide identity hashing for update mode. - Apply slide-level text color overrides in the PPTX renderer (headings/body/code/inline-code).
- Add focused parser/renderer tests and update directive smoke sample + smoke runner selection.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MarpToPptx.Tests/PptxRendererTests.cs | Adds renderer test asserting slide color affects heading text runs. |
| tests/MarpToPptx.Tests/ParserTests.cs | Adds parser coverage for global color and spot _color carry-forward behavior. |
| src/MarpToPptx.Pptx/Rendering/OpenXmlPptxRenderer.cs | Applies slide-level text color overrides by mutating the effective theme per slide. |
| src/MarpToPptx.Core/Parsing/MarpMarkdownParser.cs | Allows color from front matter to flow into default slide style. |
| src/MarpToPptx.Core/Parsing/MarpDirectiveParser.cs | Adds directive mapping for color into SlideStyle.Color. |
| src/MarpToPptx.Core/Models/SlideDeck.cs | Introduces SlideStyle.Color field. |
| src/MarpToPptx.Core/Authoring/SlideIdentityGenerator.cs | Includes Color in slide content hash (update mode correctness). |
| scripts/Invoke-AllPptxSmokeTests.ps1 | Adds 02-directives.md to the selected contrast-audit sample set. |
| samples/02-directives.md | Updates directives sample to exercise color local/spot behavior. |
| doc/marp-markdown.md | Documents color as a supported directive in feature matrix + front matter table. |
Comments suppressed due to low confidence (1)
tests/MarpToPptx.Tests/ParserTests.cs:1424
- This spot directive example uses a CSS named color (
red). Since the renderer writes run colors asa:srgbClrhex values, named colors will currently serialize to an invalid hex string. Use a hex/rgb() value here, or add proper CSS color parsing for directive values.
<!-- _color: red -->
| return theme with | ||
| { | ||
| TextColor = color, | ||
| Body = theme.Body with { Color = color }, | ||
| Code = theme.Code with { Color = color }, |
There was a problem hiding this comment.
ApplySlideTextColor stores the raw directive value (e.g. "rgb(...)" / "white") into ThemeDefinition.*.Color, but the renderer later writes colors using NormalizeColor, which currently only produces valid hex for #RGB/#RRGGBB inputs. This can emit invalid a:srgbClr values and lead to OpenXML validation errors or PowerPoint repair when users use Marpit-legal color formats. Consider normalizing/validating the directive color to a 6-digit hex string before applying it (or upgrade NormalizeColor to handle rgb()/rgba() similarly to DiagramThemeFactory).
| <!-- color: white --> | ||
| # Slide One |
There was a problem hiding this comment.
This test uses CSS named colors (white, red) for the color directive. The PPTX renderer currently only serializes colors correctly for hex inputs (it writes a:srgbClr), so these values would produce invalid color hex in output if exercised end-to-end. Either constrain directive/test values to supported hex/rgb forms, or extend color normalization to support named colors.
This issue also appears on line 1424 of the same file.
| <!-- backgroundColor: #102A43 --> | ||
| <!-- backgroundImage: --> | ||
| <!-- class: contrast --> | ||
| <!-- color: #FFFFFF --> |
There was a problem hiding this comment.
class: contrast is used here, but the front-matter style: block no longer defines a section.contrast { ... } variant (only section.lead). As a result the sample no longer demonstrates a meaningful class-variant override for contrast (it just sets a class name with no styling effect in MarpToPptx). Consider either reintroducing a minimal section.contrast style (not just color) or changing this directive to a class that is actually defined in the sample CSS.
|
@copilot apply changes based on the comments in this thread |
…ex colors, add section.contrast CSS Agent-Logs-Url: https://github.com/jongalloway/MarpToPptx/sessions/82ea9712-e556-49bd-a1f2-328f5703cbc1 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
…mple Agent-Logs-Url: https://github.com/jongalloway/MarpToPptx/sessions/82ea9712-e556-49bd-a1f2-328f5703cbc1 Co-authored-by: jongalloway <68539+jongalloway@users.noreply.github.com>
Applied all three review fixes in commit
|
Summary
colordirective in front matter, local directives, and spot directivesFixes #144.
Validation
dotnet test tests/MarpToPptx.Tests/MarpToPptx.Tests.csproj -c Release --no-restore -- --filter-method "*Parser_FrontMatterColor_AppliesToAllSlidesByDefault" --filter-method "*Parser_SpotColor_DoesNotCarryForward" --filter-method "*Renderer_AppliesSlideColorDirectiveToHeadingText"./scripts/Invoke-PptxSmokeTest.ps1 -InputMarkdown samples/02-directives.md -Configuration Release -CiSafe