feat(cli): Better CLI args validation

[srawlins]

Start adding better CLI args validation (#603). Validate log-level to start. This doesn't close the issue.

[maksimr]

LGTM

[maksimr]

How about changing

exports.LOG_DISABLE = 'OFF';
exports.LOG_ERROR   = 'ERROR';
exports.LOG_WARN    = 'WARN';
exports.LOG_INFO    = 'INFO';
exports.LOG_DEBUG   = 'DEBUG';

in constants.js to:

exports.LOG.DISABLE = 'OFF';
exports.LOG.ERROR   = 'ERROR';
exports.LOG.WARN    = 'WARN';
exports.LOG.INFO    = 'INFO';
exports.LOG.DEBUG   = 'DEBUG';

Then we can get here something like this

Object.keys(require('constants').LOG)
.map(Function.prototype.call.bind(String.prototype.toLowerCase)).join(', ')

than our knowledge will be in one place constants.js, if we add new log level then it immediately appear here.

[vojtajina]

@maksimr good idea, let's do it in a separate PR.

[dignifiedquire]

Yes please. @srawlins could you please do what @vojtajina and just print the warning and not exit. Other than that I'm happy to get this in.

[dignifiedquire]

@srawlins ping

[srawlins]

Eek, sorry. I'm on vacation for 2 more weeks. Back in late June.
On Jun 3, 2015 9:26 PM, "Friedel Ziegelmayer" notifications@github.com
wrote:

@srawlins https://github.com/srawlins ping

—
Reply to this email directly or view it on GitHub
#1234 (comment).

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please let us know the company's name.

[srawlins]

OK changed behavior to exit on unrecognized log levels, and added a test. Ready for review again. Sorry for the absurd delay 😟

[dignifiedquire]

This could be simpler, I think something like

var logLevel = options.logLevel

if (helper.isString(options.logLevel)) {
  logLevel = constant['LOG_' + options.logLevel.toUpperCase()]
}

if (!helper.isDefined(logLevel)) {
  console.error('Log level must be one of disable, error, warn, info, or debug.');
  process.exit(1);
}

[srawlins]

Actually, if logLevel is not defined (i.e. options.logLevel is not defined), this is valid. It just means the user did not pass a --log-level argument.

[srawlins]

I fixed the lint warnings and there is still a Travis failure that I don't think is related to this PR because it's sporadic. Travis builds #1600 and #1601. It passes for me locally.

[dignifiedquire]

Thanks, looks good to me