Skip to content

Fix short flags being swallowed by **kwargs (fixes #454)#652

Open
shivamtiwari3 wants to merge 1 commit intogoogle:masterfrom
shivamtiwari3:fix/short-flags-kwargs
Open

Fix short flags being swallowed by **kwargs (fixes #454)#652
shivamtiwari3 wants to merge 1 commit intogoogle:masterfrom
shivamtiwari3:fix/short-flags-kwargs

Conversation

@shivamtiwari3
Copy link

Summary

Fixes #454.

Short flags like -f and -s were being stored directly in **kwargs (as {'f': 'hello', 's': 'world'}) instead of being expanded to their matching positional argument names (first_arg, second_arg) when a function accepts **kwargs.


Problem

In _ParseKeywordArgs (core.py:887), the condition was:

if (key in fn_args
    or (is_bool_syntax and key.startswith('no') and key[2:] in fn_args)
    or fn_keywords):
  keyword = key
elif len(key) == 1:
  # single-char shortcut lookup ...

When fn_keywords is truthy (i.e. the function has **kwargs), the first branch fires immediately, setting keyword = key (the raw single character). The shortcut lookup in the elif branch is never reached.


Solution

Restructured the conditions so single-char shortcut expansion against fn_args is always attempted first. fn_keywords is only used as a fallback when no fn_arg matches the single character:

if key in fn_args or (...noXxx... in fn_args):
  keyword = key
elif len(key) == 1:
  matching_fn_args = [arg for arg in fn_args if arg[0] == key]
  if len(matching_fn_args) == 1:
    keyword = matching_fn_args[0]
  elif len(matching_fn_args) > 1:
    raise FireError(...)
  elif fn_keywords:      # ← fallback: no fn_arg match, accept into **kwargs
    keyword = key
elif fn_keywords:
  keyword = key

Multi-character unrecognised flags still fall through to **kwargs as before.


Testing

  • Added fn_with_defaults_and_kwargs to test_components.py
  • Added testSingleCharFlagParsingWithKwargs to fire_test.py covering:
    • -f=hello -s=world('hello', 'world', {})
    • -f hello -s world('hello', 'world', {})
    • unrecognised --unknown_key=42 → still forwarded to **kwargs
  • Command: python3 -m pytest fire/ --ignore=fire/docstrings_fuzz_test.py --ignore=fire/parser_fuzz_test.py
  • 261 tests pass, 0 failures

Checklist

  • Fixes the reported issue
  • Existing behaviour unchanged (no regressions)
  • Tests added
  • All 261 tests pass
  • Code style matches project conventions

…n_args

When a function accepts **kwargs, single-character flags like -f were
being stored directly as 'f' in kwargs rather than being expanded to
the matching positional argument name (e.g. 'first_arg').

The root cause was that the `or fn_keywords` branch in
_ParseKeywordArgs triggered before the single-char shortcut lookup,
so the key was accepted as-is without checking fn_args first.

Fix by restructuring the conditions so that single-char shortcut
expansion against fn_args is always attempted first, with fn_keywords
used only as a fallback when no fn_arg matches.

Fixes google#454
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug] short flags/arguments does not work with **kwargs

1 participant