Skip to content

fix: preserve thematic breaks in overwrite Markdown - #11142

Merged
vicancy merged 3 commits into
dotnet:mainfrom
vicancy:lianwei-microsoft-overwrite-thematic-break-parsing
Sep 17, 2026
Merged

vicancy merged 3 commits into
dotnet:mainfrom
vicancy:lianwei-microsoft-overwrite-thematic-break-parsing

Conversation

@vicancy

@vicancy vicancy commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Fixes #11088.

Overwrite files intentionally support multiple UID sections. Their Markdown pipeline enables Markdig's mid-document YAML parser before the thematic-break parser, so a pair of body --- lines currently consumes the enclosed Markdown as YAML. Dictionary deserialization then warns and drops that content.

This change is a bounded reduction in false positives, not an unambiguous way to infer whether an author intended metadata or prose.

Approach

Keep Markdig's delimiter handling and inspect only the first YAML root event for subsequent header candidates:

  • Mapping roots remain overwrite headers. Existing key order, quoted/flow/explicit keys, leading comments and blank lines, block scalars, repeated UIDs, and *content syntax remain supported.
  • Non-mapping roots return to normal Markdown parsing, preserving the reporter's rules, History heading, and numbered list.
  • Header validation on the first line of the file is unchanged. YAML syntax errors encountered during recognition stay on the existing diagnostic path, as do malformed/unsupported mappings and mappings missing uid.
  • Lookahead reads from the source offset without copying the entire remaining document for each candidate.

This is not a first-header-only restriction, a literal uid:/first-line heuristic, or a "deserialization failed, so render Markdown" fallback. Recognizing a mapping commits the block to metadata processing; it does not require the rest of the block to deserialize successfully.

Limitations and compatibility

The same --- delimiters can represent both overwrite headers and thematic breaks. YAML syntax can classify the structure, but cannot establish the author's intent.

Case in a later triple-dashed block Behavior and limitation
Mapping-shaped prose such as Title: Hello Still treated as metadata and reports missing uid. Adding uid makes it an overwrite section, even if the author intended to display it as prose.
Mapping begins correctly but later YAML is malformed Still reports an error; it does not fall back to Markdown.
A root-identification syntax error Keeps the original header/diagnostic path. Not every possible Markdown body is therefore guaranteed to render without a YAML diagnostic.
A malformed intended header such as uid Some.Type, or a sequence such as - uid: Some.Type Intentional diagnostic change: classified as non-mapping body content, with no invalid-header warning for that root. The intended overwrite is not applied. The same invalid header on the first line retains the original validation.
Plain non-mapping content, leading blank lines, or comments Classification follows YAML grammar, not a hand-written prefix rule. Blank lines and comments alone do not rule out a header.

Use *** or ------ for thematic breaks when unambiguous body rendering is required. The expanded Thematic breaks and limitations section in docs/tutorial/intro_overwrite_files.md explains these tradeoffs with an example. This PR does not claim to eliminate every ambiguity or preserve every diagnostic for invalid inputs.

Test structure

  • In-memory syntax coverage: 28 parameterized/fact cases in YamlHeaderTest call the real MarkdigMarkdownService through the existing TestUtility.CreateMarkdownService helper. They check exact rendered HTML, header source ranges, and diagnostics without creating files. The overwrite.md argument is only the logical source name used in diagnostics.
  • Focused reader integration: 5 file-backed cases retain the exact LF/CRLF reporter examples, distinct/repeated UID section splitting with fenced code, metadata and source positions, and missing-UID validation. The new regression cases therefore create 5 files per target framework instead of 31.
  • Existing reader tests remain intact. No fake parser, shared file fixture, production API changes for testing, or new dependencies were introduced.

Validation

Against upstream ad05df46ab615d34af3f81849ae5e45ecb4fd51e:

  • Before: the exact reporter example failed for both LF and CRLF, producing zero horizontal rules instead of two.
  • After: both cases pass through real files, HostService, and MarkdownReader.ReadMarkdownAsOverwrite, preserving the summary, two rules, heading/list content, and source line ranges.
  • Docfx.Build.Common.Tests: 25 passed on each of .NET 8 and .NET 10.
  • Docfx.MarkdigEngine.Tests: 52 passed, 5 existing skips on each of .NET 8 and .NET 10; includes conceptual-mode and initial-header diagnostic coverage.
  • Both test-project dependency graphs compile for net8.0, net9.0, and net10.0, with zero warnings/errors.
  • Changed-file whitespace formatting verification and git diff --check pass.
  • The implementation commit passed upstream Windows/macOS/Linux CI, Lint, CLA, and Codecov checks. Follow-ups clarify limitations and improve test layering without changing the production parser.

.NET 9 runtime execution was unavailable locally; its target compiled successfully. No dependencies, workflows, release/versioning, certificates, or publishing configuration changed.

vicancy and others added 3 commits September 17, 2026 18:01
Distinguish subsequent YAML mappings from Markdown body content without restricting overwrite files to a single UID section. Keep initial-header validation and mapping diagnostics, and cover the reported content loss with reader and rendering regressions.

Fixes dotnet#11088

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: bf666f99-8d30-4763-878d-b566c0ac2f2d
Explain mapping-shaped prose ambiguity, preserved syntax-error handling, and the diagnostic change for malformed non-mapping later headers. Recommend unambiguous thematic-break delimiters.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: bf666f99-8d30-4763-878d-b566c0ac2f2d
Move syntax and diagnostic cases to the real Markdown service without file I/O. Keep focused reader integration coverage for the report, repeated and distinct UIDs, metadata, source positions, and missing-UID validation.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: bf666f99-8d30-4763-878d-b566c0ac2f2d
@vicancy
vicancy merged commit 6b9aeee into dotnet:main Sep 17, 2026
9 checks passed
This was referenced Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] Markdown to Override API fails if it has multiple "---"

2 participants