Merge pull request #461 from Shopify/fix-mixing-short-and-long-options

Fix mixing short and long multi-options
Shopify · Feb 28, 2024 · 74c3737 · 74c3737
2 parents 8df7da0 + 0a196db
commit 74c3737
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 19 deletions.
diff --git a/lib/cli/kit/args/evaluation.rb b/lib/cli/kit/args/evaluation.rb
@@ -199,26 +199,15 @@ def lookup_flag(flag)
 
         sig { params(opt: Definition::Option).returns(T.any(NilClass, String, T::Array[String])) }
         def lookup_option(opt)
-          if opt.short
-            opts = T.cast(
-              parse.select { |node| node.is_a?(Parser::Node::ShortOption) },
-              T::Array[Parser::Node::ShortOption],
-            )
-            matches = opts.select { |node| node.name == opt.short }
-            if (last = matches.last)
-              return(opt.multi? ? matches.map(&:value) : last.value)
-            end
-          end
-          if opt.long
-            opts = T.cast(
-              parse.select { |node| node.is_a?(Parser::Node::LongOption) },
-              T::Array[Parser::Node::LongOption],
-            )
-            matches = opts.select { |node| node.name == opt.long }
-            if (last = matches.last)
-              return(opt.multi? ? matches.map(&:value) : last.value)
-            end
+          opts = T.cast(
+            parse.select { |node| node.is_a?(Parser::Node::ShortOption) || node.is_a?(Parser::Node::LongOption) },
+            T::Array[T.any(Parser::Node::ShortOption, Parser::Node::LongOption)],
+          )
+          matches = opts.select { |node| (opt.short && node.name == opt.short) || (opt.long && node.name == opt.long) }
+          if (last = matches.last)
+            return(opt.multi? ? matches.map(&:value) : last.value)
           end
+
           opt.default
         end
 

diff --git a/test/cli/kit/args/evaluation_test.rb b/test/cli/kit/args/evaluation_test.rb
@@ -21,6 +21,7 @@ def setup
           @defn.add_position(:second, required: false, multi: false, skip: ->(arg) { arg == 'b' })
           @defn.add_position(:third, required: false, multi: false, skip: ->(arg) { arg != 'b' })
           @defn.add_position(:rest, required: false, multi: true)
+          @defn.add_option(:yo, short: '-y', long: '--yo', multi: true)
 
           @parse = [
             Node::ShortFlag.new('f'),
@@ -32,6 +33,8 @@ def setup
             Node::Argument.new('c'),
             Node::Argument.new('d'),
             Node::LongFlag.new('print'),
+            Node::ShortOption.new('y', 'a'),
+            Node::LongOption.new('yo', 'b'),
             Node::Unparsed.new(['d', '--neato', '-f']),
           ]
         end
@@ -56,6 +59,7 @@ def test_evaluation
           assert_equal('a', evl.position.first)
           assert_nil(evl.position.second)
           assert_equal('b', evl.position.third)
+          assert_equal(['a', 'b'], evl.opt.yo)
           assert_equal(['c', 'd'], evl.position.rest)
           assert_equal(['d', '--neato', '-f'], evl.unparsed)
         end