Error on unsupported CLI arguments

[mnsn]

Summary
I fixed issue #2600 , by listening to the commander event emitter and exit with error when unknown flag is been added

[chrisbujok]

@mnsn Can I ask why did you close this PR? I was thinking about solving the issue you mentioned.

[mnsn]

The unit tests failed I need to check what I broke

[chrisbujok]

Sure. Please let me know if I can help - I'd be really happy to fix this issue.

[mnsn]

I fixed the issues, thank you for the offering Chris.
Maybe Do you understand what is the problem with appveyor ? And how to solve it?

[bestander]

Hey, @mnsn.
Thanks for the PR, it can catch a bunch of unexpected errors.

However allowing unsupported flags is a feature, this way Yarn achieves forwards compatibility, e.g. a new flag is used in a build system but not whole organization has updated to using the new version.

How about we just print a warning when flag is unrecognized?

Sorry for a delay in review, the title did was not inviting very much :)
Are you up to rebase and fix?

[bestander]

Closing for now, looking for people to give it another go

[jseminck]

I'd like to take a look at this. I can base myself on the original code from this PR, but change it to warn instead of error?

[bestander]

@jseminck, yep, give it a try

[jseminck]

I had a look into this issue, and it seems rather hard.

First of all, I noticed yarn doesn't use the most recent version of commander.js. Are there any plans to upgrade?

The only way I see to get the array of unknown options from commander.js is by listening to the command: event: https://github.com/tj/commander.js/blob/master/index.js#L627

However, yarn has some special logic to deal with an issue in commander.js: and adds + removes some special 'this-arg-will-get-stripped-later' argument:
https://github.com/yarnpkg/yarn/blob/master/src/cli/index.js#L105
https://github.com/yarnpkg/yarn/blob/master/src/cli/index.js#L114

That means that to listen to the correct command event, we'd need to listen to:

commander.on('this-arg-will-get-stripped-later', (args, unknown) => {
  console.log('unknown', unknown);
))

But listening to a specific command in commander.js, will also get it removed from the arguments array (see again: https://github.com/tj/commander.js/blob/master/index.js#L627 - thus making the code in cli/index.js#L114 unnecessary anymore.

This all seems to make things rather more complicated inside yarn.

My suggestion would be to make a PR to commander.js and also pass unknown as an arg to command:* listeners: https://github.com/tj/commander.js/blob/master/index.js#L629 (unless there is a specific reason why they did not do that).

Then in yarn we can listen to that event and get the unknown options from there.

WDYT?