Skip to content
This repository has been archived by the owner on Jul 14, 2021. It is now read-only.

Catch bad params #468

Merged
merged 4 commits into from
Aug 14, 2015
Merged

Catch bad params #468

merged 4 commits into from
Aug 14, 2015

Conversation

danielsdeleo
Copy link
Contributor

Adds error handling for the case where a CLI option requires an argument but none is given. For example, the -c option requires an argument (path to a config file), so a command like chef show-policy -c is not valid. Currently, this error bubbles up for most commands so you get output like this:

/opt/chefdk/embedded/lib/ruby/gems/2.1.0/gems/mixlib-cli-1.5.0/lib/mixlib/cli.rb:191:in `parse_options': missing argument: -c (OptionParser::MissingArgument)
    from /opt/chefdk/embedded/apps/chef-dk/lib/chef-dk/command/show_policy.rb:136:in `apply_params!'
    from /opt/chefdk/embedded/apps/chef-dk/lib/chef-dk/command/show_policy.rb:90:in `run'
    from /opt/chefdk/embedded/apps/chef-dk/lib/chef-dk/command/base.rb:57:in `run_with_default_options'
    from /opt/chefdk/embedded/apps/chef-dk/lib/chef-dk/cli.rb:73:in `run'
    from /opt/chefdk/embedded/apps/chef-dk/bin/chef:25:in `<top (required)>'
    from /usr/bin/chef:74:in `load'
    from /usr/bin/chef:74:in `<main>'

With this patch, the output is now:

bundle exec bin/chef show-policy -c
ERROR: missing argument: -c

Usage: chef show-policy [POLICY_NAME [POLICY_GROUP]] [options]

`chef show-policy` Displays the revisions of policyfiles on the server. By
default, only active policy revisions are shown. Use the `--orphans` options to
show policy revisions that are not assigned to any policy group.

When both POLICY_NAME and POLICY_GROUP are given, the command shows the content
of a the active policyfile lock for the given POLICY_GROUP. See also the `diff`
command.

The Policyfile feature is incomplete and beta quality. See our detailed README
for more information.

https://github.com/opscode/chef-dk/blob/master/POLICYFILE_README.md

Options:

    -c, --config CONFIG_FILE         Path to configuration file
    -D, --debug                      Enable stacktraces and other debug output
        --[no-]pager                 Enable/disable paged policyfile lock ouput (default: enabled)
    -o, --orphans                    Show policy revisions that are unassigned

I think this is more verbose than is ideal, but it's simple and it at least shows the usage for the option. Without the usage, the user's next step would probably be to run command -h to see the usage, which would be equally verbose. Improving this significantly (such as omitting the text description of the command) would require a bunch more changes. Since this error case shouldn't be that frequent, this seems reasonable to me.

@thommay
Copy link
Contributor

thommay commented Aug 14, 2015

LGTM, and like you say, this is exactly the same UX as getting an error and then running --help, all neatly compressed into one action. 👍

@tyler-ball
Copy link
Contributor

👍

1 similar comment
@ksubrama
Copy link

👍

@danielsdeleo danielsdeleo merged commit 4536097 into master Aug 14, 2015
@danielsdeleo danielsdeleo deleted the catch-bad-params branch August 14, 2015 16:06
danielsdeleo added a commit that referenced this pull request Aug 14, 2015
@jkeiser jkeiser added the Bug label Sep 22, 2015
ksubrama pushed a commit that referenced this pull request Jan 11, 2016
Revert version pins in chef-dk.
@thommay thommay added Type: Bug Doesn't work as expected. and removed Bug labels Feb 1, 2017
@chef-boneyard chef-boneyard locked and limited conversation to collaborators Feb 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Doesn't work as expected.
Development

Successfully merging this pull request may close these issues.

6 participants