microsoft / knack

Knack - A Python command line interface framework
https://pypi.python.org/pypi/knack
MIT License
348 stars 95 forks source link

Fix incorrect error message for arguments with choices #237

Closed fralik closed 3 years ago

fralik commented 3 years ago

Fixes #236

ghost commented 3 years ago

CLA assistant check
All CLA requirements met.

jiasli commented 3 years ago

@fralik, Thanks a lot for the contribution and this is a nice finding!

The corresponding fix in Azure CLI is done by https://github.com/Azure/azure-cli/pull/6636. The change is later backported to knack by https://github.com/microsoft/knack/pull/146, but the logic for invalid argument value handling got lost during the backporting.

As the logic of _check_value in Azur CLI has become rather complex due to addition of new features (https://github.com/Azure/azure-cli/pull/16254, https://github.com/Azure/azure-cli/pull/16257), we need more time investigating and writing tests.

jiasli commented 3 years ago

I have created a new PR https://github.com/microsoft/knack/pull/244 based on your commits so that you can still get credit and become a contributor. Please kindly see if the new PR satisfies your needs. Thanks.