Open
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 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" |
Contributor
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
Comment on lines
+399
to
+400
| 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.
Suggested change
| 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.
Suggested change
| var workspace = TemporaryWorkspace.Create(output); | |
| using var workspace = TemporaryWorkspace.Create(output); |
Comment on lines
+210
to
+211
| 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.
Contributor
|
🎬 CLI E2E Test Recordings — 73 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24688968338 |
eerhardt
reviewed
Apr 20, 2026
| ListBlock list => RenderListToPlainText(list), | ||
| CodeBlock codeBlock => GetRawCodeText(codeBlock).TrimEnd('\r', '\n'), | ||
| MarkdigTable table => RenderTableToPlainText(table), | ||
| _ => string.Empty |
Member
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?
eerhardt
reviewed
Apr 20, 2026
| return builder.ToString(); | ||
| } | ||
|
|
||
| private static string RenderListItem(ListItemBlock item, string prefix, int continuationIndent, Func<Block, string> blockRenderer) |
Member
There was a problem hiding this comment.
We are creating a lot of throw away strings. Should we just be passing in the StringBuilder into these private methods?
eerhardt
reviewed
Apr 20, 2026
| } | ||
| } | ||
|
|
||
| private static string IndentBlock(string content, string prefix) |
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.
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, adds broader regression coverage, adds a deterministic CLI E2E docs smoke test, and confirms the CLI still publishes and runs under NativeAOT.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: