Skip to content

Commit

Permalink
Flags with optional args would eat following flags. Fixed.
Browse files Browse the repository at this point in the history
I've added some specs to check for this situation, and cleaned up
the logic that was leading to it. The previous logic failed because
of the assumption that anything that isn't FlagValue::None should
get the next value. The use of a case statement makes the thinking
more explicit.
  • Loading branch information
yb66 committed Sep 11, 2021
1 parent b88b842 commit 169bdd2
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 10 deletions.
83 changes: 83 additions & 0 deletions spec/std/option_parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,89 @@ describe "OptionParser" do
expect_capture_option ["-f", "-g -h"], "-f FLAG", "-g -h"
end

describe "Consumption of flags following an ungiven optional argument" do
context "Given a short option with an optional value" do
it "doesn't eat a following short option" do
flag = nil
not_eaten = [] of String
args = ["-f", "-g", "-i"]
OptionParser.parse(args) do |opts|
opts.on("-f [FLAG]", "some flag") do |flag_value|
flag = flag_value
end
opts.on("-g", "shouldn't be eaten") do
not_eaten << "-g"
end
opts.on("-i", "shouldn't be eaten") do
not_eaten << "-i"
end
end
flag.should eq("")
not_eaten.should eq(["-g","-i"])
args.size.should eq(0)
end
it "doesn't eat a following long option" do
flag = nil
not_eaten = [] of String
args = ["-f", "--g-long", "-i"]
OptionParser.parse(args) do |opts|
opts.on("-f [FLAG]", "some flag") do |flag_value|
flag = flag_value
end
opts.on("-g", "--g-long", "shouldn't be eaten") do
not_eaten << "--g-long"
end
opts.on("-i", "shouldn't be eaten") do
not_eaten << "-i"
end
end
flag.should eq("")
not_eaten.should eq(["--g-long","-i"])
args.size.should eq(0)
end
end
context "Given a long option with an optional value" do
it "doesn't eat further short options" do
flag = nil
not_eaten = [] of String
args = ["--long-flag", "-g", "-i"]
OptionParser.parse(args) do |opts|
opts.on("--long-flag [FLAG]", "some flag") do |flag_value|
flag = flag_value
end
opts.on("-g", "shouldn't be eaten") do
not_eaten << "-g"
end
opts.on("-i", "shouldn't be eaten") do
not_eaten << "-i"
end
end
flag.should eq("")
not_eaten.should eq(["-g","-i"])
args.size.should eq(0)
end
it "doesn't eat further long options" do
flag = nil
not_eaten = [] of String
args = ["--long-flag", "--g-long", "-i"]
OptionParser.parse(args) do |opts|
opts.on("--long-flag [FLAG]", "some flag") do |flag_value|
flag = flag_value
end
opts.on("--g-long", "shouldn't be eaten") do
not_eaten << "--g-long"
end
opts.on("-i", "shouldn't be eaten") do
not_eaten << "-i"
end
end
flag.should eq("")
not_eaten.should eq(["--g-long","-i"])
args.size.should eq(0)
end
end
end

it "raises if missing required option with space" do
expect_missing_option ["-f"], "-f FLAG", "-f"
end
Expand Down
32 changes: 22 additions & 10 deletions src/option_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -368,19 +368,31 @@ class OptionParser
if (handler = @handlers[flag]?) && !(handler.value_type.none? && value)
handled_args << arg_index

# Pull in the next argument if we don't already have it and an argument
# is taken (i.e. not FlagValue::None)
if !value && !handler.value_type.none?
value = args[arg_index + 1]?
if value
handled_args << arg_index + 1
arg_index += 1
if !value
case handler.value_type
when FlagValue::Required
value = args[arg_index + 1]?
if value
handled_args << arg_index + 1
arg_index += 1
else
@missing_option.call(flag)
end
when FlagValue::Optional
value = args[arg_index + 1]?
# e.g. Do not match `-g` or `--g-long` but do match `--`
# NotThis|NotThat|GoAway|(WeWantThis)
if value && value =~ /^\-(?!\-)|^\-\-\S|^(.+)/ && $1?
handled_args << arg_index + 1
arg_index += 1
else
value = nil
end
when FlagValue::None
# do nothing
end
end

# If we require a value and we don't have one, call missing option
@missing_option.call(flag) if handler.value_type.required? && value.nil?

# If this is a subcommand (flag not starting with -), delete all
# subcommands since they are no longer valid.
unless flag.starts_with?('-')
Expand Down

0 comments on commit 169bdd2

Please sign in to comment.