Update CLI confirmation prompts to use [Y/n] convention#15663
Conversation
Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/a62011ab-6d8c-4a64-b8f2-13ab5a7aaddf Co-authored-by: maddymontaquila <12660687+maddymontaquila@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Updates Aspire CLI confirmation prompts to follow the common convention of capitalizing the default choice ([Y/n] or [y/N]) instead of Spectre.Console’s default [y/n] (y) rendering.
Changes:
- Replaces
IAnsiConsole.ConfirmAsync(...)with a manually-configuredConfirmationPrompt. - Hides Spectre’s built-in choices/default display and prepends a custom
[Y/n]or[y/N]suffix to the prompt text.
| // Use [Y/n] or [y/N] convention where the capitalized letter indicates the default value. | ||
| var yesChoice = defaultValue ? "Y" : "y"; | ||
| var noChoice = defaultValue ? "n" : "N"; | ||
| var fullPromptText = $"{promptText} [{yesChoice}/{noChoice}]"; |
There was a problem hiding this comment.
fullPromptText embeds [Y/n] using single brackets, but Spectre.Console treats [...] as markup tags. This will render incorrectly and can throw InvalidMarkupException because there’s no closing tag. Escape the choice suffix (e.g., use doubled brackets [[...]] or otherwise ensure the brackets are treated as literal text).
| var fullPromptText = $"{promptText} [{yesChoice}/{noChoice}]"; | |
| var fullPromptText = $"{promptText} [[{yesChoice}/{noChoice}]]"; |
| // Use [Y/n] or [y/N] convention where the capitalized letter indicates the default value. | ||
| var yesChoice = defaultValue ? "Y" : "y"; | ||
| var noChoice = defaultValue ? "n" : "N"; | ||
| var fullPromptText = $"{promptText} [{yesChoice}/{noChoice}]"; | ||
|
|
||
| var prompt = new ConfirmationPrompt(fullPromptText) | ||
| { | ||
| ShowChoices = false, | ||
| ShowDefaultValue = false, | ||
| }; | ||
|
|
||
| var result = await MessageConsole.PromptAsync(prompt, cancellationToken); |
There was a problem hiding this comment.
This change alters interactive prompt rendering/behavior but there are no tests validating the new [Y/n] formatting or the default-value semantics (Enter should select the configured default). Consider adding a focused unit test around ConfirmAsync similar to the existing ConsoleInteractionServiceTests patterns to prevent regressions.
|
This is inconsistent with text inputs. They display the default in parenthesis. Is it normal for CLIs to have different standards between text inputs and Yes/No. Also, this isn't localized. What about other languages? |
|
@copilot apply changes based on the comments in this thread |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 15663Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 15663" |
Agent-Logs-Url: https://github.com/microsoft/aspire/sessions/c9c4e590-0aea-4cab-8872-baae7dfcdbed Co-authored-by: maddymontaquila <12660687+maddymontaquila@users.noreply.github.com>
Applied all three fixes from the review thread in commit 996d350:
The Regarding localization: the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Text inputs should be treated separately, yeah. Y/n or y/N is the standard in like all linux/unix scripts so for boolean answers we should standadrdize on that |
|
📄 Documentation Check: No documentation updates required. This PR changes CLI confirmation prompt formatting from
|
Description
CLI confirmation prompts showed default values as
[y/n] (y):— a non-standard format. The convention is to capitalize the default choice:[Y/n].Before:
Perform updates? [y/n] (y):After:
Perform updates? [Y/n]:Changes
ConsoleInteractionService.ConfirmAsync: ReplacedIAnsiConsole.ConfirmAsync(which renders Spectre's[y/n] (y)format) with aConfirmationPromptthat hasShowChoices = falseandShowDefaultValue = false, with[Y/n]or[y/N]manually appended to the prompt text based ondefaultValue. Brackets are escaped using Spectre.Console's[[...]]syntax to produce literal bracket characters.DefaultValueis explicitly set on the prompt so pressing Enter returns the correct configured default.ConfirmAsync_DisplaysCapitalizedDefaultChoiceto verify that[Y/n]or[y/N]appears in the prompt output based on the default value. AddedConfirmAsync_WhenUserPressesEnter_ReturnsDefaultValueto verify that pressing Enter accepts the configured default. Both tests use a newTestAnsiConsoleWithInputhelper that feeds predefined keystrokes so prompts don't block on real console input.Checklist
⌨️ Start Copilot coding agent tasks without leaving your editor — available in VS Code, Visual Studio, JetBrains IDEs and Eclipse.