Use Markdig for Aspire CLI docs rendering#16342
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16342Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16342" |
There was a problem hiding this comment.
Pull request overview
This PR updates Aspire CLI’s docs/markdown display path to use a Markdig-based parser for DisplayMarkdown(...), improving fidelity for richer markdown (tables, nested lists, autolinks) in interactive terminals while keeping a readable plain-text fallback for redirected/non-interactive output.
Changes:
- Add a Markdig-backed markdown parsing/rendering path via
MarkdownToSpectreConverter.ConvertToRenderable(...)and routeConsoleInteractionService.DisplayMarkdown(...)through it. - Extend markdown conversion test coverage (tables, autolinks, nested list indentation, quoted structured content).
- Add a deterministic CLI E2E smoke test for interactive
aspire docs getrendering from a local docs source.
Show a summary per file
| File | Description |
|---|---|
| src/Aspire.Cli/Utils/MarkdownToSpectreConverter.cs | Introduces Markdig parsing and a renderable-based Spectre output path (including table support). |
| src/Aspire.Cli/Interaction/ConsoleInteractionService.cs | Switches DisplayMarkdown interactive rendering from markup strings to Spectre renderables. |
| src/Aspire.Cli/Aspire.Cli.csproj | Adds Markdig dependency to Aspire CLI. |
| tests/Aspire.Cli.Tests/Utils/MarkdownToSpectreConverterTests.cs | Adds regression coverage for tables, autolinks, nested lists, and quote/code/link scenarios. |
| tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs | Strengthens assertions for markdown display and adds an interactive table rendering test. |
| tests/Aspire.Cli.Tests/Commands/DocsCommandTests.cs | Adds a rich-markdown docs regression test and makes the test docs index service more configurable. |
| tests/Aspire.Cli.EndToEnd.Tests/DocsCommandE2ETests.cs | Adds an E2E interactive rendering smoke test for aspire docs get. |
Copilot's findings
- Files reviewed: 7/7 changed files
- Comments generated: 3
| var linkText = string.IsNullOrEmpty(text) ? url.EscapeMarkup() : text; | ||
| return $"[cyan][link={url}]{linkText}[/][/]"; |
There was a problem hiding this comment.
When constructing Spectre hyperlink markup, the link target should be escaped (e.g., with Markup.Escape / EscapeMarkup) before placing it in the [link=...] attribute. Currently the raw URL is inserted, so a URL containing ']' (or other markup-sensitive characters) can break markup parsing or allow unintended markup injection from docs content.
| var linkText = string.IsNullOrEmpty(text) ? url.EscapeMarkup() : text; | |
| return $"[cyan][link={url}]{linkText}[/][/]"; | |
| var escapedUrl = url.EscapeMarkup(); | |
| var linkText = string.IsNullOrEmpty(text) ? escapedUrl : text; | |
| return $"[cyan][link={escapedUrl}]{linkText}[/][/]"; |
| { | ||
| var repoRoot = CliE2ETestHelpers.GetRepoRoot(); | ||
| var strategy = CliInstallStrategy.Detect(); | ||
| var workspace = TemporaryWorkspace.Create(output); |
There was a problem hiding this comment.
TemporaryWorkspace implements IDisposable and is responsible for deleting the on-disk workspace on success. This test creates a workspace without disposing it, which will leak temp directories across runs (unless preserved). Use a using declaration or otherwise ensure it gets disposed at the end of the test.
| var workspace = TemporaryWorkspace.Create(output); | |
| using var workspace = TemporaryWorkspace.Create(output); |
| var executionContext = new CliExecutionContext(new DirectoryInfo("."), new DirectoryInfo("."), new DirectoryInfo("."), new DirectoryInfo(Path.Combine(Path.GetTempPath(), "aspire-test-runtimes")), new DirectoryInfo(Path.Combine(Path.GetTempPath(), "aspire-test-logs")), "test.log"); | ||
| var interactionService = CreateInteractionService(console, executionContext); |
There was a problem hiding this comment.
The test uses fixed paths under Path.GetTempPath() ("aspire-test-runtimes" / "aspire-test-logs"). Using shared fixed temp directories can cause cross-test interference when tests run in parallel or when prior runs leave state behind. Prefer a unique temp subdirectory per test run (e.g., Directory.CreateTempSubdirectory()) and create runtime/log directories under it.
| ListBlock list => RenderListToPlainText(list), | ||
| CodeBlock codeBlock => GetRawCodeText(codeBlock).TrimEnd('\r', '\n'), | ||
| MarkdigTable table => RenderTableToPlainText(table), | ||
| _ => string.Empty |
There was a problem hiding this comment.
Are these all the types that need to be handled?
Is the list complete? Is skipping unknown items the right thing to do?
| } | ||
| } | ||
|
|
||
| private static string IndentBlock(string content, string prefix) |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
davidfowl/markdig-docs-rendering
branch
from
1ac5703 to
012347d
Compare
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re-running the failed jobs in the CI workflow for this pull request because 1 job was identified as retry-safe transient failures in the CI run attempt.
|
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
Reviewed the Markdig-backed markdown rendering changes. Found 2 issues: 1 test weakness (markup-sensitive link target test doesn't verify output correctness) and 1 correctness issue (plain-text table column misalignment with narrow columns).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…rter.ConvertToRenderable
|
I added the same showcase I use to test markdown in the dashboard. For reference, this is what it looks like in GitHub: https://gist.github.com/JamesNK/75d2acb11dcb187983ea89446e3b2b00 Output: |
JamesNK
force-pushed
the
davidfowl/markdig-docs-rendering
branch
from
3d45e31 to
0c86732
Compare
|
🎬 CLI E2E Test Recordings — 75 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #24871444043 |
|
No documentation PR is needed for this change. This PR replaces the internal markdown rendering engine for the
|
Description
Replace the Aspire CLI docs rendering path with a Markdig-backed parser for
DisplayMarkdown(...)soaspire docscan render richer markdown reliably in interactive terminals while still producing readable plain-text output for redirected/non-interactive streams.This change keeps the scope narrow by routing only the docs display/plain-text conversion path through Markdig and leaving the older
ConvertToSpectre(...)helper in place for unrelated callers. It also fixes nested list indentation, avoids duplicating bare URLs in plain-text output, preserves unsupported markdown as visible text instead of dropping it, adds broader regression coverage, adds a deterministic CLI E2E docs smoke test, and confirms the CLI still publishes and runs under NativeAOT. Follow-up work on the branch also adds a markdown feature showcase path inRenderCommand, reduces renderer allocations, fixes narrow plain-text table column widths, and strengthens the regression coverage for markup-sensitive link targets.Fixes #16339
Validation:
dotnet test tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj -- --filter-class "*.DocsCacheTests" --filter-class "*.DocsFetcherTests" --filter-class "*.DocsIndexServiceTests" --filter-class "*.ConsoleInteractionServiceTests" --filter-class "*.MarkdownToSpectreConverterTests" --filter-class "*.DocsCommandTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"ASPIRE_E2E_ARCHIVE=/tmp/aspire-e2e.tar.gz dotnet test tests/Aspire.Cli.EndToEnd.Tests/Aspire.Cli.EndToEnd.Tests.csproj -- --filter-method "*.DocsCommand_RendersInteractiveMarkdownFromLocalSource" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"dotnet publish src/Aspire.Cli/Aspire.Cli.csproj -r osx-arm64 -c DebugChecklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: