Skip to content

Commit

Permalink
Fix OptionParser to handle sub-commands with hyphen (#9465)
Browse files Browse the repository at this point in the history
It was matching regexs anywhere on the string, but we want the regexs to
match the full string. In particular this behavior made that a flag
`sub-command` was interpeted as `su`, because it matches the `-(.)\S+`
regex.

Co-authored-by: Alexandre Morignot <[email protected]>
  • Loading branch information
erdnaxeli and erdnaxeli authored Jun 23, 2020
1 parent 19c0adb commit 476486e
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
10 changes: 10 additions & 0 deletions spec/std/option_parser_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -572,6 +572,16 @@ describe "OptionParser" do
USAGE
end

it "handles subcommands with hyphen" do
subcommand = false
OptionParser.parse(%w(sub-command)) do |opts|
opts.banner = "Usage: foo"
opts.on("sub-command", "Subcommand description") { subcommand = true }
end

subcommand.should be_true
end

it "stops when asked" do
args = %w(--foo --stop --bar)
foo = false
Expand Down
10 changes: 5 additions & 5 deletions src/option_parser.cr
Original file line number Diff line number Diff line change
Expand Up @@ -203,16 +203,16 @@ class OptionParser

private def parse_flag_definition(flag : String)
case flag
when /--(\S+)\s+\[\S+\]/
when /\A--(\S+)\s+\[\S+\]\z/
{"--#{$1}", FlagValue::Optional}
when /--(\S+)(\s+|\=)(\S+)?/
when /\A--(\S+)(\s+|\=)(\S+)?\z/
{"--#{$1}", FlagValue::Required}
when /--\S+/
when /\A--\S+\z/
# This can't be merged with `else` otherwise /-(.)/ matches
{flag, FlagValue::None}
when /-(.)\s*\[\S+\]/
when /\A-(.)\s*\[\S+\]\z/
{flag[0..1], FlagValue::Optional}
when /-(.)\s+\S+/, /-(.)\s+/, /-(.)\S+/
when /\A-(.)\s+\S+\z/, /\A-(.)\s+\z/, /\A-(.)\S+\z/
{flag[0..1], FlagValue::Required}
else
# This happens for -f without argument
Expand Down

0 comments on commit 476486e

Please sign in to comment.