diff --git a/docs/cli/options.md b/docs/cli/options.md index a64bf04eb..5989de05d 100644 --- a/docs/cli/options.md +++ b/docs/cli/options.md @@ -15,15 +15,15 @@ For the latest full list of supported options, see `CliOptions` [in this file](h The options below apply to most CLI commands. -| Option | Alias | Default | Description | -| --------------- | ----- | ------------------------- | -------------------------------------------------------------------------------------------- | -| `--branch, -b` | `-b` | | target branch; see [config docs][1] for details | -| `--config-path` | `-c` | [cosmiconfig][2] defaults | custom beachball config path | -| `--no-fetch` | | | skip fetching from the remote | -| `--change-dir` | | `'change'` | name of the directory to store change files | -| `--scope` | | | only consider matching package paths (can be specified multiple times); see [config docs][3] | -| `--since` | | | only consider changes or change files since this git ref (branch name, commit SHA) | -| `--verbose` | | | prints additional information to the console | +| Option | Alias | Default | Description | +| -------------- | ----- | ------------------------- | -------------------------------------------------------------------------------------------- | +| `--branch, -b` | `-b` | | target branch; see [config docs][1] for details | +| `--config` | `-c` | [cosmiconfig][2] defaults | custom beachball config path (alias: `--config-path`) | +| `--no-fetch` | | | skip fetching from the remote | +| `--change-dir` | | `'change'` | name of the directory to store change files | +| `--scope` | | | only consider matching package paths (can be specified multiple times); see [config docs][3] | +| `--since` | | | only consider changes or change files since this git ref (branch name, commit SHA) | +| `--verbose` | | | prints additional information to the console | [1]: ../overview/configuration#determining-the-target-branch-and-remote [2]: https://www.npmjs.com/package/cosmiconfig diff --git a/package.json b/package.json index be8404f1f..c0a4acf28 100644 --- a/package.json +++ b/package.json @@ -53,6 +53,7 @@ }, "dependencies": { "@vercel/detect-agent": "^1.2.1", + "commander": "^10.0.1", "cosmiconfig": "^9.0.0", "execa": "^5.0.0", "minimatch": "^3.0.4", @@ -60,8 +61,7 @@ "p-limit": "^3.0.2", "prompts": "^2.4.2", "semver": "^7.0.0", - "workspace-tools": "^0.41.0", - "yargs-parser": "^21.0.0" + "workspace-tools": "^0.41.0" }, "devDependencies": { "@jest/globals": "^29.0.0", @@ -70,7 +70,6 @@ "@types/prompts": "^2.4.2", "@types/semver": "^7.3.13", "@types/tmp": "^0.2.3", - "@types/yargs-parser": "^21.0.0", "@typescript-eslint/eslint-plugin": "^5.0.0", "@typescript-eslint/parser": "^5.0.0", "@typescript-eslint/utils": "^5.0.0", diff --git a/src/__e2e__/bump.test.ts b/src/__e2e__/bump.test.ts index b5dcc2703..fecfd90e2 100644 --- a/src/__e2e__/bump.test.ts +++ b/src/__e2e__/bump.test.ts @@ -37,7 +37,7 @@ describe('bump command', () => { function getOptions(repoOptions?: Partial, cwd?: string) { const parsedOptions = getParsedOptions({ cwd: cwd || repo?.rootPath || '', - argv: [], + argv: ['node', 'beachball', 'bump'], testRepoOptions: { branch: defaultRemoteBranchName, fetch: false, ...repoOptions }, }); return { options: parsedOptions.options, parsedOptions }; diff --git a/src/__e2e__/getChangedPackages.test.ts b/src/__e2e__/getChangedPackages.test.ts index cd51a4112..b4a94caad 100644 --- a/src/__e2e__/getChangedPackages.test.ts +++ b/src/__e2e__/getChangedPackages.test.ts @@ -30,7 +30,7 @@ describe('getChangedPackages (basic)', () => { function getChangedPackagesWrapper(options?: Partial) { const parsedOptions = getParsedOptions({ cwd: reusedRepo.rootPath, - argv: [], + argv: ['node', 'beachball', 'change'], testRepoOptions: { fetch: false, branch: defaultRemoteBranchName, diff --git a/src/__e2e__/validate.test.ts b/src/__e2e__/validate.test.ts index c5be8103f..18286f8ae 100644 --- a/src/__e2e__/validate.test.ts +++ b/src/__e2e__/validate.test.ts @@ -15,7 +15,7 @@ describe('validate', () => { function validateWrapper(validateOptions?: ValidateOptions) { const parsedOptions = getParsedOptions({ cwd: repo!.rootPath, - argv: [], + argv: ['node', 'beachball', 'check'], testRepoOptions: { branch: defaultRemoteBranchName, }, diff --git a/src/__functional__/changefile/readChangeFiles.test.ts b/src/__functional__/changefile/readChangeFiles.test.ts index 5a8c6eda6..fe6ce06dd 100644 --- a/src/__functional__/changefile/readChangeFiles.test.ts +++ b/src/__functional__/changefile/readChangeFiles.test.ts @@ -30,7 +30,7 @@ describe('readChangeFiles', () => { expect(cwd).toBeTruthy(); const parsedOptions = getParsedOptions({ cwd: cwd!, - argv: [], + argv: ['node', 'beachball', 'change'], testRepoOptions: { branch: defaultRemoteBranchName, ...repoOptions }, }); const packageInfos = getPackageInfos(parsedOptions); diff --git a/src/__functional__/changelog/writeChangelog.test.ts b/src/__functional__/changelog/writeChangelog.test.ts index a04583a62..f6f72e2dd 100644 --- a/src/__functional__/changelog/writeChangelog.test.ts +++ b/src/__functional__/changelog/writeChangelog.test.ts @@ -37,7 +37,7 @@ describe('writeChangelog', () => { function getOptionsAndPackages(repoOptions?: Partial, cwd?: string) { const parsedOptions = getParsedOptions({ cwd: cwd || repo?.rootPath || '', - argv: [], + argv: ['node', 'beachball', 'bump'], testRepoOptions: { branch: defaultRemoteBranchName, ...repoOptions }, }); const packageInfos = getPackageInfos(parsedOptions); diff --git a/src/__functional__/options/getCliOptions.test.ts b/src/__functional__/options/getCliOptions.test.ts index 09b005c0e..ecdbaa62d 100644 --- a/src/__functional__/options/getCliOptions.test.ts +++ b/src/__functional__/options/getCliOptions.test.ts @@ -11,16 +11,12 @@ jest.mock('workspace-tools', () => { // // These tests cover a mix of built-in parser behavior, provided options, and custom overrides. -// It's worth having tests for relevant built-in behaviors in case we change parsers in the future -// (likely to commander), to ensure there are no undocumented breaking changes from the beachball -// "end user" perspective. // describe('getCliOptions', () => { // This is the same mocked value as above (can't be shared in a const because jest.mock() is // not allowed to access the surrounding context) const projectRoot = 'fake-root'; const mockFindProjectRoot = findProjectRoot as jest.MockedFunction; - const defaults = { command: 'change', path: projectRoot }; /** test wrapper for `getCliOptions` which adds common args */ function getCliOptionsTest(args: string[], cwd?: string) { @@ -39,195 +35,232 @@ describe('getCliOptions', () => { expect(findProjectRoot(process.cwd())).toEqual(projectRoot); }); - it('parses no args (adds path to result)', () => { - const options = getCliOptionsTest([]); - expect(options).toEqual(defaults); + it('requires a command', () => { + expect(() => getCliOptionsTest([])).toThrow(); }); it('parses command', () => { const options = getCliOptionsTest(['check']); - expect(options).toEqual({ ...defaults, command: 'check' }); + expect(options).toEqual({ command: 'check', path: projectRoot }); }); it('parses options', () => { // use a basic option of each value type (except arrays, tested later) - const options = getCliOptionsTest(['--type', 'patch', '--access=public', '--fetch', '--depth', '1']); - expect(options).toEqual({ ...defaults, type: 'patch', access: 'public', fetch: true, depth: 1 }); + const options = getCliOptionsTest(['change', '--type', 'patch', '--access=public', '--fetch', '--depth', '1']); + expect(options).toEqual({ + command: 'change', + path: projectRoot, + type: 'patch', + access: 'public', + fetch: true, + depth: 1, + }); }); it('parses command and options', () => { const options = getCliOptionsTest(['publish', '--tag', 'foo']); - expect(options).toEqual({ ...defaults, command: 'publish', tag: 'foo' }); + expect(options).toEqual({ command: 'publish', path: projectRoot, tag: 'foo' }); }); it('parses array options with multiple values', () => { - const options = getCliOptionsTest(['--scope', 'foo', 'bar']); - expect(options).toEqual({ ...defaults, scope: ['foo', 'bar'] }); + const options = getCliOptionsTest(['change', '--scope', 'foo', 'bar']); + expect(options).toEqual({ command: 'change', path: projectRoot, scope: ['foo', 'bar'] }); }); it('parses array option specified multiple times', () => { - const options = getCliOptionsTest(['--scope', 'foo', '--scope', 'bar']); - expect(options).toEqual({ ...defaults, scope: ['foo', 'bar'] }); + const options = getCliOptionsTest(['change', '--scope', 'foo', '--scope', 'bar']); + expect(options).toEqual({ command: 'change', path: projectRoot, scope: ['foo', 'bar'] }); }); // documenting that this is not currently supported (could change in the future if desired) it('does not parse values with commas as separate array entries', () => { - const options = getCliOptionsTest(['--scope', 'a,b', '--scope=c,d']); - expect(options).toEqual({ ...defaults, scope: ['a,b', 'c,d'] }); + const options = getCliOptionsTest(['change', '--scope', 'a,b', '--scope=c,d']); + expect(options).toEqual({ command: 'change', path: projectRoot, scope: ['a,b', 'c,d'] }); }); - it('throws if non-array option is specified multiple times', () => { - expect(() => getCliOptionsTest(['--tag', 'foo', '--tag', 'baz'])).toThrow(); + it('uses last value if non-array option is specified multiple times', () => { + const options = getCliOptionsTest(['change', '--tag', 'foo', '--tag', 'baz']); + expect(options).toEqual({ command: 'change', path: projectRoot, tag: 'baz' }); }); it('parses negated boolean option', () => { - const options = getCliOptionsTest(['--no-fetch']); - expect(options).toEqual({ ...defaults, fetch: false }); + const options = getCliOptionsTest(['change', '--no-fetch']); + expect(options).toEqual({ command: 'change', path: projectRoot, fetch: false }); }); - it('parses valid boolean option values', () => { - const falseOptions = getCliOptionsTest(['--fetch=false', '--yes', 'false']); - expect(falseOptions).toEqual({ ...defaults, fetch: false, yes: false }); - - const trueOptions = getCliOptionsTest(['--fetch=true', '--yes', 'true']); - expect(trueOptions).toEqual({ ...defaults, fetch: true, yes: true }); - }); - - it('parses boolean flag with valid value', () => { - const falseOptions = getCliOptionsTest(['-y', 'false']); - expect(falseOptions).toEqual({ ...defaults, yes: false }); - - const trueOptions = getCliOptionsTest(['-y', 'true']); - expect(trueOptions).toEqual({ ...defaults, yes: true }); + it('parses negated boolean options with --no-X syntax', () => { + const options = getCliOptionsTest(['change', '--no-fetch', '--no-yes']); + expect(options).toEqual({ command: 'change', path: projectRoot, fetch: false, yes: false }); }); it('throws on invalid numeric value', () => { - expect(() => getCliOptionsTest(['--depth', 'foo'])).toThrow(); + expect(() => getCliOptionsTest(['change', '--depth', 'foo'])).toThrow(); }); it('converts hyphenated options to camel case', () => { - const options = getCliOptionsTest(['--git-tags', '--dependent-change-type', 'patch']); - expect(options).toEqual({ ...defaults, gitTags: true, dependentChangeType: 'patch' }); + const options = getCliOptionsTest(['change', '--git-tags', '--dependent-change-type', 'patch']); + expect(options).toEqual({ + command: 'change', + path: projectRoot, + gitTags: true, + dependentChangeType: 'patch', + }); }); - it('supports camel case for options defined as hyphenated', () => { + it('requires hyphenated form for multi-word options', () => { const options = getCliOptionsTest([ - '--gitTags', - '--dependentChangeType', + 'change', + '--git-tags', + '--dependent-change-type', 'patch', '--disallowed-change-types', 'major', 'minor', ]); expect(options).toEqual({ - ...defaults, + command: 'change', + path: projectRoot, gitTags: true, dependentChangeType: 'patch', disallowedChangeTypes: ['major', 'minor'], }); }); + it('suggests dashed form for camelCase boolean options', () => { + expect(() => getCliOptionsTest(['change', '--gitTags'])).toThrow('Did you mean --git-tags or --no-git-tags?'); + expect(() => getCliOptionsTest(['change', '--bumpDeps'])).toThrow('Did you mean --bump-deps or --no-bump-deps?'); + expect(() => getCliOptionsTest(['change', '--keepChangeFiles'])).toThrow( + 'Did you mean --keep-change-files or --no-keep-change-files?' + ); + }); + + it('suggests dashed form for camelCase non-boolean options', () => { + expect(() => getCliOptionsTest(['change', '--fromRef', 'main'])).toThrow('Did you mean --from-ref?'); + expect(() => getCliOptionsTest(['change', '--dependentChangeType', 'patch'])).toThrow( + 'Did you mean --dependent-change-type?' + ); + }); + + it('suggests dashed --no- form for camelCase --noX options', () => { + expect(() => getCliOptionsTest(['change', '--noFetch'])).toThrow('Did you mean --fetch or --no-fetch?'); + expect(() => getCliOptionsTest(['change', '--noBump'])).toThrow('Did you mean --bump or --no-bump?'); + expect(() => getCliOptionsTest(['change', '--noGitTags'])).toThrow('Did you mean --git-tags or --no-git-tags?'); + }); + it('parses short option aliases', () => { const options = getCliOptionsTest(['publish', '-t', 'test', '-r', 'http://whatever', '-y']); - expect(options).toEqual({ ...defaults, command: 'publish', tag: 'test', registry: 'http://whatever', yes: true }); + expect(options).toEqual({ + command: 'publish', + path: projectRoot, + tag: 'test', + registry: 'http://whatever', + yes: true, + }); }); it('parses long option aliases', () => { - const options = getCliOptionsTest(['--config', 'path/to/config.json', '--force', '--since', 'main']); - expect(options).toEqual({ ...defaults, configPath: 'path/to/config.json', forceVersions: true, fromRef: 'main' }); + const options = getCliOptionsTest(['change', '--config', 'path/to/config.json', '--force', '--since', 'main']); + expect(options).toEqual({ + command: 'change', + path: projectRoot, + configPath: 'path/to/config.json', + forceVersions: true, + fromRef: 'main', + }); }); it('for canary command, adds canary tag and ignores regular tag', () => { const options = getCliOptionsTest(['canary', '--tag', 'bar']); - expect(options).toEqual({ ...defaults, command: 'canary', tag: 'canary' }); + expect(options).toEqual({ command: 'canary', path: projectRoot, tag: 'canary' }); }); it('for canary command, uses canaryName as tag and ignores regular tag', () => { const options = getCliOptionsTest(['canary', '--canary-name', 'foo', '--tag', 'bar']); - expect(options).toEqual({ ...defaults, command: 'canary', canaryName: 'foo', tag: 'foo' }); + expect(options).toEqual({ command: 'canary', path: projectRoot, canaryName: 'foo', tag: 'foo' }); }); it('does not set tag to canaryName for non-canary command', () => { const options = getCliOptionsTest(['publish', '--canary-name', 'foo', '--tag', 'bar']); - expect(options).toEqual({ ...defaults, command: 'publish', canaryName: 'foo', tag: 'bar' }); + expect(options).toEqual({ command: 'publish', path: projectRoot, canaryName: 'foo', tag: 'bar' }); }); it('falls back to given cwd as path if findProjectRoot fails', () => { mockFindProjectRoot.mockImplementationOnce(() => { throw new Error('nope'); }); - const options = getCliOptionsTest([], 'somewhere'); - expect(options).toEqual({ ...defaults, path: 'somewhere' }); + const options = getCliOptionsTest(['change'], 'somewhere'); + expect(options).toEqual({ command: 'change', path: 'somewhere' }); }); it('uses provided branch if it contains a slash', () => { - const options = getCliOptionsTest(['--branch', 'someremote/foo']); - expect(options).toEqual({ ...defaults, branch: 'someremote/foo' }); + const options = getCliOptionsTest(['change', '--branch', 'someremote/foo']); + expect(options).toEqual({ command: 'change', path: projectRoot, branch: 'someremote/foo' }); // this is mocked at the top of the file // eslint-disable-next-line etc/no-deprecated expect(getDefaultRemoteBranch).not.toHaveBeenCalled(); }); it('adds default remote to branch without slash', () => { - const options = getCliOptionsTest(['--branch', 'foo']); - expect(options).toEqual({ ...defaults, branch: 'origin/foo' }); + const options = getCliOptionsTest(['change', '--branch', 'foo']); + expect(options).toEqual({ command: 'change', path: projectRoot, branch: 'origin/foo' }); // eslint-disable-next-line etc/no-deprecated expect(getDefaultRemoteBranch).toHaveBeenCalledWith({ branch: 'foo', verbose: undefined, cwd: projectRoot }); }); - it('preserves additional string options', () => { - const options = getCliOptionsTest(['--foo', 'bar', '--baz=qux']); - expect(options).toEqual({ ...defaults, foo: 'bar', baz: 'qux' }); + it('throws on unknown options', () => { + expect(() => getCliOptionsTest(['change', '--foo', 'bar'])).toThrow(); + expect(() => getCliOptionsTest(['change', '--foo'])).toThrow(); + expect(() => getCliOptionsTest(['change', '--no-bar'])).toThrow(); + expect(() => getCliOptionsTest(['change', '--foo', 'true'])).toThrow(); + expect(() => getCliOptionsTest(['change', '--foo', 'bar', '--foo', 'baz'])).toThrow(); }); - it('handles additional boolean flags as booleans', () => { - const options = getCliOptionsTest(['--foo', '--no-bar']); - expect(options).toEqual({ ...defaults, foo: true, bar: false }); + it('suggests --opt/--no-opt for near-match of a boolean flag', () => { + expect(() => getCliOptionsTest(['change', '--fetc'])).toThrow('Did you mean --fetch or --no-fetch?'); + expect(() => getCliOptionsTest(['change', '--git-tag'])).toThrow('Did you mean --git-tags or --no-git-tags?'); + expect(() => getCliOptionsTest(['change', '--bum'])).toThrow('Did you mean --bump or --no-bump?'); }); - it('handles additional boolean text values as booleans', () => { - const options = getCliOptionsTest(['--foo', 'true', '--bar=false']); - expect(options).toEqual({ ...defaults, foo: true, bar: false }); + it('suggests --opt/--no-opt for near-match of a --no-X flag', () => { + expect(() => getCliOptionsTest(['change', '--no-git-tag'])).toThrow('Did you mean --git-tags or --no-git-tags?'); + expect(() => getCliOptionsTest(['change', '--no-bum'])).toThrow('Did you mean --bump or --no-bump?'); }); - it('handles additional numeric values as numbers', () => { - const options = getCliOptionsTest(['--foo', '1', '--bar=2']); - expect(options).toEqual({ ...defaults, foo: 1, bar: 2 }); + it('throws on unknown command', () => { + expect(() => getCliOptionsTest(['unknown'])).toThrow(); }); - it('handles additional option specified multiple times as array', () => { - const options = getCliOptionsTest(['--foo', 'bar', '--foo', 'baz']); - expect(options).toEqual({ ...defaults, foo: ['bar', 'baz'] }); - }); - - // documenting current behavior (doesn't have to stay this way) - it('for additional options, does not handle multiple values as part of array', () => { - // in this case the trailing value "baz" would be treated as the command since it's the first - // positional option - const options = getCliOptionsTest(['--foo', 'bar', 'baz']); - expect(options).toEqual({ ...defaults, foo: 'bar', command: 'baz' }); + it('throws on extra positional arguments', () => { + expect(() => getCliOptionsTest(['check', 'extra'])).toThrow(); }); describe('config command', () => { it('parses config get with setting name', () => { const options = getCliOptionsTest(['config', 'get', 'branch']); - expect(options).toEqual({ ...defaults, command: 'config', _extraPositionalArgs: ['get', 'branch'] }); + expect(options).toEqual({ command: 'config get', path: projectRoot, configSettingName: 'branch' }); }); it('parses config get with setting name and options', () => { const options = getCliOptionsTest(['config', 'get', 'tag', '--package', 'my-pkg']); expect(options).toEqual({ - ...defaults, - command: 'config', - _extraPositionalArgs: ['get', 'tag'], + command: 'config get', + path: projectRoot, + configSettingName: 'tag', package: ['my-pkg'], }); }); - it('still throws for non-config command with extra positional args', () => { - expect(() => getCliOptionsTest(['check', 'extra'])).toThrow( - 'Only one positional argument (the command) is allowed' - ); + it('parses config list', () => { + const options = getCliOptionsTest(['config', 'list']); + expect(options).toEqual({ command: 'config list', path: projectRoot }); + }); + + it('throws on config get without setting name', () => { + expect(() => getCliOptionsTest(['config', 'get'])).toThrow(); + }); + + it('throws on config get with extra args', () => { + expect(() => getCliOptionsTest(['config', 'get', 'branch', 'extra'])).toThrow(); }); }); }); diff --git a/src/__functional__/options/getOptions.test.ts b/src/__functional__/options/getOptions.test.ts index 19c536eda..4a5bef03a 100644 --- a/src/__functional__/options/getOptions.test.ts +++ b/src/__functional__/options/getOptions.test.ts @@ -14,7 +14,7 @@ describe('getOptions (deprecated)', () => { // Don't reuse a repo in these tests! If multiple tests load beachball.config.js from the same path, // it will use the version from the require cache, which will have outdated contents. - const baseArgv = () => ['node.exe', 'bin.js']; + const baseArgv = () => ['node.exe', 'bin.js', 'change']; const inDirectory = (directory: string, cb: () => T): T => { const originalDirectory = process.cwd(); @@ -109,7 +109,7 @@ describe('getParsedOptions', () => { // it will use the version from the require cache, which will have outdated contents. // Return a new object each time since getRepoOptions caches the result based on object identity. - const baseArgv = () => ['node', 'beachball', 'stuff']; + const baseArgv = () => ['node', 'beachball', 'change']; beforeAll(() => { repositoryFactory = new RepositoryFactory('single'); @@ -130,7 +130,7 @@ describe('getParsedOptions', () => { const parsedOptions = getParsedOptions({ argv: baseArgv(), cwd: repo.rootPath }); expect(parsedOptions).toEqual({ options: expect.objectContaining({ branch: 'origin/foo' }), - cliOptions: { path: repo.rootPath, command: 'stuff' }, + cliOptions: { path: repo.rootPath, command: 'change' }, }); }); @@ -141,7 +141,7 @@ describe('getParsedOptions', () => { const parsedOptions = getParsedOptions({ argv: baseArgv(), cwd: repo.rootPath }); expect(parsedOptions).toEqual({ options: expect.objectContaining({ branch: 'origin/foo' }), - cliOptions: { path: repo.rootPath, command: 'stuff' }, + cliOptions: { path: repo.rootPath, command: 'change' }, }); }); @@ -179,7 +179,7 @@ describe('getParsedOptions', () => { }); expect(parsedOptions).toEqual({ options: expect.objectContaining({ branch: 'origin/bar' }), - cliOptions: { path: repo.rootPath, command: 'stuff', branch: 'origin/bar' }, + cliOptions: { path: repo.rootPath, command: 'change', branch: 'origin/bar' }, }); }); diff --git a/src/__tests__/bump/bumpInMemory.test.ts b/src/__tests__/bump/bumpInMemory.test.ts index d1349df37..4dc2fe039 100644 --- a/src/__tests__/bump/bumpInMemory.test.ts +++ b/src/__tests__/bump/bumpInMemory.test.ts @@ -20,7 +20,7 @@ describe('bumpInMemory', () => { }) { const { cliOptions, options } = getParsedOptions({ cwd, - argv: [], + argv: ['node', 'beachball', 'bump'], testRepoOptions: params.repoOptions, }); const originalPackageInfos = makePackageInfosByFolder({ diff --git a/src/__tests__/bump/performBump.test.ts b/src/__tests__/bump/performBump.test.ts index 2d133c978..6fe176234 100644 --- a/src/__tests__/bump/performBump.test.ts +++ b/src/__tests__/bump/performBump.test.ts @@ -71,7 +71,7 @@ describe('performBump', () => { }) { const opts = getParsedOptions({ cwd: fakeRoot, - argv: [], + argv: ['node', 'beachball', 'bump'], testRepoOptions: { branch: defaultRemoteBranchName, ...params.repoOptions }, }); diff --git a/src/__tests__/changefile/getQuestionsForPackage.test.ts b/src/__tests__/changefile/getQuestionsForPackage.test.ts index 684175d1a..c02aaf43f 100644 --- a/src/__tests__/changefile/getQuestionsForPackage.test.ts +++ b/src/__tests__/changefile/getQuestionsForPackage.test.ts @@ -32,7 +32,11 @@ describe('getQuestionsForPackage', () => { const packageInfos = makePackageInfos({ [pkg]: packageInfo }); // fill in default options - const { options } = getParsedOptions({ cwd: '', argv: [], testRepoOptions: repoOptions }); + const { options } = getParsedOptions({ + cwd: '', + argv: ['node', 'beachball', 'change'], + testRepoOptions: repoOptions, + }); return getQuestionsForPackage({ pkg, packageInfos, packageGroups, options, recentMessages }); } diff --git a/src/__tests__/commands/configGet.test.ts b/src/__tests__/commands/configGet.test.ts index 3d2f00a2b..99eb83187 100644 --- a/src/__tests__/commands/configGet.test.ts +++ b/src/__tests__/commands/configGet.test.ts @@ -11,12 +11,13 @@ import type { BeachballOptions, PackageOptions, VersionGroupOptions } from '../. describe('configGet', () => { const logs = initMockLogs(); - /** Wrapper that just provides custom args to `configGet` (for invalid argument cases) */ - function configGetArgs(args: string[]) { - configGet( - { ...getDefaultOptions(), _extraPositionalArgs: args }, - { originalPackageInfos: {}, scopedPackages: new Set(), packageGroups: {} } - ); + /** Wrapper that just provides custom name to `configGet` (for invalid argument cases) */ + function configGetWithName(name?: string) { + configGet({ ...getDefaultOptions(), command: 'config get', configSettingName: name } as BeachballOptions, { + originalPackageInfos: {}, + scopedPackages: new Set(), + packageGroups: {}, + }); } /** Get the given option (`name` will be formatted as args) with optional overrides */ @@ -32,7 +33,8 @@ describe('configGet', () => { const options: BeachballOptions = { ...getDefaultOptions(), - _extraPositionalArgs: ['get', name], + command: 'config get', + configSettingName: name, ...optionOverrides, }; const originalPackageInfos = makePackageInfos(packageInfos); @@ -45,28 +47,17 @@ describe('configGet', () => { } describe('argument validation', () => { - it('throws on missing subcommand', async () => { - await expectBeachballError(() => configGetArgs([]), 'Usage: beachball config get '); - }); - - it('throws on wrong subcommand', async () => { - await expectBeachballError(() => configGetArgs(['set', 'branch']), 'Usage: beachball config get '); - }); - - it('throws on too many args', async () => { - await expectBeachballError( - () => configGetArgs(['get', 'branch', 'extra']), - 'Usage: beachball config get ' - ); + it('throws on missing setting name', async () => { + await expectBeachballError(() => configGetWithName(), 'Usage: beachball config get '); }); it('throws on unknown config setting', async () => { - await expectBeachballError(() => configGetArgs(['get', 'nonExistent']), 'Unknown config setting: "nonExistent"'); + await expectBeachballError(() => configGetWithName('nonExistent'), 'Unknown config setting: "nonExistent"'); }); it('suggests similar config name on typo', async () => { await expectBeachballError( - () => configGetArgs(['get', 'branc']), + () => configGetWithName('branc'), 'Unknown config setting: "branc" - did you mean "branch"?' ); }); diff --git a/src/__tests__/commands/configList.test.ts b/src/__tests__/commands/configList.test.ts index cedf7f183..de6d2bbff 100644 --- a/src/__tests__/commands/configList.test.ts +++ b/src/__tests__/commands/configList.test.ts @@ -83,7 +83,6 @@ describe('configList', () => { tag: "" timeout: undefined type: null - version: false yes: true" `); }); diff --git a/src/__tests__/packageManager/listPackageVersions.test.ts b/src/__tests__/packageManager/listPackageVersions.test.ts index d8b9e4fc8..c707ab1ca 100644 --- a/src/__tests__/packageManager/listPackageVersions.test.ts +++ b/src/__tests__/packageManager/listPackageVersions.test.ts @@ -118,7 +118,7 @@ describe('list npm versions', () => { repoOptions?: Partial; }) { const parsedOptions = getParsedOptions({ - argv: ['node', 'beachball', ...(params.extraArgv || [])], + argv: ['node', 'beachball', 'publish', ...(params.extraArgv || [])], cwd: '', testRepoOptions: { registry, @@ -318,7 +318,7 @@ describe('list npm versions', () => { npmMock.setRegistryData({ foo: { 'dist-tags': { latest: '1.0.0', beta: '2.0.0-beta' } } }); const { packages, options } = getOptionsAndPackages({ packages: { foo: {} }, - extraArgv: ['--authType', 'password', '--token', 'pass'], + extraArgv: ['--auth-type', 'password', '--token', 'pass'], }); expect(options).toMatchObject({ authType: 'password', token: 'pass' }); diff --git a/src/cli.ts b/src/cli.ts index c38720013..cdf2fcad9 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -8,7 +8,7 @@ import { init } from './commands/init'; import { publish } from './commands/publish'; import { sync } from './commands/sync'; -import { showVersion, showHelp } from './help'; +import { showVersion } from './help'; import { getPackageInfos } from './monorepo/getPackageInfos'; import { getParsedOptions } from './options/getOptions'; import { validate } from './validation/validate'; @@ -28,16 +28,6 @@ import { getPackageGroups } from './monorepo/getPackageGroups'; const parsedOptions = getParsedOptions({ cwd: process.cwd(), argv: process.argv }); const options = parsedOptions.options; - if (options.help) { - showHelp(); - return; - } - - if (options.version) { - showVersion(); - return; - } - // Run the commands switch (options.command) { case 'check': { @@ -96,17 +86,21 @@ import { getPackageGroups } from './monorepo/getPackageGroups'; break; } - case 'config': { + case 'config get': { const originalPackageInfos = getPackageInfos(parsedOptions); const scopedPackages = getScopedPackages(options, originalPackageInfos); const packageGroups = getPackageGroups(originalPackageInfos, options.path, options.groups); const configContext = { originalPackageInfos, scopedPackages, packageGroups }; - const subcommand = options._extraPositionalArgs?.[0]; - if (subcommand === 'list') { - configList(options, configContext); - } else { - configGet(options, configContext); - } + configGet(options, configContext); + break; + } + + case 'config list': { + const originalPackageInfos = getPackageInfos(parsedOptions); + const scopedPackages = getScopedPackages(options, originalPackageInfos); + const packageGroups = getPackageGroups(originalPackageInfos, options.path, options.groups); + const configContext = { originalPackageInfos, scopedPackages, packageGroups }; + configList(options, configContext); break; } @@ -114,6 +108,17 @@ import { getPackageGroups } from './monorepo/getPackageGroups'; throw new Error('Invalid command: ' + options.command); } })().catch(e => { + // Handle commander's --help and --version output (exit cleanly) + if (e && typeof e === 'object' && 'code' in e) { + const code = (e as { code: string }).code; + if (code === 'commander.helpDisplayed' || code === 'commander.version') { + if ((e as { message?: string }).message) { + console.log((e as { message: string }).message); + } + return; + } + } + if (e instanceof BeachballError && e.alreadyLogged) { // Error details were already printed -- just exit } else if (e instanceof BeachballError) { diff --git a/src/commands/configGet.ts b/src/commands/configGet.ts index dd319b2a4..509c632fa 100644 --- a/src/commands/configGet.ts +++ b/src/commands/configGet.ts @@ -82,14 +82,12 @@ const validConfigNames = new Set([ export function configGet(options: BeachballOptions, context: BasicCommandContext): void { const { originalPackageInfos: packageInfos, scopedPackages } = context; - const extraArgs = options._extraPositionalArgs || []; - if (extraArgs[0] !== 'get' || extraArgs.length !== 2) { + const name = options.configSettingName; + if (!name) { throw new BeachballError( 'Usage: beachball config get \n\nGets the value of the specified config setting.' ); } - - const name = extraArgs[1]; if (!validConfigNames.has(name)) { const suggestion = findSimilar(name, [...validConfigNames]); throw new BeachballError( diff --git a/src/help.ts b/src/help.ts index 3c6937034..fc31a7acf 100644 --- a/src/help.ts +++ b/src/help.ts @@ -6,95 +6,3 @@ export function showVersion(): void { const packageJson = readJson(path.resolve(__dirname, '../package.json')); console.log(`beachball v${packageJson.version} - the sunniest version bumping tool`); } - -export function showHelp(): void { - showVersion(); - - console.log(`Usage: - - beachball [command] [options] - -Examples: - - $ beachball - $ beachball check - $ beachball publish -r http://localhost:4873 -t beta -b beta - -Commands: - - change (default) - create change files in the change/ folder - check - checks whether a change file is needed for this branch - bump - bumps versions as well as generating changelogs - publish - bumps, publishes to npm registry (optionally does dist-tags), and - pushes changelogs back into the default branch - sync - synchronize published versions of packages from the registry with - local package.json versions - config get - get the value of a config setting (with any overrides) - config list - list all config settings (with any overrides) - -Options supported by all commands except 'config': - - --branch, -b - target branch from remote (default: git config init.defaultBranch) - --change-dir - name of the directory to store change files (default: change) - --config-path, -c - custom beachball config path (default: cosmiconfig standard paths) - --no-fetch - skip fetching from the remote before determining changes - --scope - only consider package paths matching this pattern - (can be specified multiple times; supports negations) - --since - consider changes or change files since this git ref (branch name, commit SHA) - --verbose - prints additional information to the console - -'change' options: - - --message, -m - description for all changed packages (instead of prompting) - --type - type of change: minor, patch, none, ... (instead of prompting) - --package, -p - force creating a change file for this package, regardless of diffs - (can be specified multiple times) - --all - generate change files for all packages - --dependent-change-type - use this change type for dependent packages (default patch) - --no-commit - stage change files only - -'check' options: - - --changehint - give your developers a customized hint message when they - forget to add a change file - --disallow-deleted-change-files - verifies that no change files were deleted between head and - target branch. - -'bump' options: - - --keep-change-files - don't delete the change files from disk after bumping - --prerelease-prefix - prerelease prefix for packages that will receive a prerelease bump - -'publish' options: - - Also supports all 'bump' options. - - --auth-type - npm auth type: 'authtoken' or 'password' - --message, -m - commit message (default: "applying package updates") - --no-bump - skip both bumping versions and pushing changes back to git remote - --no-git-tags - don't create git tags for each published package version - --no-publish - skip publishing to the npm registry - --no-push - skip committing changes and pushing them back to the git remote - --registry, -r - registry (default https://registry.npmjs.org) - --retries - number of retries for npm publishes (default: 3) - --tag, -t - dist-tag for npm publishes (default: "latest") - --token - npm token or password - --yes, -y - skip the confirmation prompts - -'sync' options: - - --registry, -r - registry (default https://registry.npmjs.org) - --tag, -t - sync to the specified npm dist-tag (default: 'latest') - --force - use the version from the registry even if it's older than local - -'config get ' options: - - --package, -p - get the effective value for specific package(s) - (can be specified multiple times) - -'config list' options: - - (no additional options) - -`); -} diff --git a/src/options/getCliOptions.ts b/src/options/getCliOptions.ts index 37c8e020f..993f4a303 100644 --- a/src/options/getCliOptions.ts +++ b/src/options/getCliOptions.ts @@ -1,6 +1,9 @@ -import parser from 'yargs-parser'; -import type { CliOptions, ParsedOptions } from '../types/BeachballOptions'; +import { Command, CommanderError, Option } from 'commander'; +import path from 'path'; +import type { ParsedOptions } from '../types/BeachballOptions'; +import type { PackageJson } from '../types/PackageInfo'; import { getDefaultRemoteBranch, findProjectRoot } from 'workspace-tools'; +import { readJson } from '../object/readJson'; import { env } from '../env'; export interface ProcessInfo { @@ -15,109 +18,269 @@ export interface ProcessInfo { cwd: string; } -// For camelCased options, yargs will automatically accept them with-dashes too. -const arrayOptions = ['disallowedChangeTypes', 'package', 'scope'] as const; -const booleanOptions = [ - 'all', - 'bump', - 'bumpDeps', - 'commit', - 'disallowDeletedChangeFiles', - 'fetch', - 'forceVersions', - 'gitTags', - 'help', - 'keepChangeFiles', - 'new', - 'publish', - 'push', - 'verbose', - 'version', - 'yes', -] as const; -const numberOptions = ['concurrency', 'depth', 'npmReadConcurrency', 'gitTimeout', 'retries', 'timeout'] as const; -const stringOptions = [ - 'access', - 'authType', - 'branch', - 'canaryName', - 'changehint', - 'changeDir', - 'configPath', - 'dependentChangeType', - 'fromRef', - 'message', - 'packToPath', - 'prereleasePrefix', - 'registry', - 'tag', - 'token', - 'type', -] as const; - -type AtLeastOne = [T, ...T[]]; -/** Type hack to verify that an array includes all keys of a type */ -const allKeysOfType = - () => - >( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - ...x: L extends any ? (Exclude extends never ? L : Exclude[]) : never - ) => - x; - -// Verify that all the known CLI options have types specified, to ensure correct parsing. -// -// NOTE: If a prop is missing, this will have a somewhat misleading error: -// Argument of type '"disallowedChangeTypes"' is not assignable to parameter of type '"tag" | "version"' -// -// To fix, add the missing names after "parameter of type" ("tag" and "version" in this example) -// to the appropriate array above. -const knownOptions = allKeysOfType()( - ...arrayOptions, - ...booleanOptions, - ...numberOptions, - ...stringOptions, - // these options are filled in below, not respected from the command line - 'path', - 'command', - '_extraPositionalArgs' -); - -const parserOptions: parser.Options = { - configuration: { - 'boolean-negation': true, - 'camel-case-expansion': true, - 'dot-notation': false, - 'duplicate-arguments-array': true, - 'flatten-duplicate-arrays': true, - 'greedy-arrays': true, // for now; we might want to change this to false in the future - 'parse-numbers': true, - 'parse-positional-numbers': false, - 'short-option-groups': false, - 'strip-aliased': true, - 'strip-dashed': true, - }, - // spread to get rid of readonly... - array: [...arrayOptions], - boolean: [...booleanOptions], - number: [...numberOptions], - string: [...stringOptions], - alias: { - authType: ['a'], - branch: ['b'], - configPath: ['c', 'config'], - forceVersions: ['force'], - fromRef: ['since'], - help: ['h', '?'], - message: ['m'], - package: ['p'], - registry: ['r'], - tag: ['t'], - token: ['n'], - version: ['v'], - yes: ['y'], - }, -}; +/** Parse a string value as a number, throwing if the result is NaN. */ +function parseNumber(value: string, optionName: string): number { + const num = Number(value); + if (isNaN(num)) { + throw new Error(`Non-numeric value passed for numeric option "${optionName}"`); + } + return num; +} + +/** Add all shared CLI options (for every command except `config get`) to a command. */ +function addSharedOptions(cmd: Command): Command { + // -- Boolean options (with --no-X negation support) -- + cmd.option('--all', 'consider all packages to have changed'); + cmd.option('--bump', 'bump versions during publish'); + cmd.option('--no-bump', 'skip bumping versions during publish'); + cmd.option('--bump-deps', 'bump dependent packages'); + cmd.option('--no-bump-deps', 'skip bumping dependent packages'); + cmd.option('--commit', 'commit change files after creating'); + cmd.option('--no-commit', 'stage change files only'); + cmd.option('--disallow-deleted-change-files', 'verify no change files were deleted'); + cmd.option('--no-disallow-deleted-change-files'); + cmd.option('--fetch', 'fetch from the remote before determining changes'); + cmd.option('--no-fetch', 'skip fetching from the remote'); + cmd.option('--force, --force-versions', 'use version from registry even if older than local'); + cmd.option('--git-tags', 'create git tags for published package versions'); + cmd.option('--no-git-tags', 'skip creating git tags'); + cmd.option('--keep-change-files', 'keep change files on disk after bumping'); + cmd.option('--no-keep-change-files'); + cmd.option('--new', 'publish newly added packages'); + cmd.option('--no-new'); + cmd.option('--publish', 'publish to the npm registry'); + cmd.option('--no-publish', 'skip publishing to the npm registry'); + cmd.option('--push', 'push to the remote git branch when publishing'); + cmd.option('--no-push', 'skip pushing to the git remote'); + cmd.option('--verbose', 'print additional information to the console'); + cmd.option('-y, --yes', 'skip confirmation prompts'); + cmd.option('--no-yes'); + + // -- Array options -- + cmd.option('--disallowed-change-types ', 'disallow these change types'); + cmd.option('-p, --package ', 'target specific packages'); + cmd.option('--scope ', 'only consider package paths matching these patterns'); + + // -- Number options -- + cmd.addOption( + new Option('--concurrency ', 'maximum concurrency for write operations').argParser((v: string) => + parseNumber(v, 'concurrency') + ) + ); + cmd.addOption( + new Option('--depth ', 'depth of git history for shallow clones').argParser((v: string) => + parseNumber(v, 'depth') + ) + ); + cmd.addOption( + new Option('--npm-read-concurrency ', 'maximum concurrency for npm registry reads').argParser((v: string) => + parseNumber(v, 'npmReadConcurrency') + ) + ); + cmd.addOption( + new Option('--git-timeout ', 'timeout for git push operations').argParser((v: string) => + parseNumber(v, 'gitTimeout') + ) + ); + cmd.addOption( + new Option('--retries ', 'number of retries for npm publish').argParser((v: string) => parseNumber(v, 'retries')) + ); + cmd.addOption( + new Option('--timeout ', 'timeout for npm operations').argParser((v: string) => parseNumber(v, 'timeout')) + ); + + // -- String options -- + cmd.option('--access ', "access level for npm publish: 'public' or 'restricted'"); + cmd.option('-a, --auth-type ', "npm auth type: 'authtoken' or 'password'"); + cmd.option('-b, --branch ', 'target branch from remote'); + cmd.option('--canary-name ', 'canary prerelease name'); + cmd.option('--changehint ', 'custom hint message when change files are needed'); + cmd.option('--change-dir ', 'directory to store change files'); + cmd.option('-c, --config ', 'custom beachball config path'); + cmd.addOption(new Option('--config-path ').hideHelp()); // hidden alias for --config + cmd.option('--dependent-change-type ', 'change type for dependent packages'); + cmd.option('--since, --from-ref ', 'consider changes since this git ref'); + cmd.option('-m, --message ', 'change description or commit message'); + cmd.option('--pack-to-path ', 'pack packages to this path instead of publishing'); + cmd.option('--prerelease-prefix ', 'prerelease prefix for prerelease bumps'); + cmd.option('-r, --registry ', 'target npm registry'); + cmd.option('-t, --tag ', 'dist-tag for npm publishes'); + cmd.option('-n, --token ', 'npm token or password'); + cmd.option('--type ', 'type of change: major, minor, patch, none, ...'); + + return cmd; +} + +/** + * Reference command with all shared options, used for error enhancement only. + * (Separate from the real commands to avoid circular references.) + */ +const referenceCommand = addSharedOptions(new Command()); + +/** Version string, read once at module load (before tests can mock fs). */ +const beachballVersion = (() => { + try { + return readJson(path.resolve(__dirname, '../../package.json')).version || 'unknown'; + } catch { + return 'unknown'; + } +})(); +/** Convert a camelCase string to dashed form: e.g. `gitTags` => `git-tags` */ +function camelToDash(str: string): string { + return str.replace(/[A-Z]/g, letter => `-${letter.toLowerCase()}`); +} + +/** + * If the unknown option matches a boolean flag, suggest both `--opt` and `--no-opt`. + * If the unknown option is in camelCase, suggest the valid dashed form instead. + * Otherwise re-throw the original error unchanged. + */ +function enhanceUnknownOptionError(err: CommanderError): never { + // Parse the unknown flag from commander's error message format: + // "error: unknown option '--foo'" or "error: unknown option '--foo'\n(Did you mean --bar?)" + const flagMatch = err.message.match(/^error: unknown option '(--[^']+)'/); + if (!flagMatch) { + throw err; + } + const unknownFlag = flagMatch[1]; + + // Collect info about known options from the reference command + const knownLongFlags = new Set(); + const negatableBooleans = new Set(); // long flags that have a --no-X counterpart + for (const opt of referenceCommand.options) { + if (opt.long) { + knownLongFlags.add(opt.long); + if (opt.negate && knownLongFlags.has(opt.long.replace(/^--no-/, '--'))) { + negatableBooleans.add(opt.long.replace(/^--no-/, '--')); + } + } + } + // Second pass: if we saw --X before --no-X, we need to check in reverse + for (const opt of referenceCommand.options) { + if (opt.long && !opt.negate && knownLongFlags.has(`--no-${opt.long.slice(2)}`)) { + negatableBooleans.add(opt.long); + } + } + + // Check if the unknown flag contains uppercase and the dashed version is valid + const flagName = unknownFlag.replace(/^--/, ''); + if (/[A-Z]/.test(flagName)) { + const dashedFlag = `--${camelToDash(flagName)}`; + if (knownLongFlags.has(dashedFlag)) { + let suggestion: string; + if (negatableBooleans.has(dashedFlag)) { + suggestion = `(Did you mean ${dashedFlag} or --no-${dashedFlag.slice(2)}?)`; + } else if (dashedFlag.startsWith('--no-') && negatableBooleans.has(`--${dashedFlag.slice(5)}`)) { + suggestion = `(Did you mean --${dashedFlag.slice(5)} or ${dashedFlag}?)`; + } else { + suggestion = `(Did you mean ${dashedFlag}?)`; + } + throw new CommanderError(err.exitCode, err.code, `error: unknown option '${unknownFlag}'\n${suggestion}`); + } + } + + // Check if commander's existing suggestion matches a negatable boolean flag + const suggestMatch = err.message.match(/\(Did you mean (--[^ ?]+)\?\)/); + if (suggestMatch) { + const suggested = suggestMatch[1]; + if (negatableBooleans.has(suggested)) { + throw new CommanderError( + err.exitCode, + err.code, + `error: unknown option '${unknownFlag}'\n(Did you mean ${suggested} or --no-${suggested.slice(2)}?)` + ); + } + if (suggested.startsWith('--no-')) { + const positiveFlag = `--${suggested.slice(5)}`; + if (negatableBooleans.has(positiveFlag)) { + throw new CommanderError( + err.exitCode, + err.code, + `error: unknown option '${unknownFlag}'\n(Did you mean ${positiveFlag} or ${suggested}?)` + ); + } + } + } + + // No enhancement possible, re-throw as-is + throw err; +} + +interface CommandMatch { + command: string; + opts: Record; + configSettingName?: string; +} + +/** Suppress stderr but capture stdout (for help/version output). */ +function makeOutputConfig(captured: { output: string }) { + return { + writeOut: (str: string) => { + captured.output += str; + }, + // eslint-disable-next-line @typescript-eslint/no-empty-function + writeErr: () => {}, + }; +} + +/** + * Creates and configures the commander program with all beachball commands and options. + */ +function createProgram(): { + program: Command; + getMatch: () => CommandMatch | undefined; + captured: { output: string }; +} { + let match: CommandMatch | undefined; + const captured = { output: '' }; + const outputConfig = makeOutputConfig(captured); + + const program = new Command('beachball') + .description('the sunniest version bumping tool') + .version(beachballVersion, '-v, --version') + .exitOverride() + .configureOutput(outputConfig); + + // Define standard commands — each gets all shared options + const commandDefs = [ + { name: 'change', desc: 'create change files in the change/ folder' }, + { name: 'check', desc: 'check whether a change file is needed for this branch' }, + { name: 'bump', desc: 'bump versions and generate changelogs' }, + { name: 'publish', desc: 'bump, publish to npm registry, and push changelogs' }, + { name: 'canary', desc: 'publish canary prerelease versions' }, + { name: 'init', desc: 'initialize beachball config' }, + { name: 'sync', desc: 'synchronize published versions from the registry' }, + ]; + + for (const { name, desc } of commandDefs) { + const cmd = program.command(name).description(desc); + cmd.exitOverride().configureOutput(outputConfig).allowExcessArguments(false); + addSharedOptions(cmd); + cmd.action(() => { + match = { command: name, opts: cmd.opts() }; + }); + } + + // Config command with subcommands + const config = program.command('config').description('view configuration'); + config.exitOverride().configureOutput(outputConfig); + + const configGet = config.command('get').description('get the value of a config setting'); + configGet.argument('', 'config setting name'); + configGet.option('-p, --package ', 'get effective value for specific package(s)'); + configGet.exitOverride().configureOutput(outputConfig).allowExcessArguments(false); + configGet.action((name: string) => { + match = { command: 'config get', opts: configGet.opts(), configSettingName: name }; + }); + + const configList = config.command('list').description('list all config settings'); + configList.exitOverride().configureOutput(outputConfig).allowExcessArguments(false); + configList.action(() => { + match = { command: 'config list', opts: configList.opts() }; + }); + + return { program, getMatch: () => match, captured }; +} /** * Gets CLI options. @@ -131,12 +294,30 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti { argv: processOrArgv, cwd: env.isJest ? '' : process.cwd() } : processOrArgv; - // Be careful not to mutate the input argv - const trimmedArgv = processInfo.argv.slice(2); + const { program, getMatch, captured } = createProgram(); + + try { + program.parse(processInfo.argv); + } catch (err) { + if (err instanceof CommanderError) { + if (err.code === 'commander.helpDisplayed' || err.code === 'commander.version') { + // Re-throw with captured output so the caller can print it + throw new CommanderError(err.exitCode, err.code, captured.output || err.message); + } + if (err.code === 'commander.unknownOption') { + enhanceUnknownOptionError(err); + } + } + throw err; + } - const args = parser(trimmedArgv, parserOptions); + const match = getMatch(); + if (!match) { + // No command was matched — show help + program.outputHelp(); + throw new CommanderError(0, 'commander.helpDisplayed', captured.output); + } - const { _: positionalArgs, ...options } = args; let cwd = processInfo.cwd; try { // If a non-empty cwd is provided, find the project root from there. @@ -148,21 +329,41 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti // use the provided cwd } - if (positionalArgs.length > 1 && String(positionalArgs[0]) !== 'config') { - throw new Error(`Only one positional argument (the command) is allowed. Received: ${positionalArgs.join(' ')}`); - } - + // Build the cliOptions object from commander's parsed options. + // Only include options that were explicitly set on the command line (not undefined defaults). const cliOptions: ParsedOptions['cliOptions'] = { - ...options, - command: positionalArgs.length ? String(positionalArgs[0]) : 'change', + command: match.command, path: cwd, }; - // Save extra positional args for commands that support subcommands (e.g. 'config get ') - // (yargs-parser doesn't support positional arguments directly) - const extraPositionalArgs = positionalArgs.length > 1 ? positionalArgs.slice(1).map(String) : undefined; + if (match.configSettingName !== undefined) { + cliOptions.configSettingName = match.configSettingName; + } + + const commanderOpts = { ...match.opts }; + + // Handle --config-path as alias for --config (both map to configPath). + // TODO: Replace this manual alias handling with `new Option(...).alias('--config-path')` + // when upgrading to a commander version that supports .alias() on Option. + if (commanderOpts.config !== undefined) { + if (commanderOpts.configPath !== undefined) { + throw new Error('Cannot specify both --config and --config-path'); + } + commanderOpts.configPath = commanderOpts.config; + } + delete commanderOpts.config; - const branchArg = args.branch as string | undefined; + // Copy all defined options from commander to cliOptions. + // Commander already converts hyphenated option names to camelCase in its opts() output. + for (const [key, value] of Object.entries(commanderOpts)) { + if (value !== undefined) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/no-unsafe-member-access + (cliOptions as any)[key] = value; + } + } + + // Handle branch argument: add remote if missing slash + const branchArg = cliOptions.branch; if (branchArg) { // TODO: This logic assumes the first segment of any branch name with a slash must be the remote, // which is not necessarily accurate. Ideally we should check if a remote with that name exists, @@ -170,38 +371,13 @@ export function getCliOptions(processOrArgv: ProcessInfo | string[]): ParsedOpti cliOptions.branch = branchArg.indexOf('/') > -1 ? branchArg - : getDefaultRemoteBranch({ branch: branchArg, verbose: args.verbose as boolean | undefined, cwd }); + : getDefaultRemoteBranch({ branch: branchArg, verbose: cliOptions.verbose, cwd }); } + // For canary command, set tag to canaryName or default 'canary' if (cliOptions.command === 'canary') { cliOptions.tag = cliOptions.canaryName || 'canary'; } - for (const key of Object.keys(cliOptions) as (keyof CliOptions)[]) { - const value = cliOptions[key]; - if (value === undefined) { - delete cliOptions[key]; - } else if (typeof value === 'number' && isNaN(value)) { - throw new Error(`Non-numeric value passed for numeric option "${key}"`); - } else if (knownOptions.includes(key)) { - if (Array.isArray(value) && !arrayOptions.includes(key as (typeof arrayOptions)[number])) { - throw new Error(`Option "${key}" only accepts a single value. Received: ${value.join(' ')}`); - } - } else if (value === 'true') { - // For unknown arguments like --foo=true or --bar=false, yargs will handle the value as a string. - // Convert it to a boolean to avoid subtle bugs. - // eslint-disable-next-line - (cliOptions as any)[key] = true; - } else if (value === 'false') { - // eslint-disable-next-line - (cliOptions as any)[key] = false; - } - } - - // Set extra positional args after the validation loop (it's an internal array, not from CLI parsing) - if (extraPositionalArgs) { - cliOptions._extraPositionalArgs = extraPositionalArgs; - } - return cliOptions; } diff --git a/src/options/getDefaultOptions.ts b/src/options/getDefaultOptions.ts index f8e6090bd..0bc4d13e8 100644 --- a/src/options/getDefaultOptions.ts +++ b/src/options/getDefaultOptions.ts @@ -37,7 +37,6 @@ export function getDefaultOptions(): BeachballOptions { tag: '', timeout: undefined, type: null, - version: false, yes: env.isCI, }; } diff --git a/src/types/BeachballOptions.ts b/src/types/BeachballOptions.ts index 48e9a578b..65673922b 100644 --- a/src/types/BeachballOptions.ts +++ b/src/types/BeachballOptions.ts @@ -59,21 +59,15 @@ export interface CliOptions * For sync: use the version from the registry even if it's older than local. */ forceVersions?: boolean; - help?: boolean; /** Force change files for these packages */ package?: string | string[]; token?: string; type?: ChangeType | null; verbose?: boolean; - version?: boolean; yes: boolean; - /** - * Extra positional arguments after the command (for subcommands like `config get `). - * This is a workaround for `yargs-parser`'s lack of positional argument support. - * @internal - */ - _extraPositionalArgs?: string[]; + /** The config setting name for `config get `. Set by the CLI parser. */ + configSettingName?: string; // ONLY add new options here if they only make sense on the command line! // Most options should be defined in RepoOptions and added to the Pick<...> above. diff --git a/yarn.lock b/yarn.lock index 3c854fc17..7b3f71766 100644 --- a/yarn.lock +++ b/yarn.lock @@ -758,7 +758,7 @@ resolved "https://registry.yarnpkg.com/@types/tmp/-/tmp-0.2.6.tgz#d785ee90c52d7cc020e249c948c36f7b32d1e217" integrity sha512-chhaNf2oKHlRkDGt+tiKE2Z5aJ6qalm7Z9rlLdBwmOiAAf09YQvvoLXjWK4HWPF1xU/fqvMgfNfpVoBscA/tKA== -"@types/yargs-parser@*", "@types/yargs-parser@^21.0.0": +"@types/yargs-parser@*": version "21.0.3" resolved "https://registry.yarnpkg.com/@types/yargs-parser/-/yargs-parser-21.0.3.tgz#815e30b786d2e8f0dcd85fd5bcf5e1a04d008f15" integrity sha512-I4q9QU9MQv4oEOz4tAHJtNz1cwuLxn2F3xcc2iV5WdqLPpUnj30aUuxt1mAxYTG+oe8CZMV/+6rU4S4gRDzqtQ== @@ -1621,6 +1621,11 @@ combined-stream@^1.0.6, combined-stream@~1.0.6: dependencies: delayed-stream "~1.0.0" +commander@^10.0.1: + version "10.0.1" + resolved "https://registry.yarnpkg.com/commander/-/commander-10.0.1.tgz#881ee46b4f77d1c1dccc5823433aa39b022cbe06" + integrity sha512-y4Mg2tXshplEbSGzx7amzPwKKOCGuoSRP/CjEdwwk0FOGlUbq6lKuoyDZTNZkmxHdJtp54hdfY/JUrdL7Xfdug== + commander@^9.3.0: version "9.5.0" resolved "https://registry.yarnpkg.com/commander/-/commander-9.5.0.tgz#bc08d1eb5cedf7ccb797a96199d41c7bc3e60d30" @@ -5165,7 +5170,7 @@ yargs-parser@^20.2.2: resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-20.2.9.tgz#2eb7dc3b0289718fc295f362753845c41a0c94ee" integrity sha512-y11nGElTIV+CT3Zv9t7VKl+Q3hTQoT9a1Qzezhhl6Rp21gJ/IVTW7Z3y9EWXhuUBC2Shnf+DX0antecpAwSP8w== -yargs-parser@^21.0.0, yargs-parser@^21.1.1: +yargs-parser@^21.1.1: version "21.1.1" resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-21.1.1.tgz#9096bceebf990d21bb31fa9516e0ede294a77d35" integrity sha512-tVpsJW7DdjecAiFpbIB1e3qxIQsE6NoPc5/eTdrbbIC4h0LVsWhnoa3g+m2HclBIujHzsxZ4VJVA+GUuc2/LBw==