-
Notifications
You must be signed in to change notification settings - Fork 0
Convert to System.CommandLine
#276
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
Conversation
6b146fe to
9049d28
Compare
9049d28 to
b828c3d
Compare
b828c3d to
43b3da7
Compare
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.
Pull Request Overview
This PR converts the command infrastructure from Spectre.Console.Cli to System.CommandLine in order to support AOT and simplify the CLI setup.
- Removed legacy Spectre.Console.Cli-based settings and registrations.
- Introduced System.CommandLine-based commands and common options across commands.
- Updated package references and refactored command handler invocations accordingly.
Reviewed Changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Stack/Stack.csproj | Updated package references to replace Spectre.Console.Cli with System.CommandLine and Spectre.Console. |
| src/Stack/Program.cs | Replaced CommandApp configuration with a StackRootCommand setup using System.CommandLine. |
| src/Stack/Infrastructure/Commands/* | Converted all commands from the old CLI pattern to the new System.CommandLine API. |
| Other command files | Updated command option definitions and handler invocations to use ParseResult from System.CommandLine. |
Comments suppressed due to low confidence (1)
src/Stack/Commands/Stack/NewStackCommand.cs:28
- [nitpick] The option identifier 'StackName' might be confusing since this command creates a new stack; consider renaming it to 'Name' to remain consistent with its usage.
static readonly Option<string?> StackName = new("--name", "-n")
| private void WriteOutput(bool json, TResponse response) | ||
| { | ||
| if (settings.Json) | ||
| if (json) |
Copilot
AI
Jun 22, 2025
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.
[nitpick] Consider renaming the 'json' parameter to 'isJsonOutput' or a similar more descriptive name to clearly indicate it is a flag controlling the output format.
43b3da7 to
f5639f6
Compare
System.CommandLine
f5639f6 to
3edd2df
Compare
3edd2df to
b5ef711
Compare
b5ef711 to
269388a
Compare
269388a to
8e0684f
Compare
Converts from
Spectre.Console.ClitoSystem.CommandLine@2.0.0-beta5now that it has a path to a stable version, paving the way for enabling AOT.A few changes with this:
--branch.Fixes #277
Part of a series of PRs that are adding support for AOT:
System.CommandLine#276Spectre.Console.Cli0.50.0 #280Octopus.Shellfishwith directly usingProcess#284Humanizer.Corev3 beta #286Humanizer#290--working-directoryalias #298