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

Show help if mandatory arguments not used #546

Closed
pedro93 opened this issue Jun 6, 2016 · 15 comments
Closed

Show help if mandatory arguments not used #546

pedro93 opened this issue Jun 6, 2016 · 15 comments

Comments

@pedro93
Copy link

pedro93 commented Jun 6, 2016

I have the following code:

const version = JSON.parse(fs.readFileSync(path.join(__dirname, './package.json')) + '').version;

program
.version(version)
.arguments('<engine>')
.option('-u, --url <link>','indicates that the address parameter is to be interpreted as an url (value by default)')
.option('-f, --file <link>','indicates that the address parameter is to be interpreted as a file')
.action(main)

program.parse(process.argv);
if (!process.argv.slice(2).length) {
    program.outputHelp();
}

function main(engine){
    console.log(engine);
    if(program.file) {
        renderer.execute(program.file,engine, {file: true}).then(console.log);
    } else {
        renderer.execute(program.url, engine).then(console.log).done();
    }
};

I was trying to have the program output the help text if the argument engine is not specified. Is there any built-in way to do this? At the moment the code works perfectly if I specify the engine. but if I just execute with any arguments then nothing is shown.

Thanks for taking the time to read this.

@jamesr73
Copy link
Contributor

jamesr73 commented Jun 26, 2016

@pedro93

You could try testing commander.args in addition to process.argv. Bear in mind that it may contain a circular reference to commander.

I'm seeing inconsistent behaviour with the <required> arguments. I have a subcommand with arguments('<one> <two>') and a check similar to yours because the action isn't triggered if they aren't passed (as per the docs). So I was surprised to see that if I pass <one> but not <two> I get the error:

error: missing required argument 'two'

So I updated test/test.arguments.js to match this scenario and it passes. Unless I add a <third> required argument. The code has provision for flagging arguments as required/optional but isn't checking them consistently.

@shadowspawn
Copy link
Collaborator

shadowspawn commented Mar 30, 2019

The original poster asked about mandatory non-option arguments. There is not currently automatic support for this.

In the simple example with an argument to the program you can make a simple test for whether anything is left after the options are consumed by testing program.args.

const program = require("commander");

program
.arguments('<engine>')
.option('-u, --url <link>','indicates that the address parameter is to be interpreted as an url (value by default)')
.option('-f, --file <link>','indicates that the address parameter is to be interpreted as a file')
.action((engine) => {
    console.log(`Action called with ${engine}`);
});

program.parse(process.argv);
if (program.args.length == 0) {
    console.log("Would show help");
}
$ node index.js
Would show help
$ node index.js aaa
Action called with aaa
$ node index.js -u bbb
Would show help
$ node index.js -u ccc ddd
Action called with ddd

Repository owner deleted a comment from Queuecumber May 13, 2019
Repository owner deleted a comment from omgimanerd May 13, 2019
@shadowspawn
Copy link
Collaborator

(Note: deleted a couple of comments in this thread that were not contributing to improving this package.)

@shadowspawn
Copy link
Collaborator

This issue has not seen much activity in the passing years. Feel free to open a new issue if it comes up again, with new information and renewed interest.

Closing in favour of #230.

Thank you for your contributions.

@Copainbig
Copy link

Hi guys,

I am working on simple cli tool, and encountered the same error but I feel like it was never really addressed.
Indeed #230 propose a way to use ".requiredOption" for mandatory flag options. But the point here is the missing ".arguments(<this_argument_is_missing>)" of the top level command.

I there a proper way to handle that missing argument if we want to display an error message or the help (without having to write our own argv parser)? If not, I am okay to work on that.

@shadowspawn
Copy link
Collaborator

There is not currently automatic support for displaying the help if no arguments are supplied, in the example program of original poster.

I gave one approach for detecting in: #546 (comment)

And there is an open issue about detecting including more complicated cases, with some suggestions, in: #1088

@Copainbig
Copy link

Oh indeed your first approach could solve my issue.
I will take time reading about the more complicated cases and see if I can help on that.
Thank you.

@shadowspawn
Copy link
Collaborator

If interested, see also #1062 for some discussion and digging into current state of affairs, and multiple ways command:* gets sent and used.

@michga
Copy link
Contributor

michga commented Jul 24, 2020

As of today, #546 (comment) does not work anymore ([email protected])

C:\code\lib>node index.js
error: missing required argument 'engine'

C:\code\lib>node index.js aaa
Action called with aaa

C:\code\lib>node index.js -u bbb
error: missing required argument 'engine'

Finally, I cannot get this sample to display "no command given!"
https://github.com/tj/commander.js#specify-the-argument-syntax

I'll just check the process.argv.length before the parse command.

my ugly workaround
let args = process.argv.slice(2);
let arg, foundArg = false;
do {
    arg = args.shift();
    if (arg) {
        arg.replace(/^--/, '-');
        switch (arg[1]) {
            case 'c':
            case 'n':
                // c and n are 2 optional parameters that specify a single, assuming the user did things correctly here :/
                args.shift();
                break;
            case 'd':
                // optional boolean parameter
                break;
            default:
                foundArg = true;
                break;
        }
    }
} while (!foundArg && args.length > 0);
if (!foundArg) {
    program.help();
}
program.parse(process.argv);
Slightly uglier workaround but more reliable
program.exitOverride();
try {
    program.parse(process.argv);
} catch (err) {
    program._exitCallback = null;
    program.help();
}

@shadowspawn
Copy link
Collaborator

@michga

The behaviour changed in 5.0.0:

Looks like I missed that entry in the README! I opened #1311 to update that.

@shadowspawn
Copy link
Collaborator

I think you could use .parseOptions() to do the work for you to look for missing non-option arguments, which would be in the spirit of the previous pattern. Instead of hand parsing. It leaves behind state so you'll want to make a different copy of the program. If that sounds like what you want and you would like an example, I can add one.

@michga
Copy link
Contributor

michga commented Jul 25, 2020

@shadowspawn thank you for your input.
I couldn't find a way with .parseOptions().
After some additional digging, I came up with this, which seems acceptable in terms of compliancy since .outputHelp() is marked as public:

program.exitOverride(() => {
    // program.args is not defined when asking for the version -V
    let args = program.args || [];
    //  this prevents duplicating the help when asking for it, not ideal but it seems to work
    if (!(args.includes('-h') || args.includes('--help'))) {
        program.outputHelp();
    }
});
program.parse(process.argv);

The only minor issue that remains is that the help is now displayed when asking for the version since it doesn't show up in the args but I can live with that.

commander.js/index.js

Lines 1587 to 1596 in c5a5e7b

/**
* Output help information for this command.
*
* When listener(s) are available for the helpLongFlag
* those callbacks are invoked.
*
* @api public
*/
outputHelp(cb) {

@shadowspawn
Copy link
Collaborator

I had to remind myself what .parseOptions returns. This is the pre-flighting I had in mind to check for no arguments after top-level parse.

const { Command } = require("commander");

function makeProgram() {
  const program = new Command();
  program
    .arguments('<engine>')
    .option('-u, --url <link>','indicates that the address parameter is to be interpreted as an url (value by default)')
    .option('-f, --file <link>','indicates that the address parameter is to be interpreted as a file')
    .action((engine) => {
        console.log(`Action called with ${engine}`);
    });
    return program;
  }

if (makeProgram().parseOptions(process.argv).operands.length <= 2) {
  makeProgram().help();
}

makeProgram().parse(process.argv);

@shadowspawn
Copy link
Collaborator

shadowspawn commented Jul 25, 2020

And here is a more specific implementation of .exitOverride to look for the error of interest:

const { program } = require("commander");

program.exitOverride((err) => {
  if (err.code === 'commander.missingArgument') {
    program.outputHelp();
  }
  process.exit(err.exitCode);
});

program
  .arguments('<engine>')
  .option('-u, --url <link>','indicates that the address parameter is to be interpreted as an url (value by default)')
  .action((engine) => {
    console.log(`Action called with ${engine}`);
});

program.parse();

@michga
Copy link
Contributor

michga commented Jul 25, 2020

@shadowspawn Thank you, this implementation you gave does exactly what I wanted.

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

No branches or pull requests

5 participants