Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add error message for invalid argument value #244

Merged
merged 5 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions examples/exapp2
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,16 @@ type: group
short-summary: An experimental command group
"""

helps['demo'] = """
type: group
short-summary: A command group for demos.
"""

helps['demo arg'] = """
type: group
short-summary: A command showing how to use arguments.
"""


def abc_show_command_handler():
"""
Expand Down Expand Up @@ -151,6 +161,13 @@ def hello_command_handler(greetings=None):
return ['Hello World!', greetings]


def demo_arg_handler(move=None):
if move:
print("Your move was: {}".format(move))
return
print("Nothing to do.")


WELCOME_MESSAGE = r"""
_____ _ _____
/ ____| | |_ _|
Expand Down Expand Up @@ -179,6 +196,7 @@ class MyCommandsLoader(CLICommandsLoader):
g.command('hello', 'hello_command_handler', confirmation=True)
g.command('sample-json', 'sample_json_handler')
g.command('sample-logger', 'sample_logger_handler')

with CommandGroup(self, 'abc', '__main__#{}') as g:
g.command('list', 'abc_list_command_handler')
g.command('show', 'abc_show_command_handler')
Expand All @@ -202,12 +220,20 @@ class MyCommandsLoader(CLICommandsLoader):
# A deprecated command group
with CommandGroup(self, 'dep', '__main__#{}', deprecate_info=g.deprecate(redirect='ga', hide='1.0.0')) as g:
g.command('range', 'range_command_handler')

with CommandGroup(self, 'demo', '__main__#{}') as g:
g.command('arg', 'demo_arg_handler')

return super(MyCommandsLoader, self).load_command_table(args)

def load_arguments(self, command):
with ArgumentsContext(self, 'ga range') as ac:
ac.argument('start', type=int, is_preview=True)
ac.argument('end', type=int, is_experimental=True)

with ArgumentsContext(self, 'demo arg') as ac:
ac.argument('move', choices=['rock', 'paper', 'scissors'])

super(MyCommandsLoader, self).load_arguments(command)


Expand Down
33 changes: 19 additions & 14 deletions knack/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,19 +266,24 @@ def _check_value(self, action, value):
import sys

if action.choices is not None and value not in action.choices:
# parser has no `command_source`, value is part of command itself
error_msg = "{prog}: '{value}' is not in the '{prog}' command group. See '{prog} --help'.".format(
prog=self.prog, value=value)
logger.error(error_msg)
candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7)
if candidates:
print_args = {
's': 's' if len(candidates) > 1 else '',
'verb': 'are' if len(candidates) > 1 else 'is',
'value': value
}
suggestion_msg = "\nThe most similar choice{s} to '{value}' {verb}:\n".format(**print_args)
suggestion_msg += '\n'.join(['\t' + candidate for candidate in candidates])
if action.dest in ["_command", "_subcommand"]:
# Command
error_msg = "{prog}: '{value}' is not in the '{prog}' command group. See '{prog} --help'.".format(
prog=self.prog, value=value)
logger.error(error_msg)
# Show suggestions
candidates = difflib.get_close_matches(value, action.choices, cutoff=0.7)
if candidates:
suggestion_msg = "\nThe most similar choices to '{value}':\n".format(value=value)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to play around with English grammar for third person singular form.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

suggestion_msg += '\n'.join(['\t' + candidate for candidate in candidates])
print(suggestion_msg, file=sys.stderr)
else:
# Argument
error_msg = "{prog}: '{value}' is not a valid value for '{name}'.".format(
prog=self.prog, value=value,
name=argparse._get_action_name(action)) # pylint: disable=protected-access
logger.error(error_msg)
# Show all allowed values
suggestion_msg = "Allowed values: " + ', '.join(action.choices)
Copy link
Member Author

@jiasli jiasli Mar 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of allowed argument values is usually very limited. Show all of them.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

print(suggestion_msg, file=sys.stderr)

self.exit(2)
23 changes: 21 additions & 2 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from knack.parser import CLICommandParser
from knack.commands import CLICommand
from knack.arguments import enum_choice_list
from tests.util import MockContext
from tests.util import MockContext, redirect_io


class TestParser(unittest.TestCase):
Expand Down Expand Up @@ -83,7 +83,7 @@ def test_handler():
parser.parse_args('test command -req yep'.split())
self.assertTrue(CLICommandParser.error.called)

def test_case_insensitive_enum_choices(self):
def _enum_parser(self):
from enum import Enum

class TestEnum(Enum): # pylint: disable=too-few-public-methods
Expand All @@ -102,7 +102,10 @@ def test_handler():

parser = CLICommandParser()
parser.load_command_table(self.mock_ctx.commands_loader)
return parser

def test_case_insensitive_enum_choices(self):
parser = self._enum_parser()
args = parser.parse_args('test command --opt alL_cAps'.split())
self.assertEqual(args.opt, 'ALL_CAPS')

Expand All @@ -112,6 +115,22 @@ def test_handler():
args = parser.parse_args('test command --opt sNake_CASE'.split())
self.assertEqual(args.opt, 'snake_case')

@redirect_io
def test_check_value_invalid_command(self):
parser = self._enum_parser()
with self.assertRaises(SystemExit) as cm:
parser.parse_args('test command1'.split()) # 'command1' is invalid
actual = self.io.getvalue()
assert "is not in the" in actual and "command group" in actual

@redirect_io
def test_check_value_invalid_argument_value(self):
parser = self._enum_parser()
with self.assertRaises(SystemExit) as cm:
parser.parse_args('test command --opt foo'.split()) # 'foo' is invalid
actual = self.io.getvalue()
assert "is not a valid value for" in actual

def test_cli_ctx_type_error(self):
with self.assertRaises(TypeError):
CLICommandParser(cli_ctx=object())
Expand Down