-
Notifications
You must be signed in to change notification settings - Fork 61
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
feat: add CLI #161
feat: add CLI #161
Conversation
`parseArgs` is used as a command line argument parser, because it polyfills an experimental Node.js API. Eventually this means this dependency can be removed. `@types/node` was updated to version 18 in order to provide types for `@pkgjs/parseargs`. Closes microsoft#81
It’s unused.
lib/bin.ts
Outdated
|
||
Options | ||
--vscode-executable-path The VS Code executable path used for testing. | ||
--version The VS Code version to download. |
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.
This flag is a bit weird IMO. --version
usually prints the version of a tool, but in this case it matches the version
option of runTests
. Any suggestions?
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.
I would probably rename it to --vscode-version
. PS: I'm not a maintainer.
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.
I agree. I’m glad to see someone mention this, regardless whether you’re a maintainer or not. :)
I changed it to --vscode-version
, and repurposed the --version
flag to print version information.
Ping @connor4312! Given your recent contributions I thought maybe you had overseen this PR. |
Also the `--version` flag was repurposed to print the package version.
'vscode-version': { type: 'string' }, | ||
platform: { type: 'string' }, | ||
'reuse-machine-install': { type: 'boolean' }, | ||
'extension-development-path': { type: 'string', multiple: true }, |
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.
Did you intentionally miss this one in help? Also, is it intentionally made "multiple"?
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.
Good catch! I accidentally missed that one.
It’s multiple
, because runTests()
accepts it as an array.
Thanks for the PR! I opted to go with a bit more involved approach in #235 Namely, the CLI 'mandates' a Mocha setup that we'll be able to use to have UI integration for running extension tests. This solves a problem with the current test setup where the runner is entirely user-defined, with unknown ways of handling arguments or filtering tests. This lack of prescription, while good to give power to users (and not an interface we'll ever remove), meant we couldn't really build anything on top of it. I wanted to take the CLI as an opportunity to solve this problem. Again, thanks for your contribution of code and ideas, and I hope you can provide some feedback on the alternative approach above 🙂 |
parseArgs
is used as a command line argument parser, because it polyfills an experimental Node.js API. Eventually this means this dependency can be removed.Closes #81