sindresorhus / meow

🐈 CLI app helper
MIT License
3.53k stars 150 forks source link

isRequired does not recognize kebab-case flags #165

Closed alexmcmanus closed 3 years ago

alexmcmanus commented 3 years ago

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 commented 3 years ago

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

alexmcmanus commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

Solution:

sbencoding commented 3 years ago

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 commented 3 years ago

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