Skip to content
Open
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
23 changes: 21 additions & 2 deletions knack/help.py
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ def __init__(self, help_ctx, delimiters, parser):
'deprecate_info': getattr(action, 'deprecate_info', None),
'preview_info': getattr(action, 'preview_info', None),
'experimental_info': getattr(action, 'experimental_info', None),
'description': action.help,
'description': self._expand_action_help(action),
'choices': action.choices,
'required': False,
'default': None,
Expand All @@ -291,9 +291,28 @@ def __init__(self, help_ctx, delimiters, parser):
help_param = next(p for p in self.parameters if p.name == '--help -h')
help_param.group_name = 'Global Arguments'

@staticmethod
def _expand_action_help(action):
"""Expand argparse-style help placeholders for Knack-rendered help."""
if not isinstance(action.help, str) or '%' not in action.help:
return action.help

parser = getattr(action.container, '_parser', None)
prog = getattr(parser, 'prog', '')
params = dict(vars(action), prog=prog)
for key in list(params):
if params[key] is argparse.SUPPRESS:
del params[key]

try:
return action.help % params
except (KeyError, TypeError, ValueError):
# Keep help resilient even when token expansion cannot be resolved.
return action.help.replace('%%', '%')

def _add_parameter_help(self, param):
param_kwargs = {
'description': param.help,
'description': self._expand_action_help(param),
'choices': param.choices,
'required': param.required,
'default': param.default,
Expand Down
46 changes: 46 additions & 0 deletions knack/parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,50 @@

class CLICommandParser(argparse.ArgumentParser):

@staticmethod
def _sanitize_help_for_argparse(help_text):
"""Escape literal '%' while preserving argparse mapping placeholders.

argparse interpolates help text with ``%`` formatting against a dict.
Keep valid ``%(name)s``-style placeholders as-is and escape everything
else so help text like date formats (for example, ``%Y-%m-%d``) doesn't
crash during parser construction.
"""
if not isinstance(help_text, str) or '%' not in help_text:
return help_text

result = []
idx = 0
text_len = len(help_text)
while idx < text_len:
char = help_text[idx]
if char != '%':
result.append(char)
idx += 1
continue

# Keep already-escaped percent signs as-is.
if idx + 1 < text_len and help_text[idx + 1] == '%':
result.append('%%')
idx += 2
continue

# Preserve mapping placeholders, e.g. %(default)s.
if idx + 1 < text_len and help_text[idx + 1] == '(':
closing_paren = help_text.find(')', idx + 2)
if closing_paren != -1 and closing_paren + 1 < text_len:
conversion_char = help_text[closing_paren + 1]
if conversion_char.isalpha():
result.append(help_text[idx:closing_paren + 2])
idx = closing_paren + 2
continue

# Any other '%' is literal and must be escaped for argparse.
result.append('%%')
idx += 1

return ''.join(result)

@staticmethod
def create_global_parser(cli_ctx=None):
global_parser = argparse.ArgumentParser(prog=cli_ctx.name, add_help=False)
Expand All @@ -43,6 +87,8 @@ def create_global_parser(cli_ctx=None):
def _add_argument(obj, arg):
""" Only pass valid argparse kwargs to argparse.ArgumentParser.add_argument """
argparse_options = {name: value for name, value in arg.options.items() if name in ARGPARSE_SUPPORTED_KWARGS}
if 'help' in argparse_options:
argparse_options['help'] = CLICommandParser._sanitize_help_for_argparse(argparse_options['help'])
if arg.options_list:
scrubbed_options_list = []
for item in arg.options_list:
Expand Down
46 changes: 46 additions & 0 deletions tests/test_help.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,9 @@ def load_command_table(self, args):
g.command('n3', 'example_handler')
g.command('n4', 'example_handler')
g.command('n5', 'example_handler')
g.command('n6', 'example_handler')
g.command('n7', 'example_handler')
g.command('n8', 'example_handler')

with CommandGroup(self, 'group alpha', '{}#{{}}'.format(__name__)) as g:
g.command('n1', 'example_handler')
Expand All @@ -90,6 +93,16 @@ def load_arguments(self, command):
c.argument('arg2', options_list=['--foobar2'], required=True)
c.argument('arg3', options_list=['--foobar3'], help='the foobar3')

with ArgumentsContext(self, 'n6') as c:
c.argument('arg1', options_list=['--fmt'], default='my-default',
help='default=%(default)s prog=%(prog)s')

with ArgumentsContext(self, 'n7') as c:
c.argument('arg1', options_list=['--pct'], help='ratio 100%%')

with ArgumentsContext(self, 'n8') as c:
c.argument('arg1', options_list=['--bad'], help='bad % token')

super().load_arguments(command)

helps['n2'] = """
Expand Down Expand Up @@ -400,6 +413,39 @@ def test_help_group_help(self):
expected = expected.format(self.cli_ctx.name)
self.assertEqual(actual, expected)

@redirect_io
def test_help_argparse_default_and_prog_placeholders(self):
"""Ensure argparse placeholders are expanded in help text."""

with self.assertRaises(SystemExit):
self.cli_ctx.invoke('n6 -h'.split())

actual = self.io.getvalue()
self.assertIn('Default=my-default prog=', actual)
self.assertNotIn('%(default)s', actual)
self.assertNotIn('%(prog)s', actual)

@redirect_io
def test_help_argparse_escaped_percent(self):
"""Ensure escaped percent signs render as a single literal percent."""

with self.assertRaises(SystemExit):
self.cli_ctx.invoke('n7 -h'.split())

actual = self.io.getvalue()
self.assertIn('Ratio 100%.', actual)
self.assertNotIn('Ratio 100%%.', actual)

@redirect_io
def test_help_argparse_bad_percent_falls_back(self):
"""Ensure malformed formatting falls back to the original help text."""

with self.assertRaises(SystemExit):
self.cli_ctx.invoke('n8 -h'.split())

actual = self.io.getvalue()
self.assertIn('Bad % token.', actual)

@redirect_io
@mock.patch('knack.cli.CLI.register_event')
def test_help_global_params(self, _):
Expand Down
16 changes: 16 additions & 0 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,22 @@ def remove_test_file(file):

remove_test_file('test.json')

def test_help_string_with_literal_percent_does_not_crash(self):
def test_handler():
pass

command = CLICommand(self.mock_ctx, 'test command', test_handler)
command.add_argument('date_fmt', '--date-fmt', help='Expected format: %Y-%m-%d')
cmd_table = {'test command': command}
self.mock_ctx.commands_loader.command_table = cmd_table

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

def test_help_string_preserves_argparse_placeholders(self):
sanitized = CLICommandParser._sanitize_help_for_argparse('default is %(default)s (100% expected)')
self.assertEqual(sanitized, 'default is %(default)s (100%% expected)')


class VerifyError(object): # pylint: disable=too-few-public-methods

Expand Down