Fix short flags being swallowed by **kwargs (fixes #454)#652
Open
shivamtiwari3 wants to merge 1 commit intogoogle:masterfrom
Open
Fix short flags being swallowed by **kwargs (fixes #454)#652shivamtiwari3 wants to merge 1 commit intogoogle:masterfrom
shivamtiwari3 wants to merge 1 commit intogoogle:masterfrom
Conversation
…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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #454.
Short flags like
-fand-swere 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:When
fn_keywordsis truthy (i.e. the function has**kwargs), the first branch fires immediately, settingkeyword = key(the raw single character). The shortcut lookup in theelifbranch is never reached.Solution
Restructured the conditions so single-char shortcut expansion against
fn_argsis always attempted first.fn_keywordsis only used as a fallback when nofn_argmatches the single character:Multi-character unrecognised flags still fall through to
**kwargsas before.Testing
fn_with_defaults_and_kwargstotest_components.pytestSingleCharFlagParsingWithKwargstofire_test.pycovering:-f=hello -s=world→('hello', 'world', {})-f hello -s world→('hello', 'world', {})--unknown_key=42→ still forwarded to**kwargspython3 -m pytest fire/ --ignore=fire/docstrings_fuzz_test.py --ignore=fire/parser_fuzz_test.pyChecklist