Repository: google/python-fire
File: fire/core.py (Python)

Code before:
  remaining_kwargs = []
  remaining_args = []
  fn_keywords = fn_spec.varkw
  fn_args = fn_spec.args + fn_spec.kwonlyargs

  if not args:
    return kwargs, remaining_kwargs, remaining_args

  skip_argument = False

  for index, argument in enumerate(args):
    if skip_argument:
      skip_argument = False
      continue

    arg_consumed = False
    if argument.startswith('--'):
      # This is a named argument; get its value from this arg or the next.
      got_argument = False

      keyword = argument[2:]
      contains_equals = '=' in keyword
      is_bool_syntax = (not contains_equals and
                        (index + 1 == len(args) or
                         args[index + 1].startswith('--') or
                         re.match('^-[a-z]$', args[index + 1])))
      if contains_equals:
        keyword, value = keyword.split('=', 1)
        got_argument = True
      elif is_bool_syntax:
        # Since there's no next arg or the next arg is a Flag, we consider
        # this flag to be a boolean.
        got_argument = True
        if keyword in fn_args:
          value = 'True'
        elif keyword.startswith('no'):
          keyword = keyword[2:]
          value = 'False'
        else:
          value = 'True'
      else:
        if index + 1 < len(args):
          value = args[index + 1]
          got_argument = True

      keyword = keyword.replace('-', '_')

      # In order for us to consume the argument as a keyword arg, we either:
      # Need to be explicitly expecting the keyword, or we need to be
      # accepting **kwargs.
      if got_argument:

Review comment (refactor):
Let's refactor `arg.startswith('--') or re.match('^-[a-z]$', arg)` into a function _IsFlag so that we don't have to worry about keeping the regexes in sync. And then let's use that function both here and at 792 as part of handling any type (not just bools) with single-character args.


I think the way we should organize this is (pseudocode):

def _IsFlag:  return _IsSingleCharFlag or _IsMultiCharFlag
def _IsSingleCharFlag: ...
def _IsMultiCharFlag: ...

line 792:
```
if _IsFlag(argument):
  if _IsSingleCharFlag(argument):
    # your logic from 838 for determining the keyword
  else:
    # the logic from 796 for determining the keyword
  # continue from from 797 as is...
```


---

Then (either in this PR or in a subsequent change) I'm a proponent of modifying _IsMultiCharFlag to work even if argument only starts with -[a-zA-Z] instead of requiring two hyphens.

Code after:
  remaining_kwargs = []
  remaining_args = []
  fn_keywords = fn_spec.varkw
  fn_args = fn_spec.args + fn_spec.kwonlyargs

  if not args:
    return kwargs, remaining_kwargs, remaining_args

  skip_argument = False

  for index, argument in enumerate(args):
    if skip_argument:
      skip_argument = False
      continue

    arg_consumed = False
    if _IsFlag(argument):
      # This is a named argument; get its value from this arg or the next.
      got_argument = False

      keyword = ''
      if _IsSingleCharFlag(argument):
        keychar = argument[1]
        potential_args = [arg for arg in fn_args if arg[0] == keychar]
        if len(potential_args) == 1:
          keyword = potential_args[0]
        elif len(potential_args) > 1:
          raise FireError("The argument '{}' is ambiguous as it could "
                          "refer to any of the following arguments: {}".format(
                              argument, potential_args))

      else:
        keyword = argument[2:]

      contains_equals = '=' in keyword
      is_bool_syntax = (not contains_equals and
                        (index + 1 == len(args) or _IsFlag(args[index + 1])))
      if contains_equals:
        keyword, value = keyword.split('=', 1)
        got_argument = True
      elif is_bool_syntax:
        # Since there's no next arg or the next arg is a Flag, we consider
        # this flag to be a boolean.
        got_argument = True
        if keyword in fn_args:
          value = 'True'
        elif keyword.startswith('no'):
          keyword = keyword[2:]
          value = 'False'
        else:
          value = 'True'