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 13 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
12 changes: 12 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,11 @@ 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.
Two arguments are passed to the function.
The first arguments is the flags object, it contains the flags converted to camelCase excluding aliases.
The second arugment is the input string array, it contains the non-flag arguments.
The function should return a Boolean, true if the flag is requried, otherwise false.
Copy link
Owner

Choose a reason for hiding this comment

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

The function docs should be at https://github.com/sindresorhus/meow/pull/141/files#diff-b52768974e6bc0faccb7d4b75b162c99R5 and you don't have to mention the types. It's already shown by the actual types. Mention what it does and what problem it solves.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was just trying to make sure that the index.d.ts and the readme.md file reflect each other as much as possible, that's why I left the type information there, because although typescript has a better way of documenting type information, in the readme file this is the only way.
Anyways I left the documentation with the type information in the readme file and updated the typescript file with a better documentation


@example
```
Expand All @@ -32,6 +39,11 @@ declare namespace meow {
type: 'string',
alias: 'u',
default: 'rainbow'
isRequired: (flags, input) => {
if (flags.otherFlag) {
return true;
}
}
}
}
```
Expand Down
40 changes: 40 additions & 0 deletions index.js
Expand Up @@ -14,6 +14,37 @@ const normalizePackageData = require('normalize-package-data');
delete require.cache[__filename];
const parentDir = path.dirname(module.parent.filename);

const isFlagMissing = (flagName, definedFlags, receivedFlags, input) => {
const flag = definedFlags[flagName];
let isFlagRequired = true;

if (typeof flag.isRequired === 'function') {
isFlagRequired = flag.isRequired(receivedFlags, input);
}

return typeof receivedFlags[flagName] === 'undefined' && isFlagRequired;
};

const getMissingRequiredFlags = (flags, receivedFlags, input) => {
const missingRequiredFlags = [];
if (typeof flags !== 'undefined') {
Copy link
Owner

Choose a reason for hiding this comment

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

Use an early-return here to reduce the nesting.

for (const flagName of Object.keys(flags)) {
if (flags[flagName].isRequired && isFlagMissing(flagName, flags, receivedFlags, input)) {
missingRequiredFlags.push({key: flagName, ...flags[flagName]});
}
}
}

return missingRequiredFlags;
};

const reportMissingRequiredFlags = missingRequiredFlags => {
console.error(`Missing required flag${missingRequiredFlags.length > 1 ? 's' : ''}`);
for (const flag of missingRequiredFlags) {
console.error(`\t--${flag.key}${flag.alias ? `, -${flag.alias}` : ''}`);
}
};

const meow = (helpText, options) => {
if (typeof helpText !== 'string') {
options = helpText;
Expand Down Expand Up @@ -118,6 +149,15 @@ const meow = (helpText, options) => {
}
}

// Get a list of missing flags that are required
const missingRequiredFlags = getMissingRequiredFlags(options.flags, flags, input);

// Print error message for missing flags that are required
if (missingRequiredFlags.length > 0) {
reportMissingRequiredFlags(missingRequiredFlags);
process.exit(2);
}

sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
return {
input,
flags,
Expand Down
5 changes: 5 additions & 0 deletions package.json
Expand Up @@ -63,5 +63,10 @@
"rules": {
"unicorn/no-process-exit": "off"
}
},
"ava": {
"files": [
"test/*"
]
}
}
10 changes: 10 additions & 0 deletions readme.md
Expand Up @@ -97,6 +97,11 @@ 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 specifies if this flag is required.
Two arguments are passed to the function.
The first arguments is the **flags** object, it contains the flags converted to camelCase excluding aliases.
The second arugment is the **input** string array, it contains the non-flag arguments.
Copy link
Owner

Choose a reason for hiding this comment

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

typo

The function should return a Boolean, true if the flag is requried, otherwise false.
Copy link
Owner

Choose a reason for hiding this comment

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

typo


Example:

Expand All @@ -106,6 +111,11 @@ flags: {
type: 'string',
alias: 'u',
default: 'rainbow'
isRequired: (flags, input) => {
if (flags.otherFlag) {
return true;
}
sindresorhus marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
```
Expand Down
24 changes: 24 additions & 0 deletions test/fixtures/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 test/fixtures/fixture-required.js
@@ -0,0 +1,26 @@
#!/usr/bin/env node
'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}`);
2 changes: 1 addition & 1 deletion fixture.js → test/fixtures/fixture.js
@@ -1,6 +1,6 @@
#!/usr/bin/env node
'use strict';
const meow = require('.');
const meow = require('../..');

const cli = meow({
description: 'Custom description',
Expand Down
72 changes: 72 additions & 0 deletions test/test-is-required-flag.js
@@ -0,0 +1,72 @@
import test from 'ava';
Copy link
Owner

Choose a reason for hiding this comment

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

The filename can just be is-required-flag.js

import execa from 'execa';
const path = require('path');

const fixtureRequiredPath = path.join(__dirname, 'fixtures', 'fixture-required.js');
const fixtureRequiredFunctionPath = path.join(__dirname, 'fixtures', 'fixture-required-function.js');

test('spawn cli and test not specifying required flags', async t => {
try {
await execa(fixtureRequiredPath, []);
} catch (error) {
const {stderr, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stderr, /Missing required flag/);
t.regex(stderr, /--test, -t/);
t.regex(stderr, /--number/);
t.notRegex(stderr, /--notRequired/);
}
});

test('spawn cli and test specifying all required flags', async t => {
const {stdout} = await execa(fixtureRequiredPath, [
'-t',
'test',
'--number',
'6'
]);
t.is(stdout, 'test,6');
});

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

test('spawn cli and test specifying required number flag without a number', async t => {
try {
await execa(fixtureRequiredPath, ['--number']);
} catch (error) {
const {stderr, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stderr, /Missing required flag/);
t.regex(stderr, /--number/);
}
});

test('spawn cli and test setting isRequired as a function and not specifying any flags', async t => {
const {stdout} = await execa(fixtureRequiredFunctionPath, []);
t.is(stdout, 'false,undefined');
});

test('spawn cli and test setting isRequired as a function and specifying only the flag that activates the isRequired condition for the other flag', async t => {
try {
await execa(fixtureRequiredFunctionPath, ['--trigger']);
} catch (error) {
const {stderr, message} = error;
t.regex(message, /Command failed with exit code 2/);
t.regex(stderr, /Missing required flag/);
t.regex(stderr, /--withTrigger/);
}
});

test('spawn cli and test setting isRequired as a function and specifying both the flags', async t => {
const {stdout} = await execa(fixtureRequiredFunctionPath, ['--trigger', '--withTrigger', 'specified']);
t.is(stdout, 'true,specified');
});
25 changes: 14 additions & 11 deletions test.js → test/test.js
@@ -1,8 +1,11 @@
import test from 'ava';
import indentString from 'indent-string';
import execa from 'execa';
import pkg from './package.json';
import meow from '.';
import pkg from '../package.json';
import meow from '..';
const path = require('path');

const fixturePath = path.join(__dirname, 'fixtures', 'fixture.js');

test('return object', t => {
const cli = meow({
Expand Down Expand Up @@ -36,47 +39,47 @@ test('support help shortcut', t => {
});

test('spawn cli and show version', async t => {
const {stdout} = await execa('./fixture.js', ['--version']);
const {stdout} = await execa(fixturePath, ['--version']);
t.is(stdout, pkg.version);
});

test('spawn cli and disabled autoVersion and autoHelp', async t => {
const {stdout} = await execa('./fixture.js', ['--version', '--help']);
const {stdout} = await execa(fixturePath, ['--version', '--help']);
t.is(stdout, 'version\nhelp\nmeow\ncamelCaseOption');
});

test('spawn cli and disabled autoVersion', async t => {
const {stdout} = await execa('./fixture.js', ['--version', '--no-auto-version']);
const {stdout} = await execa(fixturePath, ['--version', '--no-auto-version']);
t.is(stdout, 'version\nautoVersion\nmeow\ncamelCaseOption');
});

test('spawn cli and not show version', async t => {
const {stdout} = await execa('./fixture.js', ['--version=beta']);
const {stdout} = await execa(fixturePath, ['--version=beta']);
t.is(stdout, 'version\nmeow\ncamelCaseOption');
});

test('spawn cli and show help screen', async t => {
const {stdout} = await execa('./fixture.js', ['--help']);
const {stdout} = await execa(fixturePath, ['--help']);
t.is(stdout, indentString('\nCustom description\n\nUsage\n foo <input>\n\n', 2));
});

test('spawn cli and disabled autoHelp', async t => {
const {stdout} = await execa('./fixture.js', ['--help', '--no-auto-help']);
const {stdout} = await execa(fixturePath, ['--help', '--no-auto-help']);
t.is(stdout, 'help\nautoHelp\nmeow\ncamelCaseOption');
});

test('spawn cli and not show help', async t => {
const {stdout} = await execa('./fixture.js', ['--help=all']);
const {stdout} = await execa(fixturePath, ['--help=all']);
t.is(stdout, 'help\nmeow\ncamelCaseOption');
});

test('spawn cli and test input', async t => {
const {stdout} = await execa('./fixture.js', ['-u', 'cat']);
const {stdout} = await execa(fixturePath, ['-u', 'cat']);
t.is(stdout, 'unicorn\nmeow\ncamelCaseOption');
});

test('spawn cli and test input flag', async t => {
const {stdout} = await execa('./fixture.js', ['--camel-case-option', 'bar']);
const {stdout} = await execa(fixturePath, ['--camel-case-option', 'bar']);
t.is(stdout, 'bar');
});

Expand Down