Conversation
- Introduce IAbstractCommand and CommandLineRoot abstractions - Replace legacy pipeline with robust middleware system (Pipeline.cs) - Add CommandLineConfiguration for middleware & handler config - Refactor CommandLineContext to remove System.CommandLine deps - Add reflection-based command/option registration (AbstractExtensions, CommandLineBuilder<T>) - Support option aliases in OptionAttribute - Replace RunConsoleCommands<T> with RunCommandLine<T> - Add sample project with commands, handler, and middleware - Remove obsolete HostBuilderExtensions, CommandLineBuilderConfiguration, and MIDDLEWARE.md - Update solution/projects for .NET 10.0 and new sample
Enable global customization of help output via UseCommandLineHelp extension. Introduce ConfigureHelpDelegate in CommandLineConfiguration and update CommandLineBuilder to assign custom help actions recursively. Add CustomHelpAction example to demonstrate extending help behavior. This allows flexible, application-wide help customization for command-line apps.
There was a problem hiding this comment.
Pull request overview
Updates Arcturus.Extensions.CommandLine to a newer System.CommandLine version and refactors the CLI execution pipeline/configuration model, including a new sample project.
Changes:
- Upgrade System.CommandLine dependency and refactor command execution to use a new internal
CommandLineBuilder<T>and middlewarePipeline. - Introduce
CommandLineConfigurationfor handler lifetime, default exception handling, middleware registration, and help customization. - Add a new
Arcturus.CommandLine.Sampleproject and wire it into the solution.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Arcturus.Extensions.CommandLine/ServiceCollectionExtensions.cs | Adds CommandLineConfiguration + handler auto-registration changes |
| src/Arcturus.Extensions.CommandLine/MIDDLEWARE.md | Removes middleware pipeline documentation file |
| src/Arcturus.Extensions.CommandLine/Internals/Pipeline.cs | New middleware pipeline builder |
| src/Arcturus.Extensions.CommandLine/Internals/CommandLineBuilderConfiguration.cs | Removes old builder configuration container |
| src/Arcturus.Extensions.CommandLine/Internals/CommandLineBuilder.cs | New command graph builder + option binding + handler invocation |
| src/Arcturus.Extensions.CommandLine/Internals/AbstractExtensions.cs | New reflection helpers for attributes/options |
| src/Arcturus.Extensions.CommandLine/HostExtensions.cs | Replaces old run method and middleware/help registration entry points |
| src/Arcturus.Extensions.CommandLine/HostBuilderExtensions.cs | Removes old HostBuilder commandline builder customization |
| src/Arcturus.Extensions.CommandLine/CommandLineContext.cs | Moves CommandLineContext and updates surface (removes Parser) |
| src/Arcturus.Extensions.CommandLine/CommandLineConfiguration.cs | New configuration object for CLI behavior |
| src/Arcturus.Extensions.CommandLine/Arcturus.Extensions.CommandLine.csproj | Updates/conditions package references per TFM |
| src/Arcturus.Extensions.CommandLine/Abstractions/OptionAttribute.cs | Adds aliases support to options |
| src/Arcturus.Extensions.CommandLine/Abstractions/ICommand.cs | Makes ICommand derive from IAbstractCommand |
| src/Arcturus.Extensions.CommandLine/Abstractions/IAbstractCommand.cs | Adds new marker abstraction |
| src/Arcturus.Extensions.CommandLine/Abstractions/CommandLineRoot.cs | Adds new root base class |
| samples/Arcturus.CommandLine.Sample/Services/MessageService.cs | Adds sample service |
| samples/Arcturus.CommandLine.Sample/Program.cs | Adds sample program wiring command line + middleware + help |
| samples/Arcturus.CommandLine.Sample/Middleware/LogMiddleware.cs | Adds sample middleware |
| samples/Arcturus.CommandLine.Sample/CustomHelpAction.cs | Adds sample custom help action |
| samples/Arcturus.CommandLine.Sample/Commands/SampleCommand.cs | Adds sample command + handler |
| samples/Arcturus.CommandLine.Sample/Commands/CommandRoot.cs | Adds sample root command |
| samples/Arcturus.CommandLine.Sample/Arcturus.CommandLine.Sample.csproj | Adds sample project file |
| Arcturus.sln | Adds solution folder + sample project; modifies header formatting |
Comments suppressed due to low confidence (2)
src/Arcturus.Extensions.CommandLine/CommandLineContext.cs:4
- CommandLineContext was moved to namespace Arcturus.CommandLine, but abstractions like ICommandLineMiddleware and CommandLineDelegate (in Arcturus.CommandLine.Abstractions) still reference CommandLineContext without qualifying/using the new namespace. This will fail to compile (type cannot be found) and is also a public breaking change for consumers. Either keep CommandLineContext in the Abstractions namespace or update those abstractions (and any consumers) to reference the new namespace explicitly.
src/Arcturus.Extensions.CommandLine/ServiceCollectionExtensions.cs:36 - AppDomain.CurrentDomain.GetAssemblies().SelectMany(a => a.GetTypes()) can throw ReflectionTypeLoadException (or other loader exceptions) at startup if any loaded assembly can’t be fully reflected. Consider using a safe GetTypes wrapper (catching ReflectionTypeLoadException and returning the successfully loaded Types) to avoid crashing handler auto-registration.
var handlerTypes = AppDomain.CurrentDomain.GetAssemblies()
.SelectMany(a => a.GetTypes())
.Where(t => t.GetInterfaces().Any(i =>
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| var handlerType = typeof(ICommandHandler<>).MakeGenericType(subCommandType); | ||
| var handler = services.GetService(handlerType); | ||
| if (handler is not null && handlerType.TryGetMethod("Handle", out var methodHandler)) | ||
| { | ||
| // Pre-compile handler invocation — eliminates MethodInfo.Invoke at parse time | ||
| var handlerInvoker = BuildHandlerInvoker(handler, methodHandler, subCommandType); | ||
|
|
There was a problem hiding this comment.
The handler is resolved once during command registration (services.GetService(handlerType)) and then captured in a compiled delegate. This effectively turns every handler into a singleton, ignoring the configured DI lifetime (HandlerLifeTime) and can also break scoped/transient dependencies and disposal. Resolve the handler inside the command action (ideally from a scope created per invocation) so DI lifetimes behave as intended.
| wrappedCommand.SetAction(async (pr, ct) => | ||
| { | ||
| foreach (var setter in optionSetters) | ||
| setter(command, pr); | ||
|
|
||
| await handlerInvoker(command, cancellationToken); | ||
| }); |
There was a problem hiding this comment.
wrappedCommand.SetAction provides a per-invocation CancellationToken (ct), but the handler invocation uses the outer cancellationToken parameter instead. This prevents cancellation triggered by the command invocation (e.g., Ctrl+C / invocation token) from flowing to handlers. Pass ct through (or link it with the outer token if you need both) when calling the handler.
| private void AssignHelpRecrusive(Command commandLineRoot) | ||
| { |
There was a problem hiding this comment.
Method name "AssignHelpRecrusive" appears to be a misspelling of "AssignHelpRecursive". Renaming to the correct spelling will improve readability and searchability (and keep naming consistent with the intended meaning).
| private void AssignCommandsRecrusive( | ||
| IAbstractCommand parentCommand | ||
| , Command commandLineCommand | ||
| , IServiceProvider services | ||
| , CancellationToken cancellationToken) |
There was a problem hiding this comment.
Method name "AssignCommandsRecrusive" appears to be a misspelling of "AssignCommandsRecursive". Consider renaming for clarity and to avoid propagating the typo across call sites.
| /// <summary> | ||
| /// Gets or sets the service lifetime used for the handler instance. | ||
| /// </summary> | ||
| /// <remarks>The handler's lifetime determines how long the handler instance is retained by the dependency | ||
| /// injection container. Changing this value affects the handler's reuse and disposal behavior.</remarks> | ||
| public ServiceLifetime HandlerLifeTime { get; set; } = ServiceLifetime.Transient; | ||
| /// <summary> |
There was a problem hiding this comment.
Public property name "HandlerLifeTime" is likely intended to be "HandlerLifetime" (consistent with ServiceLifetime naming). Since this is part of the public API surface, consider correcting it now (or providing an obsolete alias) to avoid locking in a typo for consumers.
| public static async Task RunCommandLine<T>(this IHost host, string[] args) | ||
| where T : CommandLineRoot | ||
| { |
There was a problem hiding this comment.
RunConsoleCommands() was removed/renamed to RunCommandLine() (and the generic constraint changed). Since HostExtensions is a public API, this is a breaking change for existing consumers. If backward compatibility is expected, consider keeping RunConsoleCommands as an [Obsolete] wrapper that forwards to the new implementation (or document the breaking change as part of a major version bump).
…s.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…te.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary & Motivation
A brief description of the changes in this pull request explaining why these changes are necessary. Please delete this paragraph.
Checklist