Skip to content

Fix validation of -- options #2104

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

Merged
merged 1 commit into from
Oct 7, 2016
Merged

Conversation

rosen-vladimirov
Copy link
Contributor

@rosen-vladimirov rosen-vladimirov commented Oct 7, 2016

The validation of -- options passed on the command line depends on yargs module. Update it to latest versions as the previously used one didn't set correct values of options marked as strings.
Also in our code we pass object of valid options to yargs. Options which have - in the name are treated in a special manner in yargs. However instead of passing them with -, we pass their secondary representation (remove the - and capitalize the next letter).
This way, when the user passes option with -, yargs treat it with it's default behavior and does not respect our options for it (as we have not defined that we have option with -).
Example:

var yargs = require("yargs");
var opts = {
  "profile-dir": { type: "string" }
};
console.log(yargs(process.argv).options(opts).argv);

var opts1 = {
  "profileDir": { type: "string" }
};
console.log(yargs(process.argv).options(opts1).argv);

When called with:

node testYargs.js --profile-dir

The result is:

{
  'profile-dir': '',
  profileDir: '',
  '$0': 'testYargs.js'
}

{
  'profile-dir': true,
  profileDir: true,
  '$0': 'testYargs.js'
}

As you can see, passing "profileDir" and defining it as string does not work. But when we use "profile-dir" in our opts, everything works as expected and yargs sets the value to empty string.

In order to resolve the problem, make sure we pass options with dashes to yargs.

When boolean value is passed to isNullOrWhitespace helper method, it fails as true.replace is not available function.

Add unit tests for:

  • options issue - assert correct behavior when dashed option is passed on the terminal
  • helpers isNullOrWhitespace method
  • helpers isBoolean method

Changes in mobile-cli-lib are here: telerik/mobile-cli-lib#819
Fixes #2094

The validation of -- options passed on the command line depends on yargs module. Update it to latest versions as the previously used one didn't set correct values of options marked as strings.
Also in our code we pass object of valid options to yargs. Options which have `-` in the name are treated in a special manner in yargs. However instead of passing them with `-`, we pass their secondary representation (remove the `-` and capitalize the next letter).
This way, when the user passes option with `-`, yargs treat it with it's default behavior and does not respect our options for it (as we have not defined that we have option with `-`).
Example:
```JavaScript
var yargs = require("yargs");
var opts = {
  "profile-dir": { type: "string" }
};
console.log(yargs(process.argv).options(opts).argv);

var opts1 = {
  "profileDir": { type: "string" }
};
console.log(yargs(process.argv).options(opts1).argv);
```

When called with:
```
node testYargs.js --profile-dir
```
The result is:
```
{
  'profile-dir': '',
  profileDir: '',
  '$0': 'testYargs.js'
}

{
  'profile-dir': true,
  profileDir: true,
  '$0': 'testYargs.js'
}
```

As you can see, passing "profileDir" and defining it as string does not work. But when we use "profile-dir" in our opts, everything works as expected and yargs sets the value to empty string.

In order to resolve the problem, make sure we pass options with dashes to yargs.

When boolean value is passed to isNullOrWhitespace helper method, it fails as true.replace is not available function.

Add unit tests for:
 - options issue - assert correct behavior when dashed option is passed on the terminal
 - helpers isNullOrWhitespace method
 - helpers isBoolean method
@rosen-vladimirov rosen-vladimirov merged commit 6180c8e into master Oct 7, 2016
@rosen-vladimirov rosen-vladimirov deleted the vladimirov/fix-dashed-opts branch October 7, 2016 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"--framework-path" with no argument fails with "input.replace is not a function" message
2 participants