Clarify Aspire update notification#17366
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 17366Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 17366" |
There was a problem hiding this comment.
Pull request overview
This PR clarifies the Aspire CLI “update available” shutdown banner by separating guidance for updating the AppHost (project packages) vs updating the Aspire CLI binary itself, reducing ambiguity for users.
Changes:
- Updates the notification text to always include an AppHost update instruction (
aspire update) plus a separate CLI update instruction (from detection/fallback). - Changes the standalone/non-dotnet-tool fallback recommendation from
aspire updatetoaspire update --self. - Updates unit tests and localization resources to reflect the new wording and commands.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/Aspire.Cli.Tests/Utils/CliUpdateNotificationServiceTests.cs | Updates test expectations to use aspire update --self for standalone archive installs. |
| tests/Aspire.Cli.Tests/Interaction/ConsoleInteractionServiceTests.cs | Updates notification output assertions to expect separate AppHost vs CLI guidance and to validate markup escaping with the new command. |
| src/Aspire.Cli/Utils/CliUpdateNotifier.cs | Changes the non-dotnet-tool fallback update command to aspire update --self. |
| src/Aspire.Cli/Interaction/ConsoleInteractionService.cs | Updates the displayed notification to show distinct AppHost and CLI update lines. |
| src/Aspire.Cli/Resources/InteractionServiceStrings.resx | Replaces the single “To update” string with separate AppHost and CLI update string resources. |
| src/Aspire.Cli/Resources/InteractionServiceStrings.Designer.cs | Updates generated resource accessors to match the new resource keys. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.cs.xlf | Adds new trans-units for AppHost/CLI update lines and removes the old combined one. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.de.xlf | Same as above for de. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.es.xlf | Same as above for es. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.fr.xlf | Same as above for fr. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.it.xlf | Same as above for it. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ja.xlf | Same as above for ja. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ko.xlf | Same as above for ko. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.pl.xlf | Same as above for pl. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.pt-BR.xlf | Same as above for pt-BR. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.ru.xlf | Same as above for ru. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.tr.xlf | Same as above for tr. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.zh-Hans.xlf | Same as above for zh-Hans. |
| src/Aspire.Cli/Resources/xlf/InteractionServiceStrings.zh-Hant.xlf | Same as above for zh-Hant. |
Files not reviewed (1)
- src/Aspire.Cli/Resources/InteractionServiceStrings.Designer.cs: Language not supported
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateCliRunCommand, updateCommand.EscapeMarkup())); | ||
| } | ||
|
|
||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.MoreInfoNewCliVersion, MarkupHelpers.SafeLink(this, UpdateUrl))); |
There was a problem hiding this comment.
Append this to the end of "A new version of.."? 4 lines is visually heavy.
There was a problem hiding this comment.
Updated. The AppHost guidance is now appended to the version line as (use aspire update for this AppHost), so the AppHost case is three lines instead of four.
| <data name="ToUpdateRunCommand" xml:space="preserve"> | ||
| <value>[dim]To update, run: {0}[/]</value> | ||
| <data name="ToUpdateAppHostRunCommand" xml:space="preserve"> | ||
| <value>[dim]To update this AppHost, run: {0}[/]</value> |
There was a problem hiding this comment.
| <value>[dim]To update this AppHost, run: {0}[/]</value> | |
| <value>[dim]To update this AppHost, use: {0}[/]</value> |
There was a problem hiding this comment.
Updated the AppHost wording to use use instead of
un.
| <comment>Do not translate [dim]. Also leave [/] as-is. {0} is the command to run</comment> | ||
| </data> | ||
| <data name="ToUpdateCliRunCommand" xml:space="preserve"> | ||
| <value>[dim]To update the Aspire CLI, run: {0}[/]</value> |
There was a problem hiding this comment.
| <value>[dim]To update the Aspire CLI, run: {0}[/]</value> | |
| <value>[dim]To update the Aspire CLI, use: {0}[/]</value> |
There was a problem hiding this comment.
Updated the CLI wording to use use instead of
un.
| private static readonly TimeSpan s_detachedStartupStabilityWindow = TimeSpan.FromSeconds(2); | ||
|
|
||
| protected override bool UpdateNotificationsEnabled => !_isDetachMode; | ||
| protected override bool IncludeAppHostUpdateCommandInUpdateNotification => _hasAppHostContext; |
There was a problem hiding this comment.
Added this for start too. AppHostLauncher now reports when it has selected an AppHost, so start includes the AppHost guidance only after that context is known.
| if (includeAppHostUpdateCommand) | ||
| { | ||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateAppHostRunCommand, "aspire update")); | ||
| } |
There was a problem hiding this comment.
This will only display if the CLI is out of date. And it will display if the CLI is out of date and the app host is up to date.
Really there should be seperate checks for the CLI version and app host version, and then the correct notifications are displayed based on each result.
There was a problem hiding this comment.
Yep ideally they'd be separate checks, but today we always ship them together. Today it won't display if the CLI is up to date but the apphost is not IIUC. I think this is a good incremental improvement though.
There was a problem hiding this comment.
We can get the app host version from the backchannel. It's displayed in aspire ps. We should be able to get it from the app host and compare it with either the installed version of the CLI. Only comparing it with the installed version makes sense because aspire update won't do anything if there is a newer version of the CLI until it is installed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
❓ CLI E2E Tests unknown — 95 passed, 0 failed, 5 unknown (commit View all recordings
📹 Recordings uploaded automatically from CI run #26260499367 |
|
Moving this to draft while we clarify some things. |
Description
The shutdown update notification currently only says to run
aspire update, which can make it unclear whether users are updating the AppHost or the CLI binary. This clarifies the notification so users see separate guidance for updating the AppHost and updating the Aspire CLI.User-facing usage
When an
aspire runsession ends and an Aspire update is available, the notification now distinguishes the commands:The fallback standalone CLI update recommendation now uses
aspire update --self, while dotnet tool installs still show their appropriatedotnet tool update ...command.Validation
./restore.cmddotnet build /t:UpdateXlf ./src/Aspire.Cli/Aspire.Cli.csprojdotnet test --project ./tests/Aspire.Cli.Tests/Aspire.Cli.Tests.csproj --no-launch-profile -- --filter-class "*.ConsoleInteractionServiceTests" --filter-class "*.CliUpdateNotificationServiceTests" --filter-not-trait "quarantined=true" --filter-not-trait "outerloop=true"Fixes # (issue)
Checklist
<remarks />and<code />elements on your triple slash comments?