Open
Conversation
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 16304Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 16304" |
Revalidate the parsed docs index against the fetched docs source so cached docs refresh when the source changes, while still falling back to cached content when refresh fails. Also improve docs markdown output by reconstructing inline tables, avoiding table-row wrapping, and using a plain-text path for redirected or non-interactive output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidfowl
force-pushed
the
davidfowl/fix-docs-command
branch
from
aea83d9 to
d9c8c3f
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes aspire docs behavior by preventing stale cached indexes when the underlying docs source changes, and by improving CLI markdown rendering for tables/links—especially when output is redirected or non-interactive.
Changes:
- Revalidates the cached parsed docs index using a computed source fingerprint, refreshing when the fetched source content changes and falling back to cache when refresh fails.
- Improves docs-get rendering by (a) normalizing minified inline markdown tables and (b) avoiding table-row wrapping; adds a plain-text rendering path for redirected/non-interactive output.
- Adds/updates regression tests for cache refresh behavior and markdown rendering behavior.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Hosting.JavaScript.Tests/AddViteAppTests.cs | Updates Node-version selection test coverage for generated Dockerfiles (includes new .tool-versions tab case and package.json engines expectations). |
| tests/Aspire.Cli.Tests/Utils/MarkdownToSpectreConverterTests.cs | Adds coverage for the new plain-text markdown conversion behavior. |
| tests/Aspire.Cli.Tests/Mcp/Docs/DocsIndexServiceTests.cs | Adds coverage for cached index revalidation/refresh and inline table normalization. |
| tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs | Verifies non-interactive output uses a plain-text markdown fallback. |
| tests/Aspire.Cli.Tests/Commands/DocsCommandTests.cs | Adds regression coverage ensuring table rows are not wrapped for console output. |
| src/Aspire.Hosting.JavaScript/JavaScriptHostingExtensions.cs | Changes Node.js version resolution logic for Dockerfile base image selection. |
| src/Aspire.Cli/Utils/MarkdownToSpectreConverter.cs | Adds ConvertToPlainText for redirected/non-interactive output rendering. |
| src/Aspire.Cli/Interaction/ConsoleInteractionService.cs | Routes markdown output to plain-text conversion when non-interactive or redirected. |
| src/Aspire.Cli/Documentation/Docs/DocsIndexService.cs | Implements fingerprint-based cache revalidation and inline table normalization during content normalization. |
| src/Aspire.Cli/Commands/DocsGetCommand.cs | Prevents wrapping of markdown table lines during console wrapping. |
Copilot's findings
- Files reviewed: 8/8 changed files
- Comments generated: 1
Comment on lines
+75
to
+78
| /// Converts markdown to plain text suitable for redirected or non-interactive output. | ||
| /// </summary> | ||
| /// <param name="markdown">The markdown text to convert.</param> | ||
| /// <returns>Markdown with links rewritten to plain text and image references removed.</returns> |
There was a problem hiding this comment.
The XML doc for ConvertToPlainText is inaccurate: the implementation removes headers and basic formatting (bold/italic/strikethrough) in addition to rewriting links and removing images. Please update the text (and/or summary) to describe the full set of transformations so callers know this is a lossy plain-text conversion, not just a link/image rewrite.
Suggested change
| /// Converts markdown to plain text suitable for redirected or non-interactive output. | |
| /// </summary> | |
| /// <param name="markdown">The markdown text to convert.</param> | |
| /// <returns>Markdown with links rewritten to plain text and image references removed.</returns> | |
| /// Converts markdown to a lossy plain-text representation suitable for redirected or non-interactive output. | |
| /// </summary> | |
| /// <param name="markdown">The markdown text to convert.</param> | |
| /// <returns>Plain text with links rewritten to <c>text (url)</c>, image references removed, header markers stripped, and basic formatting markers for bold, italic, and strikethrough removed.</returns> |
Contributor
|
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.
|
Update the ConvertToPlainText XML documentation so it accurately describes the lossy plain-text conversion performed for redirected and non-interactive output. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🎬 CLI E2E Test Recordings — 72 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #24618970596 |
mitchdenny
approved these changes
Apr 20, 2026
JamesNK
reviewed
Apr 20, 2026
Comment on lines
+79
to
+102
| public static string ConvertToPlainText(string markdown) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(markdown)) | ||
| { | ||
| return markdown; | ||
| } | ||
|
|
||
| var result = markdown.Replace("\r\n", "\n").Replace("\r", "\n"); | ||
| result = ConvertImages(result); | ||
| result = ConvertLinksToPlainText(result); | ||
| result = HeaderLevel6Regex().Replace(result, "$1"); | ||
| result = HeaderLevel5Regex().Replace(result, "$1"); | ||
| result = HeaderLevel4Regex().Replace(result, "$1"); | ||
| result = HeaderLevel3Regex().Replace(result, "$1"); | ||
| result = HeaderLevel2Regex().Replace(result, "$1"); | ||
| result = HeaderLevel1Regex().Replace(result, "$1"); | ||
| result = BoldDoubleAsterisksRegex().Replace(result, "$1"); | ||
| result = BoldDoubleUnderscoresRegex().Replace(result, "$1"); | ||
| result = ItalicSingleAsteriskRegex().Replace(result, "$1"); | ||
| result = ItalicSingleUnderscoreRegex().Replace(result, "$1"); | ||
| result = StrikethroughRegex().Replace(result, "$1"); | ||
|
|
||
| return result; | ||
| } |
Member
There was a problem hiding this comment.
I think we're going to keep running into issues with using regex. For example, markdown tables aren't converted to Spectre.Console tables.
We should consider using Markdig. We can parse to a markdown DOM, then transform to whatever format we want.
JamesNK
approved these changes
Apr 20, 2026
Member
JamesNK
left a comment
JamesNK
left a comment
There was a problem hiding this comment.
LGTM. Cache revalidation logic, table normalization regexes, and non-interactive plain-text fallback all look correct. No issues found.
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
Fixes stale
aspire docscache behavior and the broken rendering path inaspire docs get.llms-small.txtchanges, while still falling back to cached content if refresh fails.Azure:SubscriptionIdremain readable.Fixes #16302
Checklist
<remarks />and<code />elements on your triple slash comments?aspire.devissue: