Refactor OptionParser#parse (#16233)

Extracts logic into a couple of helper methods. This should not alter behavior in any way.

Preparatory step toward enabling short option combinations (#10981).

Co-authored-by: Johannes Müller <[email protected]>

Author: kojix2

src/option_parser.cr

@@ -401,67 +401,9 @@ class OptionParser
           break
         end
 
-        if arg.starts_with?("--")
-          value_index = arg.index('=')
-          if value_index
-            flag = arg[0...value_index]
-            value = arg[value_index + 1..-1]
-          else
-            flag = arg
-            value = nil
-          end
-        elsif arg.starts_with?('-')
-          if arg.size > 2
-            flag = arg[0..1]
-            value = arg[2..-1]
-          else
-            flag = arg
-            value = nil
-          end
-        else
-          flag = arg
-          value = nil
-        end
-
-        # Fetch handler of the flag.
-        # If value is given even though handler does not take value, it is invalid, then it is skipped.
-        if (handler = @handlers[flag]?) && !(handler.value_type.none? && value)
-          handled_args << arg_index
-
-          if !value
-            case handler.value_type
-            in FlagValue::Required
-              value = args[arg_index + 1]?
-              if value
-                handled_args << arg_index + 1
-                arg_index += 1
-              else
-                @missing_option.call(flag)
-              end
-            in FlagValue::Optional
-              unless gnu_optional_args?
-                value = args[arg_index + 1]?
-                if value && !@handlers.has_key?(value)
-                  handled_args << arg_index + 1
-                  arg_index += 1
-                else
-                  value = nil
-                end
-              end
-            in FlagValue::None
-              # do nothing
-            end
-          end
+        flag, value = parse_arg_to_flag_and_value(arg)
 
-          # If this is a subcommand (flag not starting with -), delete all
-          # subcommands since they are no longer valid.
-          unless flag.starts_with?('-')
-            @handlers.select! { |k, _| k.starts_with?('-') }
-            @flags.select!(&.starts_with?("    -"))
-          end
-
-          handler.block.call(value || "")
-        end
+        arg_index = handle_flag(flag, value, arg_index, args, handled_args)
 
         arg_index += 1
       end
@@ -476,20 +418,7 @@ class OptionParser
       end
 
       # After argument parsing, delete handled arguments from args.
-      # We reverse so that we delete args from
-      handled_args.reverse!
-      i = 0
-      args.reject! do
-        # handled_args is sorted in reverse so we know that i <= handled_args.last
-        handled = i == handled_args.last?
-
-        # Maintain the i <= handled_args.last invariant
-        handled_args.pop if handled
-
-        i += 1
-
-        handled
-      end
+      remove_handled_args(args, handled_args)
 
       # Since we've deleted all handled arguments, `args` is all unknown arguments
       # which we split by the index of any double dash argument
@@ -514,4 +443,92 @@ class OptionParser
       end
     end
   end
+
+  # Parses a command-line argument into a flag and optional inline value.
+  private def parse_arg_to_flag_and_value(arg : String) : {String, String?}
+    if arg.starts_with?("--")
+      value_index = arg.index('=')
+      if value_index
+        flag = arg[0...value_index]
+        value = arg[value_index + 1..-1]
+      else
+        flag = arg
+        value = nil
+      end
+    elsif arg.starts_with?('-')
+      if arg.size > 2
+        flag = arg[0..1]
+        value = arg[2..-1]
+      else
+        flag = arg
+        value = nil
+      end
+    else
+      flag = arg
+      value = nil
+    end
+    {flag, value}
+  end
+
+  # Processes a single flag/subcommand. Matches original behaviour exactly.
+  private def handle_flag(flag : String, value : String?, arg_index : Int32, args : Array(String), handled_args : Array(Int32)) : Int32
+    if (handler = @handlers[flag]?) && !(handler.value_type.none? && value)
+      handled_args << arg_index
+
+      if !value
+        case handler.value_type
+        in FlagValue::Required
+          value = args[arg_index + 1]?
+          if value
+            handled_args << arg_index + 1
+            arg_index += 1
+          else
+            @missing_option.call(flag)
+          end
+        in FlagValue::Optional
+          unless gnu_optional_args?
+            value = args[arg_index + 1]?
+            if value && !@handlers.has_key?(value)
+              handled_args << arg_index + 1
+              arg_index += 1
+            else
+              value = nil
+            end
+          end
+        in FlagValue::None
+          # do nothing
+        end
+      end
+
+      # If this is a subcommand (flag not starting with -), delete all
+      # subcommands since they are no longer valid.
+      unless flag.starts_with?('-')
+        @handlers.select! { |k, _| k.starts_with?('-') }
+        @flags.select!(&.starts_with?("    -"))
+      end
+
+      handler.block.call(value || "")
+    end
+
+    arg_index
+  end
+
+  # Removes handled arguments from the args array based on handled_args indexes.
+  private def remove_handled_args(args : Array(String), handled_args : Array(Int32)) : Nil
+    # After argument parsing, delete handled arguments from args.
+    # We reverse so that we delete args from the end
+    handled_args.reverse!
+    i = 0
+    args.reject! do
+      # handled_args is sorted in reverse so we know that i <= handled_args.last
+      handled = i == handled_args.last?
+
+      # Maintain the i <= handled_args.last invariant
+      handled_args.pop if handled
+
+      i += 1
+
+      handled
+    end
+  end
 end