Skip to content

Fix #113: Allow positional arguments after optional arguments#151

Open
ahpooch wants to merge 1 commit into
Py-KMS-Organization:mainfrom
ahpooch:fix/issue-113-client-argument-parsing
Open

Fix #113: Allow positional arguments after optional arguments#151
ahpooch wants to merge 1 commit into
Py-KMS-Organization:mainfrom
ahpooch:fix/issue-113-client-argument-parsing

Conversation

@ahpooch

@ahpooch ahpooch commented Jun 26, 2026

Copy link
Copy Markdown

Description

This PR fixes issue #113 where pykms_Client.py fails to parse command line arguments correctly when positional arguments are placed after optional arguments.

Problem

The kms_parser_check_optionals() function in pykms_Misc.py was incorrectly validating command-line arguments. When a positional argument (like an IP address) appeared after an optional argument (like -m Windows10), the parser would treat it as an unknown option and raise an error.

Example of the bug:

$ python3 pykms_Client.py -m Windows10 192.168.10.1
optional py-kms client argument `-m`: expected one argument, unrecognized: '192.168.10.1'. Exiting...

$ python3 pykms_Client.py -y 192.168.10.1
optional py-kms client argument `-y`: expected zero arguments, unrecognized: '192.168.10.1'. Exiting...

This happened because the validation logic checked if the element following an option was a known optional argument, and if not, raised an error - without considering that it might be a legitimate positional argument.

Solution

Modified the validation logic in pykms_Misc.py (line 430) to only raise an error for unrecognized optional arguments (those starting with -), while allowing positional arguments to pass through.

Change made:

# Before:
if elem and elem not in allarg:
    raise KmsParserException(...)

# After:
if elem and elem.startswith('-') and elem not in allarg:
    raise KmsParserException(...)

This simple change ensures that:

  1. Unknown options (starting with -) are still caught and reported as errors
  2. Positional arguments (not starting with -) are allowed after optional arguments
  3. Argument order flexibility is maintained

Testing

After the fix, all the following commands work correctly:

With positional arguments after optional arguments:

python3 pykms_Client.py -m Windows10 192.168.10.1
python3 pykms_Client.py -y 192.168.10.1
python3 pykms_Client.py -m Windows10 192.168.10.1 1688
python3 pykms_Client.py -F STDOUT -m Windows10 192.168.10.1

Traditional order (positional before optional) still works:

python3 pykms_Client.py 192.168.10.1 -m Windows10
python3 pykms_Client.py 192.168.10.1 1688 -m Windows10

Error handling for invalid options still works:

$ python3 pykms_Client.py -m InvalidMode 192.168.10.1
argument -m/--mode: invalid choice: 'InvalidMode' (choose from ...). Exiting...

Impact

  • Changed files: 1 (py-kms/pykms_Misc.py)
  • Lines changed: 1 line modified
  • Breaking changes: None - this is a pure bug fix that adds functionality
  • Backward compatibility: Fully maintained - all existing valid command-line patterns continue to work

Related Issues

Fixes #113

Checklist

  • The code has been tested locally
  • The fix is minimal and focused
  • No breaking changes introduced
  • Existing functionality is preserved
  • The commit message follows the project conventions

…l arguments

The kms_parser_check_optionals() function incorrectly rejected
positional arguments that appeared after optional arguments,
treating them as unknown options.
Changed the validation logic in pykms_Misc.py (line 430) to only
raise an error for unrecognized optional arguments (those starting
with '-'), while allowing positional arguments to pass through.

This fixes commands like:
python3 pykms_Client.py -m Windows10 192.168.10.1
python3 pykms_Client.py -y 192.168.10.1
python3 pykms_Client.py -m Windows10 192.168.10.1 1688

Previously these would fail with 'expected X argument, unrecognized'
errors. Now they work correctly regardless of argument order.
Fixes Py-KMS-Organization#113
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.

pykms_Client fails to parse command line options correctly

1 participant