Skip to content

Conversation

Mpdreamz
Copy link
Member

@Mpdreamz Mpdreamz commented Feb 22, 2025

If an included snippet defines it's own headers and anchors they did not contribute to the pages anchors and headers.

-- page.md

:::{include} _snippet/snippet.md
:::

This resulted in link check failures to page.md#included-from-snippet, as well as the headers from a snippet not appearing on the page's table of contents.

This PR also extends our testing framework by allowing to collect data from the conversion using IConversionCollector.

The authoring test framework uses the same DocumentationGenerator that docs-builder uses. The default for IConversionCollector is null in real world usage as we don't want the added memory pressure.

An upside of this is that we can now really fully test the whole HTML layout vs just the markdown HTML.

Lastly this PR includes a tiny fix to DiagnosticLinkInlineParser.cs to always report the resolved path as the full path. That way folks don't need to parse all the parent up ../ instructions themselves when dealing with this error instance.

If an included snippet defines it's own headers and anchors they did not contribute to the pages anchors and headers.

-- page.md
```markdown
:::{include} _snippet/snippet.md
:::
```

This resulted in link check failures to `page.md#included-from-snippet`, as well as the headers from a snippet not appearing on the page's table of contents.

This PR also extends our testing framework by allowing to collect data from the conversion using `IConversionCollector`.

The `authoring` test framework uses the same `DocumentationGenerator` that `docs-builder` uses. The default for `IConversionCollector` is `null` in real world usage as we don't want the added memory pressure.

An upside of this is that we can now really fully test the whole HTML layout vs just the markdown HTML.

Lastly this PR includes a tiny fix to DiagnosticLinkInlineParser.cs to always report the resolved path as the full path. That way folks don't need to parse all the parent up `../` instructions themselves when dealing with this error instance.
@Mpdreamz Mpdreamz added the fix label Feb 22, 2025
@Mpdreamz Mpdreamz self-assigned this Feb 22, 2025
@Mpdreamz Mpdreamz requested a review from a team February 22, 2025 12:18
@Mpdreamz Mpdreamz linked an issue Feb 22, 2025 that may be closed by this pull request
3 tasks
@Mpdreamz Mpdreamz changed the title fix/include anchors toc Support anchors and table of contents from included files Feb 22, 2025
@Mpdreamz Mpdreamz marked this pull request as ready for review February 22, 2025 14:31
@Mpdreamz Mpdreamz merged commit a853a3b into main Feb 24, 2025
5 checks passed
@Mpdreamz Mpdreamz deleted the fix/include-anchors-toc branch February 24, 2025 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

False positive about link fails?

2 participants