For args that have skipValidation set to true, check if the parsed arg is true

[faazshift]

When an arg is defined, it will always exist in the parsed argv. Thus, validation would always be skipped, regardless of whether an arg that has skipValidation set to true is actually passed on the command line. This commit checks to see if the parsed argv value is actually set to true.

[bcoe]

@faazshift awesome \o/

mind adding a test for this behavior? otherwise, looks great.

[faazshift]

@bcoe Thanks! A test is now added.

[maxrimue]

LGTM 👍

[bcoe]

@faazshift looks good to me too! one comment though, noticing skipValidation isn't in the docs; do you want to add a section as part of this pull, or I'm happy to follow this pull request up with an addition to the docs.

[faazshift]

@bcoe This should already be in the docs. Specifically, in the opts section here: https://github.com/yargs/yargs/blob/master/README.md#optionskey-opt

[bcoe]

@faazshift looks good to me, don't know how I missed that.

[bcoe]

@faazshift mind giving this a spin:

npm cache clear; yargs@6.0.0-alpha.1

Your work will go out with the next release 👍

[faazshift]

@bcoe In doing some testing yesterday, my use-case now works as expected. However after some more extensive testing, I realized that more extensive fixes needed to be made to make things actually work how they are supposed to. Specifically, I needed to make some changes to yargs/yargs-parser to support some more changes here. I will open a new PR on both repos with my proposed changes.

Update: New PRs are #645 and yargs/yargs-parser#60

[bcoe]

Hey @faazshift mind providing an example of some of the edge-cases you're thinking of? I think we might be able to avoid the changes to yargs-parser by changing the logic slightly. I think basically we want:

• check if skip-validation === true if typeof skip-validation = boolean.
• check if skip-validation is truthy otherwise., this would allow for the case of strings.

[faazshift]

@bcoe The problem exists in the fact that the data returned from yargs-parser gives no indication of which options were passed on the command line. The argv that is being checked in the skipValidation logic (returned from yargs-parser) includes all configured options, along with their computed values (passed or default). Essentially, that logic is comparing against the value of the option, not whether that option's skipValidation is true. Without either augmenting yargs-parser (as I have attempted to do), or manually extracting option names passed on the command line in the yargs code base (hackish at best, in my opinion), there is no real way to tell if any of the options configured with skipValidation: true were actually passed on the command line.

Not sure if all that made sense, but that's why I made the changes I did in my PR for yargs-parser. If it would be preferred to approach this problem in a different way, I'm all for it. However we approach it, though, the current logic is wrong (though it accomplishes the goal in limited circumstances) and I believe we will somehow need to augment yargs-parser to indicate what options were passed on the command line.