Space-separated subcommands #51
Conversation
Previously, `Main.run` considered the first item in `argv` array a command ID, for example, in the `['user:add', 'Peter']` array, the command was `user:add` and the rest were its args. This doesn't work with space-separated commands which produce `argv` array like this: ``` ['user', 'add', 'Peter'] ``` and would be parsed as a `user` command that gets `add` and `Peter` as args. The new implementation looks at `config.commandIDs` and tries to find the longest match with items from `argv`. For example, if `commandIDs` contained `user` and `user add` commands, the new implementation would invoke a `user add` command with `['Peter']` as args. If, on the other hand, `commandIds` only contained the `user` command, it would invoke a `user` command with `['add', 'Peter']` as args. A couple of notes: - It's up to @oclif/config (or possibly a plugin) to produce command IDs that contain spaces. As of @oclif/config@1.9.0, the command IDs contain a colon, e.g., `user:add`. - The change is backwards compatible. Argv `['user:add', 'Peter']` is still parsed as command ID `user:add` and `['Peter']` as args. - The command separator is currently hardcoded to be a space. If oclif introduced a new config option like `"separator": "<any_character>"`, the argv splitting logic should be updated. - The `splitArgv` function should be unit-tested. I didn't find a testing infrastructure in this package, will seek advice on how to best do it.
|
Thanks for the contribution! Before we can merge this, we need @borekb to sign the Salesforce.com Contributor License Agreement. |
Codecov Report
@@ Coverage Diff @@
## master #51 +/- ##
=========================================
+ Coverage 70.37% 73.07% +2.7%
=========================================
Files 4 5 +1
Lines 135 156 +21
Branches 28 32 +4
=========================================
+ Hits 95 114 +19
- Misses 24 26 +2
Partials 16 16
Continue to review full report at Codecov.
|
|
Sorry, total blindness, tests are obviously there :) |
|
Pushed some improvements but also realized that the algo is not entirely correct, for example, command IDs Update: this has been fixed by 42179f4. |
|
@borekb I'm very interested in seeing your PRs merged. Could you rebase on latest master so that they are ready whenever the maintainers are ready to (hopefully) approve them? 🤞 |
|
Hi @lots0logs, I don't think there's a big desire by the team to support this. If they change their minds, I'll be more than happy to help with PRs. |
|
@borekb Curious if you are using this at all in the wild? How hardened is the code? Would it be worth it to maintain a separate fork if the project owners aren't willing to support the efforts? I noticed Adobe open-sourced a cli based on oclif and they included space separated commands, but unfortunately their implementation isn't complete. All this to say I'd be happy to help maintain a separate fork if your interested in some help. |
|
@tcf909 Hi, we moved off of oclif so I won't be able to help. |
|
@oclif @heroku-cli I'm happy to champion this PR if you're willing to review it :) |
|
@G-Rath Plugins would have to support this too, as help and autocomplete are currently incompatible. I like this feature and want it (I wrote a hack to implement it), but we're prioritizing some help templating work first. |
This PR adds support for space-separated subcommands at the @oclif/command level. The support also needs to be added in @oclif/config or possibly via plugin, I'm not sure yet – I'll open a PR in @oclif/config to show one way to do it (UPDATE: PR created: oclif/config#64).
The change is rather small and backwards-compatible with colon-separated topics, here is a copy of the commit description:
Previously,
Main.runconsidered the first item inargvarray a command ID, for example, in the['user:add', 'Peter']array, the command wasuser:addand the rest were its args. This doesn't work with space-separated commands which produceargvarray like this:and would be parsed as a
usercommand that getsaddandPeteras args.The new implementation looks at
config.commandIDsand tries to find the longest match with items fromargv. For example, ifcommandIDscontaineduseranduser addcommands, the new implementation would invoke auser addcommand with['Peter']as args. If, on the other hand,commandIdsonly contained theusercommand, it would invoke ausercommand with['add', 'Peter']as args.A couple of notes:
user:add.['user:add', 'Peter']is still parsed as command IDuser:addand['Peter']as args."separator": "<any_character>", the argv splitting logic should be updated.ThesplitArgvfunction should be unit-tested. I didn't find a testing infrastructure in this package, will seek advice on how to best do it.Related: