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

Rewrite flags input #63

Merged

Conversation

vadimdemedes
Copy link

Uses minimist-options to define minimist options for flags more comfortably.

Before:

const cli = meow({
	description: 'Custom description',
}, {
	alias: {
		u: 'unicorn'
	},
	default: {
		meow: 'dog',
		camelCaseOption: 'foo'
	}
});

After:

const cli = meow({
	description: 'Custom description',
	flags: {
		unicorn: {alias: 'u'},
		meow: {default: 'dog'},
		camelCaseOption: {default: 'foo'}
	}
});

Note to @sindresorhus: removed the docs from readme, because you wanted to bundle args documentation inside.

@SamVerschueren
Copy link
Contributor

Awesome! This looks much much better!

@sindresorhus Might be a good idea to update aliases as well. Or maybe we should create a new one?

@vadimdemedes vadimdemedes force-pushed the integrate-minimist-options branch from 759fa01 to 2730a67 Compare May 31, 2017 10:10
@sindresorhus
Copy link
Owner

I found an issue. If I use:

flags: {
		foo: {
			type: 'boolean',
			alias: 'f'
		}
	}

And pass in $ cli -f, I get flags: { f: true },, but I would have expected flags: { foo: true },. Needs a regression test too.

@sindresorhus
Copy link
Owner

I'm also not sure about the special arguments name. It kinda feels weird to have it in the flags option. Maybe we should just have a separate top-level option for it here in Meow. So it would be:

const cli = meow({
	arguments: 'string',
	flags: {
		foo: {
			type: 'boolean',
			alias: 'f'
		}
	}
});

Maybe also call it input, to match the cli.input property?

@vadimdemedes
Copy link
Author

Agree about arguments => input.

@vadimdemedes
Copy link
Author

Re issue with an alias, I couldn't reproduce it. I added and pushed a test to be sure and demo it. Plus, several other existing tests validate the alias functionality.

@sindresorhus
Copy link
Owner

Weird. I'll try to create a failing test.

@sindresorhus
Copy link
Owner

Ah, it's because I have the options as the second argument, so they're not read at all.

const cli = meow(`
	Usage
	  $ internal-ip
`, {
	flags: {
		foo: {
			type: 'boolean',
			alias: 'f'
		}
	}
});

The above should be supported too.

@vadimdemedes
Copy link
Author

Added support for specifying options both as first and second arguments.

@sindresorhus sindresorhus merged commit 43401c3 into sindresorhus:master Sep 23, 2017
@sindresorhus
Copy link
Owner

@vadimdemedes Sorry totally forgot about this. I'll write some docs for it tomorrow.

@vadimdemedes
Copy link
Author

@sindresorhus No probs ;) Glad it got merged in!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants