Use Spectre.Console link markup for CLI URL output#15573
Open
Use Spectre.Console link markup for CLI URL output#15573
Conversation
- Wrap URLs in [link] markup so terminal hyperlinks work and aren't broken by line wrapping - DescribeCommand: use DisplayName as link text when present, sort endpoints matching dashboard order (SortOrder desc, https first, then by Name) - PsCommand: dashboard URL uses link markup with scheme://authority as display text - DoctorCommand: prerequisites URL and environment check links use link markup - Program.cs: telemetry notice URL uses link markup - Extract URL scheme deny-list into shared KnownUnsupportedUrlSchemes.cs, used by both CLI and Dashboard - Add redis and rediss to unsupported URL schemes
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15573Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15573" |
Contributor
There was a problem hiding this comment.
Pull request overview
Updates Aspire CLI (and Dashboard URL rendering) to use Spectre.Console [link] markup so URLs render as terminal hyperlinks and aren’t broken by wrapping; also centralizes the “unsupported URL scheme” deny-list into a shared helper.
Changes:
- Added
KnownUnsupportedUrlSchemesinsrc/Shared/and wired it into Dashboard + CLI. - Updated CLI commands (Describe/Ps/Doctor/Program) to emit
[link]markup for URLs and to sort/format endpoint lists. - Parameterized hard-coded URLs in resource strings to support injecting link markup at runtime.
Reviewed changes
Copilot reviewed 36 out of 38 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Shared/KnownUnsupportedUrlSchemes.cs | New shared deny-list + helper for determining whether a URL should be rendered as a link. |
| src/Aspire.Dashboard/Model/ResourceUrlHelpers.cs | Uses shared unsupported-scheme list when deciding whether to surface URLs as links. |
| src/Aspire.Dashboard/Aspire.Dashboard.csproj | Links the new shared file into the Dashboard build. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.zh-Hant.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.zh-Hans.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.tr.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.ru.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.pt-BR.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.pl.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.ko.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.ja.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.it.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.fr.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.es.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.de.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/RootCommandStrings.cs.xlf | Telemetry notice string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.zh-Hant.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.zh-Hans.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.tr.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.ru.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.pt-BR.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.pl.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.ko.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.ja.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.it.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.fr.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.es.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.de.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/xlf/DoctorCommandStrings.cs.xlf | Prerequisites link string updated to accept a URL placeholder. |
| src/Aspire.Cli/Resources/RootCommandStrings.resx | Parameterized telemetry URL and added translator comment. |
| src/Aspire.Cli/Resources/RootCommandStrings.Designer.cs | Regenerated designer docs for the updated resource format string. |
| src/Aspire.Cli/Resources/DoctorCommandStrings.resx | Parameterized prerequisites URL and added translator comment. |
| src/Aspire.Cli/Resources/DoctorCommandStrings.Designer.cs | Regenerated designer docs for the updated resource format string. |
| src/Aspire.Cli/Program.cs | Emits telemetry notice URL as Spectre [link] markup. |
| src/Aspire.Cli/Commands/PsCommand.cs | Renders Dashboard URL as [link] with shortened display text (scheme://authority). |
| src/Aspire.Cli/Commands/DoctorCommand.cs | Uses [link] markup for prerequisites and environment-check links. |
| src/Aspire.Cli/Commands/DescribeCommand.cs | Sorts/formats endpoints and renders them using [link] markup. |
| src/Aspire.Cli/Aspire.Cli.csproj | Links the new shared file into the CLI build. |
Files not reviewed (2)
- src/Aspire.Cli/Resources/DoctorCommandStrings.Designer.cs: Language not supported
- src/Aspire.Cli/Resources/RootCommandStrings.Designer.cs: Language not supported
Comment on lines
+15
to
+26
| public static readonly HashSet<string> Schemes = new(StringComparer.OrdinalIgnoreCase) | ||
| { | ||
| "gopher", | ||
| "ws", | ||
| "wss", | ||
| "news", | ||
| "nntp", | ||
| "telnet", | ||
| "tcp", | ||
| "redis", | ||
| "rediss" | ||
| }; |
There was a problem hiding this comment.
Schemes is a mutable HashSet<string> exposed as a public static readonly field. Even though the containing type is internal, any code in the assembly can still add/remove entries at runtime, which can lead to hard-to-debug behavior changes and thread-safety issues. Consider making the set private/immutable (e.g., FrozenSet<string> or a private HashSet + helper methods like IsUnsupportedScheme / IsLinkableUrl).
Comment on lines
+303
to
307
| var endpoints = OrderUrls(snapshot.Urls.Where(e => !e.IsInternal)) | ||
| .Select(e => (e.Url, DisplayName: e.DisplayProperties?.DisplayName ?? "")) | ||
| .ToArray(); | ||
|
|
||
| return new ResourceDisplayState(displayName, snapshot.State, snapshot.HealthStatus, endpoints); |
There was a problem hiding this comment.
ResourceDisplayState is used for deduplication via Equals, but Endpoints is now an array. Record equality will compare arrays by reference, so the newly-created ToArray() will almost always make displayState unequal to the previous value even when endpoints didn't change, defeating the deduplication logic. Use a value type with structural equality (e.g., store a normalized string, use ImmutableArray + custom IEquatable, or implement Equals using SequenceEqual).
src/Aspire.Cli/Commands/PsCommand.cs
Outdated
Comment on lines
+218
to
+221
| if (!string.IsNullOrEmpty(appHost.DashboardUrl) && Uri.TryCreate(appHost.DashboardUrl, UriKind.Absolute, out var dashboardUri)) | ||
| { | ||
| var displayText = $"{dashboardUri.Scheme}://{dashboardUri.Authority}"; | ||
| dashboard = $"[link={Markup.Escape(appHost.DashboardUrl)}]{Markup.Escape(displayText)}[/]"; |
There was a problem hiding this comment.
If appHost.DashboardUrl is non-empty but fails Uri.TryCreate (unexpected format, missing scheme, etc.), the UI now shows - and drops the original value. It would be more robust to fall back to displaying the escaped raw string (without link markup) so users can still see/copy the URL even if it isn't parseable.
Suggested change
| if (!string.IsNullOrEmpty(appHost.DashboardUrl) && Uri.TryCreate(appHost.DashboardUrl, UriKind.Absolute, out var dashboardUri)) | |
| { | |
| var displayText = $"{dashboardUri.Scheme}://{dashboardUri.Authority}"; | |
| dashboard = $"[link={Markup.Escape(appHost.DashboardUrl)}]{Markup.Escape(displayText)}[/]"; | |
| if (!string.IsNullOrEmpty(appHost.DashboardUrl)) | |
| { | |
| if (Uri.TryCreate(appHost.DashboardUrl, UriKind.Absolute, out var dashboardUri)) | |
| { | |
| var displayText = $"{dashboardUri.Scheme}://{dashboardUri.Authority}"; | |
| dashboard = $"[link={Markup.Escape(appHost.DashboardUrl)}]{Markup.Escape(displayText)}[/]"; | |
| } | |
| else | |
| { | |
| // Fallback: show the raw dashboard URL as plain, escaped text when it cannot be parsed. | |
| dashboard = Markup.Escape(appHost.DashboardUrl); | |
| } |
Contributor
|
🎬 CLI E2E Test Recordings — 49 recordings uploaded (commit View recordings
📹 Recordings uploaded automatically from CI run #23529440751 |
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
Use Spectre.Console
[link]markup for all CLI URL output so terminal hyperlinks work correctly and aren't broken by line wrapping.Changes
[link]markup withDisplayNameas link text when present. Endpoints are sorted matching the dashboard order (SortOrder desc, https first, then by Name).[link]markup withscheme://authorityas display text (hides the login token path/querystring).[link]markup. URL extracted from resource string into code.[link]markup viaMarkupLine. URL extracted from resource string into code.KnownUnsupportedUrlSchemes: Extracted the browser-unsupported URL scheme deny-list fromResourceUrlHelpers.csinto a shared file undersrc/Shared/, now used by both CLI and Dashboard. Addedredisandredissto the list.Validation
ResourceUrlHelperstests passChecklist