-
Notifications
You must be signed in to change notification settings - Fork 887
Clarify Aspire update notification #17366
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -693,15 +693,20 @@ public void DisplayEmptyLine() | |
|
|
||
| private const string UpdateUrl = "https://aka.ms/aspire/update"; | ||
|
|
||
| public void DisplayVersionUpdateNotification(string newerVersion, string? updateCommand = null) | ||
| public void DisplayVersionUpdateNotification(string newerVersion, string? updateCommand = null, bool includeAppHostUpdateCommand = false) | ||
| { | ||
| // Write to stderr to avoid corrupting stdout when JSON output is used | ||
| _errorConsole.WriteLine(); | ||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.NewCliVersionAvailable, newerVersion.EscapeMarkup())); | ||
|
|
||
| if (includeAppHostUpdateCommand) | ||
| { | ||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateAppHostRunCommand, "aspire update")); | ||
| } | ||
|
Comment on lines
+700
to
+707
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can get the app host version from the backchannel. It's displayed in |
||
|
|
||
| if (!string.IsNullOrEmpty(updateCommand)) | ||
| { | ||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateRunCommand, updateCommand.EscapeMarkup())); | ||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.ToUpdateCliRunCommand, updateCommand.EscapeMarkup())); | ||
| } | ||
|
|
||
| _errorConsole.MarkupLine(string.Format(CultureInfo.CurrentCulture, InteractionServiceStrings.MoreInfoNewCliVersion, MarkupHelpers.SafeLink(this, UpdateUrl))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Append this to the end of "A new version of.."? 4 lines is visually heavy.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
|
|
||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -241,8 +241,12 @@ | |||||
| <value>[dim]For more information, see: {0}[/]</value> | ||||||
| <comment>Do not translate [dim]. Also leave [/] as-is. {0} is a pre-formatted link</comment> | ||||||
| </data> | ||||||
| <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> | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the AppHost wording to use use instead of |
||||||
| <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> | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated the CLI wording to use use instead of |
||||||
| <comment>Do not translate [dim]. Also leave [/] as-is. {0} is the command to run</comment> | ||||||
| </data> | ||||||
| <data name="UnbuildableAppHostsDetected" xml:space="preserve"> | ||||||
|
|
||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add to
StartCommand?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.