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

enforce option values where needed #1918

Merged
merged 1 commit into from
Jun 12, 2017
Merged

enforce option values where needed #1918

merged 1 commit into from
Jun 12, 2017

Conversation

arlimus
Copy link
Contributor

@arlimus arlimus commented Jun 11, 2017

Due to limitations in Thor it is not possible to set an argument to be both optional and its value to be mandatory. E.g. the user supplying the --password argument is optional and not always required, but whenever it is used, it requires a value. Handle options that were defined with mandatory values in a way that fails with an ArgumentError if the value is missing, i.e.:

> inspec exec examples/profile --password
ArgumentError: Please provide a value for --password. For example: --password=hello.

It works without --password or with --password=arg. Also handled for --sudo-password.

Fixes: #1901
An alternative approach: #1904

if o[v.to_sym] == -1
raise ArgumentError, "Please provide a value for --#{v}. For example: --#{v}=hello."
end
end
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The trick here is that due to the type constraint these values will never reach -1, even if called with --password=-1 as it's turned into "-1". We can't use nil because it just says the default is not defined (or really it's empty, since it cannot check for "not defined"), so this looked like a nice alternative. We could also take the golang approach and pass a {}, but since it's a full hash in ruby unlike Golang's empty interface I just picked the next best thing...

Due to limitations in Thor it is not possible to set an argument to be both optional and its value to be mandatory. E.g. the user supplying the --password argument is optional and not always required, but whenever it is used, it requires a value. Handle options that were defined with mandatory values in a way that fails with an `ArgumentError` if the value is missing, i.e.:

```
> inspec exec examples/profile --password
ArgumentError: Please provide a value for --password. For example: --password=hello.
```

It works without `--password` or with `--password=arg`. Also handled for `--sudo-password`.

Fixes: #1901
As suggested: #1904

Signed-off-by: Dominik Richter <dominik.richter@gmail.com>
Copy link
Contributor

@adamleff adamleff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty crafty way of handling this. And TIL about lazy_default in Thor. Thanks, @arlimus!

@adamleff adamleff merged commit 9e3706a into master Jun 12, 2017
@adamleff adamleff added Type: Enhancement Improves an existing feature and removed in progress labels Jun 12, 2017
@adamleff adamleff changed the title bugfix: enforce option values where needed enforce option values where needed Jun 12, 2017
@adamleff adamleff added Type: Bug Feature not working as expected and removed Type: Enhancement Improves an existing feature labels Jun 12, 2017
@arlimus arlimus deleted the dr/mandatory-opts-args branch June 13, 2017 14:31
@arlimus
Copy link
Contributor Author

arlimus commented Jun 14, 2017

😁 kudos Adam!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Feature not working as expected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants