feat(commander): support positional argument completions#129
Conversation
|
commit: |
|
@shadowspawn your review is very appreciated! Thanks in advance ❤️ so commander already exposes what we need on each command |
| function processArguments(tabCommand: TabCommand, cmd: CommanderCommand): void { | ||
| for (const arg of cmd.registeredArguments) { | ||
| const choices = arg.argChoices; | ||
| if (choices?.length) { |
There was a problem hiding this comment.
Ho ho! Support for completing choices, nice.
| ]) | ||
| ); | ||
|
|
||
| program.argument('[project]', 'Project name'); |
There was a problem hiding this comment.
You can not directly mix arguments and subcommands on same command in current Commander. While the completion works, the runtime does not. (Can achieve it with a default command, but more advanced and would require working out how to feed the default command into tab!)
Particularly for an example file, I suggest arguments on a leaf command are cleaner. As the lint command has.
% pnpm tsx ./examples/demo.commander.ts complete --
dev Start dev server
serve Start the server
build Build the project
deploy Deploy the application
lint Lint source files
copy Copy files
my-app My application
my-lib My library
my-tool My tool
:4
% pnpm tsx ./examples/demo.commander.ts my-lib
error: unknown command 'my-lib'
% pnpm tsx ./examples/demo.commander.ts complete -- lint ""
main.ts Main file
index.ts Index file
:4
% pnpm tsx ./examples/demo.commander.ts lint main.ts
Linting files...
| processArguments(t, command); | ||
| } | ||
|
|
||
| function processArguments(tabCommand: TabCommand, cmd: CommanderCommand): void { |
There was a problem hiding this comment.
The processArguments integration is very straight-forward and makes good use of Commander. Nice!
|
|
||
| describe.each(cliTools)('cli completion tests for %s', (cliTool) => { | ||
| // Commander does not have custom completions for arguments yet. | ||
| const shouldSkipTest = cliTool === 'commander'; |
There was a problem hiding this comment.
Goodbye test suppression. Woop!
|
The integration looks clean and good. Adding a root argument to the demo when there are subcommands is not actually functional with Commander. (Of interest. The |
in this pr commander adapter now read the positional args. same as the other two adapters we have which finally makes us able not to skip the tests for this case.
before this PR, commander completions only worked for flags.