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

Fix command name identification #89

Conversation

@orien
Copy link
Contributor

orien commented Mar 8, 2020

Context

In issue #32, it is reported that global options are not available at command level when placed before command name. I found this to be a defect in the identification of command names from arguments.

def valid_command_names_from(*args)
arg_string = args.delete_if { |value| value =~ /^-/ }.join ' '
commands.keys.find_all { |name| name if arg_string =~ /^#{name}\b/ }
end

This method was not correctly accounting for global options with provided values. ie. in the example provided by @msabramo:

aptly-cli --server 10.3.0.46 repo_list

It would incorrectly determine the args_string to be "10.3.0.46 repo_list" thus expecting the command name to start with 10.3.0.46.

Proposed Fix

I propose the valid_command_names_from method be modified to remove all global options from the args. This way it correctly identify the command name(s).

This will result in the global_option_proc finding a non-nil active_command and thus provide it the global option.

def global_option_proc(switches, &block)
lambda do |value|
unless active_command.nil?
active_command.global_options << [Runner.switch_to_sym(switches.last), value]
end
yield value if block && !value.nil?
end
end

Fixes #32.

Specifically, in the case where a global option value is provided before
the command name.
@ggilder

This comment has been minimized.

Copy link
Member

ggilder commented Mar 9, 2020

This looks great! Thanks for the fix, I'm glad it was relatively simple.

@ggilder ggilder merged commit d3127b1 into commander-rb:master Mar 9, 2020
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@ggilder

This comment has been minimized.

Copy link
Member

ggilder commented Mar 12, 2020

Released in v4.5.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.