Do not treat arguments that start with '--' as string.

[greenmoon55]

Fixes #84

[dbieber]

This is great, I wish I'd noticed your pull request sooner. 2 comments inline.

[dbieber]

We can remove arg_consumed altogether (here and at 698), and replace the if at 739 with an else.

[dbieber]

• Please update the Returns: section of the docstring.
• Also how about "remaining_kwargs" instead of unconsumed_named_args for consistency with the existing terminology?

[greenmoon55]

Thanks! I'll look into it.

[dbieber]

Heads up that some of the tests are failing. You can see the details here (https://travis-ci.org/google/python-fire/jobs/287352133) or by clicking "Details" next to the continuous-integration line after "Some checks were not successfull".

It looks like the majority of the failures are using the components tc.Kwargs or tc.MixedDefaults.

A good first place to investigate might be why this assert is failing [1] [2]:

python-fire/fire/fire_test.py

Lines 344 to 346 in b5053ba

	self.assertEqual(
	fire.Fire(tc.MixedDefaults,
	command=['identity', '--alpha', '--beta', '10']), (True, 10))

Here's the definition of MixedDefaults:

python-fire/fire/test_components.py

Lines 74 to 83 in a0791c9

	class MixedDefaults(object):
	def ten(self):
	return 10
	def sum(self, alpha=0, beta=0):
	return alpha + 2 * beta
	def identity(self, alpha, beta='0'):
	return alpha, beta

---

For what it's worth, these components are also used in many of the failing tests:

python-fire/fire/test_components.py

Lines 56 to 62 in a0791c9

	class WithDefaults(object):
	def double(self, count=0):
	return 2 * count
	def triple(self, count=0):
	return 3 * count

python-fire/fire/test_components.py

Lines 175 to 184 in a0791c9

	class Kwargs(object):
	def props(self, **kwargs):
	return kwargs
	def upper(self, **kwargs):
	return ' '.join(sorted(kwargs.keys())).upper()
	def run(self, positional, named=None, **kwargs):
	return (positional, named, kwargs)

[greenmoon55]

I add a return statement here because pylint complains about "Too many nested blocks (6/5)". Not sure if I should do it this way.

[dbieber]

Seems ok to me.

Another possibility would be moving some of the inner blocks into their own functions, but I don't see a super clean way to do that.

[greenmoon55]

@dbieber Thanks for the tip! I forgot to save the value corresponding to the keyword.

[dbieber]

This looks good!

[dbieber]

nit: for consistency w/ codebase, let's do:

kwargs, remaining_kwargs, remaining_args = _ParseKeywordArgs(
    args, all_args, fn_spec.varkw)

[dbieber]

Seems ok to me.

Another possibility would be moving some of the inner blocks into their own functions, but I don't see a super clean way to do that.

[dbieber]

lgtm.

[dbieber]

Another thing that may be worthwhile to test is the behavior in the presence of separators / chained function calls.

For example, with these components:

def example_component(arg1, kwarg2='default2'):
  def inner_component(kwarg3='default3'):
    return kwarg3
  return inner_component

def example_component(arg1, *varargs):
  def inner_component(kwarg3='default3'):
    return kwarg3
  return inner_component

What is the behavior of these calls?

fire.Fire(example_component, command=['value1', '-', '--kwarg3', 'value3']) == 'value3'
fire.Fire(example_component, command=['value1', '--kwarg3', 'value3'])

[dbieber]

I'm going to go ahead and squash + merge. If you're up for adding those tests and/or making the nit fix, go ahead and do so in a follow up PR.