refactor(cli): cleanup Command.ts #757
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This is a comprehensive refactoring of the CLI module focused on simplifying the API and improving type safety. The PR removes unnecessary type widening, fixes --version to have global precedence (git/npm style), and renames HelpFormatter to CliOutput for better clarity. It also introduces internal separation of concerns by splitting Command implementation into separate modules (command.ts, config.ts).
Key changes:
- Simplified
withSubcommandsto accept an array instead of variadic arguments, removing type casting - Fixed
--version/--helpto work anywhere in command invocation (global precedence) - Renamed
HelpFormattertoCliOutputanddefaultHelpRenderertodefaultFormatter - Refactored internal command structure with new
config.tsandcommand.tsmodules
Reviewed changes
Copilot reviewed 33 out of 33 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| Command.ts | Major refactoring: simplified API, extracted internal logic to command.ts, updated to use array-based withSubcommands |
| CliOutput.ts | Renamed from HelpFormatter.ts, added formatErrors method for batch error display |
| parser.ts | Added extensive documentation, restructured for clarity, implements --version global precedence |
| command.ts (new) | Internal implementation module containing command construction and validation logic |
| config.ts (new) | Internal module for parsing and reconstructing nested config structures |
| Param.ts | Renamed Argument/Flag constants to argumentKind/flagKind, improved type variance |
| Flag.ts | Updated to use new Param constants, renamed withPseudoName → withMetavar, removed repeated combinator |
| Argument.ts | Updated to use new Param constants, added missing combinators (filter, filterMap, orElse, etc.) |
| Primitive.ts | Renamed fileString → fileText, keyValueMap → keyValuePair, added isBoolean helper |
| Test files | Updated all test files to use array syntax for withSubcommands and new CliOutput API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Rename FlagSpec → FlagRegistry, FlagBag → FlagAccumulator - Add explicit ParseMode and FirstValueResult discriminated unions - Break scanCommandLevel into processFlag, processValue, resolveFirstValue - Remove vestigial leadingArguments field - Rename ParsedConfig/InputSpec → Command.Config.Internal - Move CommandConfig → Command.Config namespace - Move InferConfig → Command.Config.Infer - Add isFlagParam helper to Param.ts - Extract config parsing to internal/config.ts
- Remove static completion generators (bash.ts, fish.ts, zsh/) - --completions now generates dynamic shims that call binary at runtime - Extract CommandInternal, toImpl, makeCommand to internal/command.ts - makeCommand only accepts parsed ConfigInternal - Remove getHelpDoc from public API
- Change from variadic to array parameter: withSubcommands([a, b]) - Add showHelp helper to deduplicate error handling - Remove unnecessary CliError cast - Update all test files to use new array syntax
- Rewrite withSubcommands with clearer structure and minimal casts - Add TODO comment on prompt constructor semantic mismatch - Use explicit type aliases for NewInput, NewService - Document remaining casts with explanatory comments
Remove UserError wrap/unwrap dance. Use Effect.catch with instanceof check to handle ShowHelp errors directly.
- Extract commandPath computation once - Group built-in flag handlers together - Use return yield* for early exits - Consolidate parsing error handling
Replace recursive conditional types with simpler T[number] pattern. 18 lines → 6 lines for the same functionality.
- Fix withHandler JSDoc to use array syntax for withSubcommands - Remove redundant args variable in runWith - Yield HelpRenderer once at start of built-in handling - Extract mapHandler helper to consolidate provide* functions
- HelpFormatter -> CliOutput (formats help, errors, version) - HelpRenderer -> Formatter (service interface) - defaultHelpRenderer -> defaultFormatter - RawInput -> ParsedTokens (clearer purpose) - ParentCommand -> CommandContext (describes service role)
- Remove NewInput type widening from withSubcommands return type - Track subcommand info internally via _subcommand, not in public type - Eliminate 4 type casts (as NewInput, as Input, as NewService) - Fix tests to use yield* parent instead of yield* parent.service - Add TODO for process.argv browser compatibility - Simplify withHandler docstring example
- CommandContext: explain how to access parent config via yield* - withSubcommands: show parent context access pattern - prompt: add docstring with example - provide: add example showing input-dependent layer
…mbinators - Make Param module internal (not exported from index.ts) - Fix Flag.none passing string instead of Param.Flag constant - Rename fileString to fileText for consistency - Fix @SInCE tags in Flag.ts (1.0.0 → 4.0.0) - Fix mapTryCatch using hardcoded "unknown" for option name - Add missing combinators to Argument: choiceWithValue, withPseudoName, filter, filterMap, orElse, orElseResult - Fix optional @category: constructors → combinators - Add architecture documentation to Param.ts
- Add Command.Any type alias for type-safe command navigation - Extract findMatchingFlag helper to eliminate 3x duplication in handler.ts - Remove all `as any` casts from handler.ts and internal/command.ts
- parseArgs now accepts Command.Any instead of being generic - Removed generics from CommandContext and internal functions - Eliminated `as unknown as` cast for subcommand recursion
- Add formatErrors to CliOutput.Formatter for multiple error display - Update showHelp() to accept error array instead of single error - Pass all accumulated errors in runWith (was only showing first) - Improve createFlagRegistry error messages - Add comprehensive error tests
- findMatchingFlag -> lookupFlag - SingleFlagMeta -> FlagDescriptor - parseOption -> parseFlag - Add Primitive.isBoolean() helper
- Remove deprecated: keyValueMap, repeated, withPseudoName, Argument/Flag constants - Fix AnyArgument/AnyFlag to use argumentKind/flagKind - Make CliError.TypeId internal - Normalize @category tags to lowercase
kitlangton
force-pushed
the
cli-cleanup
branch
from
800892b to
2455735
Compare
📊 JSDoc Documentation Analysis📈 Current Analysis ResultsThis comment is automatically updated on each push. View the analysis script for details. |
Bundle Size Analysis
|
Remove handler from root command in test fixture - commands without handlers should show help by default, which the framework already supported but wasn't being tested.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
withSubcommands: remove type widening andascasts--versionto have global precedence (git/npm style)T[number]patternHelpFormattertoCliOutputCommandContext,withSubcommands,prompt,provideprocess.argvbrowser compatibilityTest plan
--versionglobal precedence