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

Using permitted: restricts the allowed values that a end-user inputs to a pre-defined list #147

Merged
merged 2 commits into from
May 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 40 additions & 7 deletions lib/optimist.rb
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ def initialize(*a, &b)
## [+:default+] Set the default value for an argument. Without a default value, the hash returned by #parse (and thus Optimist::options) will have a +nil+ value for this key unless the argument is given on the commandline. The argument type is derived automatically from the class of the default value given, so specifying a +:type+ is not necessary if a +:default+ is given. (But see below for an important caveat when +:multi+: is specified too.) If the argument is a flag, and the default is set to +true+, then if it is specified on the the commandline the value will be +false+.
## [+:required+] If set to +true+, the argument must be provided on the commandline.
## [+:multi+] If set to +true+, allows multiple occurrences of the option on the commandline. Otherwise, only a single instance of the option is allowed. (Note that this is different from taking multiple parameters. See below.)
## [+:permitted+] Specify an Array of permitted values for an option. If the user provides a value outside this list, an error is thrown.
##
## Note that there are two types of argument multiplicity: an argument
## can take multiple values, e.g. "--arg 1 2 3". An argument can also
Expand Down Expand Up @@ -150,6 +151,7 @@ def opt(name, desc = "", opts = {}, &b)
raise ArgumentError, "you already have an argument named '#{name}'" if @specs.member? o.name
raise ArgumentError, "long option name #{o.long.inspect} is already taken; please specify a (different) :long" if @long[o.long]
raise ArgumentError, "short option name #{o.short.inspect} is already taken; please specify a (different) :short" if @short[o.short]
raise ArgumentError, "permitted values for option #{o.long.inspect} must be either nil or an array;" unless o.permitted.nil? or o.permitted.is_a? Array
@long[o.long] = o.name
@short[o.short] = o.name if o.short?
@specs[o.name] = o
Expand Down Expand Up @@ -330,6 +332,10 @@ def parse(cmdline = ARGV)
params << (opts.array_default? ? opts.default.clone : [opts.default])
end

params[0].each do |p|
raise CommandlineError, "option '#{arg}' only accepts one of: #{opts.permitted.join(', ')}" unless opts.permitted.include? p
end unless opts.permitted.nil?

vals["#{sym}_given".intern] = true # mark argument as specified on the commandline

vals[sym] = opts.parse(params, negative_given)
Expand Down Expand Up @@ -390,7 +396,7 @@ def educate(stream = $stdout)

spec = @specs[opt]
stream.printf " %-#{leftcol_width}s ", left[opt]
desc = spec.description_with_default
desc = spec.full_description

stream.puts wrap(desc, :width => width - rightcol_start - 1, :prefix => rightcol_start)
end
Expand Down Expand Up @@ -585,7 +591,7 @@ def cloaker(&b)

class Option

attr_accessor :name, :short, :long, :default
attr_accessor :name, :short, :long, :default, :permitted
attr_writer :multi_given

def initialize
Expand All @@ -595,6 +601,7 @@ def initialize
@multi_given = false
@hidden = false
@default = nil
@permitted = nil
@optshash = Hash.new()
end

Expand Down Expand Up @@ -639,9 +646,16 @@ def educate
(short? ? "-#{short}, " : "") + "--#{long}" + type_format + (flag? && default ? ", --no-#{long}" : "")
end

## Format the educate-line description including the default-value(s)
def description_with_default
return desc unless default
## Format the educate-line description including the default and permitted value(s)
def full_description
desc_str = desc
desc_str += default_description_str(desc) if default
desc_str += permitted_description_str(desc) if permitted
desc_str
end

## Generate the default value string for the educate line
private def default_description_str str
default_s = case default
when $stdout then '<stdout>'
when $stdin then '<stdin>'
Expand All @@ -651,8 +665,23 @@ def description_with_default
else
default.to_s
end
defword = desc.end_with?('.') ? 'Default' : 'default'
return "#{desc} (#{defword}: #{default_s})"
defword = str.end_with?('.') ? 'Default' : 'default'
" (#{defword}: #{default_s})"
end

## Generate the permitted values string for the educate line
private def permitted_description_str str
permitted_s = permitted.map do |p|
case p
when $stdout then '<stdout>'
when $stdin then '<stdin>'
when $stderr then '<stderr>'
else
p.to_s
end
end.join(', ')
permword = str.end_with?('.') ? 'Permitted' : 'permitted'
" (#{permword}: #{permitted_s})"
end

## Provide a way to register symbol aliases to the Parser
Expand Down Expand Up @@ -691,8 +720,12 @@ def self.create(name, desc="", opts={}, settings={})
## fill in :default for flags
defvalue = opts[:default] || opt_inst.default

## fill in permitted values
permitted = opts[:permitted] || nil

## autobox :default for :multi (multi-occurrence) arguments
defvalue = [defvalue] if defvalue && multi_given && !defvalue.kind_of?(Array)
opt_inst.permitted = permitted
opt_inst.default = defvalue
opt_inst.name = name
opt_inst.opts = opts
Expand Down
11 changes: 11 additions & 0 deletions test/optimist/parser_educate_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,17 @@ def test_help_has_grammatical_default_text
assert help[1] =~ /Default/
assert help[2] =~ /default/
end

def test_help_has_grammatical_permitted_text
parser.opt :arg1, 'description with period.', :type => :strings, :permitted => %w(foo bar)
Copy link
Member

Choose a reason for hiding this comment

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

Does this also work with other types that are not strings? At a minimum I was thinking :string type, but even something like an integer would be useful to constrain. If we can have other types constrained, I think we should also have some specs for those.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@akhoury6 @Fryguy This functionality is was added in Optimist_XL (based on your code/idea that I merged in), see:

Along with some other enhancements to the idea including range / regex matching and permitted_response:

I do get that some of the optimist-xl features aren't for everyone, but if you decide to merge this permitted: feature in it may be worth considering borrowing code (or at least the tests) from Optimist_XL

Copy link
Contributor Author

@akhoury6 akhoury6 Apr 26, 2024

Choose a reason for hiding this comment

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

If I had known you had an OptimistXL I would have just used that instead! For this and the other PRs too.

@Fryguy That's just a test, the code should work with all types. See Line 334.

parser.opt :arg2, 'description without period', :type => :strings, :permitted => %w(foo bar)
sio = StringIO.new 'w'
parser.educate sio

help = sio.string.split "\n"
assert help[1] =~ /Permitted/
assert help[2] =~ /permitted/
end
############

private
Expand Down
8 changes: 8 additions & 0 deletions test/optimist/parser_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,14 @@ def test_required_flags_are_required
assert_raises(CommandlineError) { @p.parse(%w(--arg2 --arg3)) }
end

def test_permitted_flags_filter_inputs
@p.opt "arg", "desc", :type => :strings, :permitted => %w(foo bar)

result = @p.parse(%w(--arg foo))
assert_equal ["foo"], result["arg"]
assert_raises(CommandlineError) { @p.parse(%w(--arg baz)) }
end

## flags that take an argument error unless given one
def test_argflags_demand_args
@p.opt "goodarg", "desc", :type => String
Expand Down