Improve CLI docs markdown rendering and simplify DocsGetCommand#16428
Open
Improve CLI docs markdown rendering and simplify DocsGetCommand#16428
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16428Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16428" |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets improved markdown rendering in the Aspire CLI by tightening paragraph/table spacing and introducing a width cap for markdown rendering in certain command outputs.
Changes:
- Trim trailing newlines when converting markdown paragraphs to Spectre markup to avoid extra blank lines before tables.
- Add an optional
maxWidthparameter toIInteractionService.DisplayMarkdownand apply it when rendering resource command markdown output. - Expand docs content normalization logic and add unit tests covering normalization and a heading+paragraph+table rendering scenario.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/MarkdownToSpectreConverterTests.cs | Stabilizes Spectre console output and adds an exact-output test for paragraph+table spacing. |
| tests/Aspire.Cli.Tests/TestServices/TestInteractionService.cs | Updates test stub to match new DisplayMarkdown(..., maxWidth) signature. |
| tests/Aspire.Cli.Tests/TestServices/TestExtensionInteractionService.cs | Updates test stub to match new DisplayMarkdown(..., maxWidth) signature. |
| tests/Aspire.Cli.Tests/Templating/DotNetTemplateFactoryTests.cs | Updates local test interaction stub to match new DisplayMarkdown signature. |
| tests/Aspire.Cli.Tests/Projects/ExtensionGuestLauncherTests.cs | Updates local test interaction stub to match new DisplayMarkdown signature. |
| tests/Aspire.Cli.Tests/Mcp/Docs/DocsIndexServiceTests.cs | Adds extensive unit tests for docs markdown normalization behavior. |
| tests/Aspire.Cli.Tests/Commands/UpdateCommandTests.cs | Updates wrapper to forward maxWidth to the inner interaction service. |
| tests/Aspire.Cli.Tests/Commands/PublishCommandPromptingIntegrationTests.cs | Updates test interaction implementation to match new DisplayMarkdown signature. |
| tests/Aspire.Cli.Tests/Commands/DocsCommandTests.cs | Removes tests for markdown wrapping logic that was deleted from DocsGetCommand. |
| src/Aspire.Cli/Utils/Markdown/MarkdownToSpectreConverter.Markup.cs | Trims trailing newlines after paragraph conversion to avoid double-blank-line output. |
| src/Aspire.Cli/Interaction/IInteractionService.cs | Adds optional maxWidth parameter to DisplayMarkdown. |
| src/Aspire.Cli/Interaction/ExtensionInteractionService.cs | Updates signature to accept maxWidth (but currently doesn’t forward it to console rendering). |
| src/Aspire.Cli/Interaction/ConsoleInteractionService.cs | Implements maxWidth by temporarily adjusting the target console profile width. |
| src/Aspire.Cli/Documentation/Docs/DocsIndexService.cs | Makes NormalizeContent internal and adds normalization rules for spacing after headings/tables/lists. |
| src/Aspire.Cli/Commands/ResourceCommandHelper.cs | Uses maxWidth: 100 when displaying markdown results from resource commands. |
| src/Aspire.Cli/Commands/DocsGetCommand.cs | Removes prior wrapping/splitting logic, but currently contains debugger launch + hard-coded markdown output. |
Comments suppressed due to low confidence (3)
src/Aspire.Cli/Commands/DocsGetCommand.cs:104
- This command currently prints a hard-coded markdown block (the API table) before printing
doc.Content, which will duplicate/override the actual docs output and doesn't match the PR description. It looks like debugging/test scaffolding—please remove and only display the fetched document content (optionally with any intended formatting/wrapping).
return ExitCodeConstants.Success;
}
}
src/Aspire.Cli/Interaction/ExtensionInteractionService.cs:435
maxWidthis accepted by this method but not forwarded to the console renderer (_consoleInteractionService.DisplayMarkdown), so width limiting is ignored in extension mode. PassmaxWidththrough (and ensure consoleOverride/maxWidth semantics stay consistent across modes).
public void DisplayMarkdown(string markdown, ConsoleOutput? consoleOverride = null, int? maxWidth = null)
{
// Send raw markdown to extension (it can handle markdown natively)
// Convert to Spectre markup for console display
var result = _extensionTaskChannel.Writer.TryWrite(() => Backchannel.LogMessageAsync(LogLevel.Information, markdown, _cancellationToken));
Debug.Assert(result);
_consoleInteractionService.DisplayMarkdown(markdown, consoleOverride);
}
src/Aspire.Cli/Commands/DocsGetCommand.cs:63
Debugger.Launch()should not be present in a shipped CLI command; it will prompt/attach a debugger every timeaspire docs getruns (and can hang CI or user sessions). Remove this call before merging.
using var activity = Telemetry.StartDiagnosticActivity(Name);
JamesNK
changed the title
Contributor
|
🎬 CLI E2E Test Recordings — 75 recordings uploaded (commit View all recordings
📹 Recordings uploaded automatically from CI run #24877400952 |
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
Improve how the CLI renders documentation markdown fetched from aspire.dev.
Changes
Simplify
DocsGetCommand: Replace the manualWrapMarkdownForConsole/SplitMarkdownBlockslogic with a singleDisplayMarkdowncall usingmaxWidth: 100. The old code manually word-wrapped, split blocks, and re-assembled markdown — all of which is now handled by the markdown converter and Spectre.Console.Add
maxWidthparameter toDisplayMarkdown: TheIInteractionService.DisplayMarkdownmethod now accepts an optionalmaxWidthparameter so callers can cap the rendering width (e.g., for wide tables in docs or resource commands).Fix paragraph trailing newline in
MarkdownToSpectreConverter: Trim trailing\nfrom paragraph blocks so that a paragraph followed by a table produces exactly one blank line instead of two when rendered viaConvertToRenderable.Improve
NormalizeContentinDocsIndexService: The llms.txt format strips most whitespace from markdown. Added regex passes to insert blank lines after headings, tables, and list items so the content parses as valid markdown. Made the methodinternalfor testability and added xmldoc explaining its purpose.Resource command markdown: Set
maxWidth: 100when displaying markdown from resource command results.Cleanup: Removed a leftover
Debugger.Launch()call fromDocsGetCommand.Checklist