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

isRequired does not recognize kebab-case flags #165

Closed
alexmcmanus opened this issue Oct 5, 2020 · 7 comments · Fixed by #168
Closed

isRequired does not recognize kebab-case flags #165

alexmcmanus opened this issue Oct 5, 2020 · 7 comments · Fixed by #168

Comments

@alexmcmanus
Copy link
Contributor

The isRequired option does not seem to work when the flag is defined as kebab-case. I think it's probably looking for the kebab-case key in a map of camelCase flags when it checks to see if it has been provided.

Given this test fixture:

const cli = meow({
	description: "Custom description",
	help: `
		Usage
		  foo <input>
  `,
	flags: {
		test: {
			type: "string",
			alias: "t",
			isRequired: true,
		},
		"kebab-case": {
			type: "string",
			isRequired: true,
		},
		number: {
			type: "number",
			isRequired: true,
		},
		notRequired: {
			type: "string",
		},
	},
});

console.log(`${cli.flags.test},${cli.flags.kebabCase},${cli.flags.number}`);

This test case fails:

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

The failure is:

Missing required flag␊
    	--kebab-case
@sindresorhus
Copy link
Owner

The keys in the flags option are expected to be camel-case.

@alexmcmanus
Copy link
Contributor Author

Thanks, I didn't realize the code matches camelCase flag definitions with kebab-case arguments. The only (minor) consequence is that my help text documents the kebab-case flags, but the isRequired error complains about the missing flag using camelCase.

@sindresorhus
Copy link
Owner

I didn't realize the code matches camelCase flag definitions with kebab-case arguments

Sounds like we need to improve the docs for that.

but the isRequired error complains about the missing flag using camelCase.

That is a bug.

// @sbencoding

@alexmcmanus
Copy link
Contributor Author

Thanks. Given the flag is missing, the only way of influencing how it is reported seems to be either defining the flag in the preferred case, or having some Meow option to control the case used for reporting. The former seems simpler to me, and generally Meow seems to work OK if the the flag keys are kebab-case (although probably they then wouldn't match against camelCase flags if that's how they were provided).

Any thoughts on how to approach this? I'll contribute if I can.

@sindresorhus
Copy link
Owner

Solution:

  • Decamelize the flag from options before showing it in the error message: camelCase => --camel-case.
  • Throw an error if any of the flags option keys contains -, to prevent accidental usage of camel-case instead of camelCase.

@sbencoding
Copy link
Contributor

There's a test case (actually it is the first test case in test.js) named return object, that defines just -- as the flag's name.
Is this a separate case that should be allowed regardless of throwing an error when the flag names contain a - or should this test case be updated because - is not allowed?

@sindresorhus
Copy link
Owner

Is this a separate case that should be allowed regardless of throwing an error when the flag names contain a - or should this test case be updated because - is not allowed?

-- is special as it's an indicator that an argument parser should stop parsing. https://stackoverflow.com/questions/55506492/double-dash-to-stop-flag-parsing

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

Successfully merging a pull request may close this issue.

3 participants