Skip to content
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

Add isRequired flag option #141

Merged
merged 26 commits into from May 7, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
abd6b12
Add isRequired option for flags
sbencoding Mar 24, 2020
3d2f6c8
Add ability to specify isRequired as a function
sbencoding Mar 24, 2020
9b0c22a
Add type definition for isRequired
sbencoding Mar 24, 2020
45fb69a
Sync readme with typedef comments and examples
sbencoding Mar 24, 2020
aebcf85
Merge feature branch with upstream/master
sbencoding Apr 28, 2020
6bde170
Use console.error, better variable naming
sbencoding Apr 28, 2020
c095990
Update documentation
sbencoding Apr 28, 2020
1d9fa7d
Use stderr, because missing flags are printed with console.error
sbencoding Apr 28, 2020
1f6423a
Split out isRequired logic to separate methods
sbencoding Apr 28, 2020
a5c24a2
Organize tests and fixtures, update affected paths
sbencoding Apr 28, 2020
bf0d816
Split out isRequired flag related tests to separate file, better test…
sbencoding Apr 28, 2020
8b03f9f
Specify fixture path better
sbencoding Apr 28, 2020
854e294
Add isRequired (as function) return value to documentation
sbencoding Apr 28, 2020
e25cfdc
Merge branch 'master' into feature-flag-is-required
sbencoding May 6, 2020
c54437d
Fix broken state after merge
sbencoding May 6, 2020
80cc740
Improve documentation
sbencoding May 6, 2020
019dd32
Simplify code, check isRequired callback return value type
sbencoding May 6, 2020
940fb96
Add test for isRequired callback return value validation
sbencoding May 6, 2020
3c1c710
Add handling of isMultiple flags to the isRequired logic
sbencoding May 6, 2020
22f0a77
Add tests for isRequired and isMultiple options set at the same time
sbencoding May 6, 2020
e11a0ad
Fix estest path
sbencoding May 6, 2020
e5dac8b
Update index.d.ts
sindresorhus May 7, 2020
1e04bdc
Update readme.md
sindresorhus May 7, 2020
59de8b8
Update fixture-required-function.js
sindresorhus May 7, 2020
3dee237
Update fixture-required-multiple.js
sindresorhus May 7, 2020
4b7a7ac
Update fixture-required.js
sindresorhus May 7, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions fixture-required-function.js
@@ -0,0 +1,24 @@
#!/usr/bin/env node
'use strict';
const meow = require('.');

const cli = meow({
description: 'Custom description',
help: `
Usage
foo <input>
`,
flags: {
trigger: {
type: 'boolean',
alias: 't'
},
withTrigger: {
type: 'string',
isRequired: (flags, _) => {
return flags.trigger;
}
}
}
});
console.log(`${cli.flags.trigger},${cli.flags.withTrigger}`);
26 changes: 26 additions & 0 deletions fixture-required.js
@@ -0,0 +1,26 @@
#!/usr/bin/env node
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a lot of fixtures now. Can you move them into a test/fixtures directory?

'use strict';
const meow = require('.');

const cli = meow({
description: 'Custom description',
help: `
Usage
foo <input>
`,
flags: {
test: {
type: 'string',
alias: 't',
isRequired: true
},
number: {
type: 'number',
isRequired: true
},
notRequired: {
type: 'string'
}
}
});
console.log(`${cli.flags.test},${cli.flags.number}`);
6 changes: 6 additions & 0 deletions index.d.ts
Expand Up @@ -2,11 +2,13 @@ import {PackageJson} from 'type-fest';

declare namespace meow {
type FlagType = 'string' | 'boolean' | 'number';
type IsRequiredPredicate = (flags: AnyFlags, input: string[]) => boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
type IsRequiredPredicate = (flags: AnyFlags, input: string[]) => boolean;
type IsRequiredPredicate = (flags: AnyFlags, input: readonly string[]) => boolean;


interface Flag<Type extends FlagType, Default> {
readonly type?: Type;
readonly alias?: string;
readonly default?: Default;
readonly isRequired?: boolean | IsRequiredPredicate
}

type StringFlag = Flag<'string', string>;
Expand All @@ -24,6 +26,7 @@ declare namespace meow {
- `type`: Type of value. (Possible values: `string` `boolean` `number`)
- `alias`: Usually used to define a short flag alias.
- `default`: Default value when the flag is not specified.
- `isRequired`: Boolean or Function that specifies if this flag is required.

@example
```
Expand All @@ -32,6 +35,9 @@ declare namespace meow {
type: 'string',
alias: 'u',
default: 'rainbow'
isRequired: (flags, input) => {
if (flags.otherFlag) return true;
}
}
}
```
Expand Down
34 changes: 34 additions & 0 deletions index.js
Expand Up @@ -51,6 +51,16 @@ const meow = (helpText, options) => {
options.flags
) : options.flags;

// Get a list of required flags
const requiredFlags = [];
if (typeof options.flags !== 'undefined') {
for (const flagName of Object.keys(options.flags)) {
if (options.flags[flagName].isRequired) {
requiredFlags.push(flagName);
}
}
}

let minimistoptions = {
arguments: options.input,
...minimistFlags
Expand Down Expand Up @@ -118,6 +128,30 @@ const meow = (helpText, options) => {
}
}

// Get a list of missing flags, that are required
const missing = [];
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"missing" what? Needs a more descriptive name.

for (const requiredFlag of requiredFlags) {
let requiredByFunction = true;

if (typeof options.flags[requiredFlag].isRequired === 'function') {
requiredByFunction = options.flags[requiredFlag].isRequired(flags, input);
}

if (typeof flags[requiredFlag] === 'undefined' && requiredByFunction) {
missing.push({key: requiredFlag, ...options.flags[requiredFlag]});
}
}

// Print error message for missing flags
if (missing.length > 0) {
console.log(`Missing required option${missing.length > 1 ? 's' : ''}`);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should be consistent with the word "flag" instead of "option".

for (const flag of missing) {
console.log(`\t--${flag.key}${flag.alias ? `, -${flag.alias}` : ''}`);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be console.error?

}

process.exit(2);
}

sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
return {
input,
flags,
Expand Down
4 changes: 4 additions & 0 deletions readme.md
Expand Up @@ -97,6 +97,7 @@ The key is the flag name and the value is an object with any of:
- `type`: Type of value. (Possible values: `string` `boolean` `number`)
- `alias`: Usually used to define a short flag alias.
- `default`: Default value when the flag is not specified.
- `isRequired`: Boolean or Function that specified is this flag is required.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to describe the parameters the function receives.


Example:

Expand All @@ -106,6 +107,9 @@ flags: {
type: 'string',
alias: 'u',
default: 'rainbow'
isRequired: (flags, input) => {
if (flags.otherFlag) return true;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use braces

}
}
}
```
Expand Down
66 changes: 66 additions & 0 deletions test.js
Expand Up @@ -80,6 +80,72 @@ test('spawn cli and test input flag', async t => {
t.is(stdout, 'bar');
});

test('spawn cli and test required flag with missing args', async t => {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move these new tests into a new test file? And also move all tests into a test directory?

try {
await execa('./fixture-required.js', []);
} catch (error) {
const {stdout, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stdout, /Missing required option/);
t.regex(stdout, /--test, -t/);
t.regex(stdout, /--number/);
t.notRegex(stdout, /--notRequired/);
}
});

test('spawn cli and test required flag with all args given', async t => {
const {stdout} = await execa('./fixture-required.js', [
'-t',
'test',
'--number',
'6'
]);
t.is(stdout, 'test,6');
});

test('spawn cli and test required flag with empty string', async t => {
try {
await execa('./fixture-required.js', ['--test', '']);
} catch (error) {
const {stdout, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stdout, /Missing required option/);
t.notRegex(stdout, /--test, -t/);
}
});

test('spawn cli and test required flag with empty number', async t => {
try {
await execa('./fixture-required.js', ['--number']);
} catch (error) {
const {stdout, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stdout, /Missing required option/);
t.regex(stdout, /--number/);
}
});

test('spawn cli and test required (specified as function) flag with without arguments', async t => {
const {stdout} = await execa('./fixture-required-function.js', []);
t.is(stdout, 'false,undefined');
});

test('spawn cli and test required (specified as function) flag with trigger only', async t => {
try {
await execa('./fixture-required-function.js', ['--trigger']);
} catch (error) {
const {stdout, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stdout, /Missing required option/);
t.regex(stdout, /--withTrigger/);
}
});

test('spawn cli and test required (specified as function) flag with trigger and dynamically required option', async t => {
const {stdout} = await execa('./fixture-required-function.js', ['--trigger', '--withTrigger', 'specified']);
t.is(stdout, 'true,specified');
});

test.serial('pkg.bin as a string should work', t => {
meow({
pkg: {
Expand Down