Skip to content
Closed
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
5 changes: 5 additions & 0 deletions .changes/next-release/feature-help-57237.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"type": "feature",
"category": "help",
"description": "Add a ``--help`` parameter that renders the same help as the ``help`` subcommand on every command (provider, service, operation, and custom commands)."
}
32 changes: 32 additions & 0 deletions awscli/argparser.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
" aws help\n"
" aws <command> help\n"
" aws <command> <subcommand> help\n"
" aws <command> [<subcommand> ...] --help\n"
)
USAGE = (
"aws [options] <command> <subcommand> [<subcommand> ...] [parameters]\n"
Expand Down Expand Up @@ -60,6 +61,37 @@ def choices(self, val):
pass


def _is_help_option_token(arg):
"""Return True only for the literal ``--help`` token.

Abbreviations (``--he``/``--hel``/``--h``) and ``=``-bearing forms
(``--help=x``, ``--instance-ids=--help``) are not help; they fall through to
the normal parser.
"""
return arg == '--help'


def first_help_option_index(args):
"""Return the index of the first ``--help`` token, or ``None`` if absent."""
for index, arg in enumerate(args):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Help detection should stop at the -- end-of-options marker. aws configure set test.key -- --help normally treats --help as the positional value, but this interprets it as a help request and renders command help instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed, it should behave as other parameters in this aspect. I'll publish a revision

if _is_help_option_token(arg):
return index
return None


def is_help_option_present(args):
"""Return True if the literal ``--help`` token is present in ``args``."""
return first_help_option_index(args) is not None


def strip_help_options(args):
"""Return ``args`` with every ``--help`` token removed.

All other tokens keep their original position and value.
"""
return [arg for arg in args if not _is_help_option_token(arg)]


class CLIArgParser(argparse.ArgumentParser):
Formatter = argparse.RawTextHelpFormatter

Expand Down
113 changes: 110 additions & 3 deletions awscli/clidriver.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@
from awscli import __version__
from awscli.alias import AliasCommandInjector, AliasLoader
from awscli.argparser import (
ArgParseException,
ArgTableArgParser,
FirstPassGlobalArgParser,
MainArgParser,
ServiceArgParser,
SubCommandArgParser,
first_help_option_index,
is_help_option_present,
strip_help_options,
)
from awscli.argprocess import unpack_argument
from awscli.arguments import (
Expand Down Expand Up @@ -94,7 +98,7 @@
HISTORY_RECORDER = get_global_history_recorder()
METADATA_FILENAME = 'metadata.json'
INSTALL_FILENAME = 'install.json'
_NO_AUTO_PROMPT_ARGS = ['help', '--version']
_NO_AUTO_PROMPT_ARGS = ['help', '--help', '--version']
_CLI_AUTO_PROMPT_OPTION = '--cli-auto-prompt'
_NO_CLI_AUTO_PROMPT_OPTION = '--no-cli-auto-prompt'
# Don't remove this line. The idna encoding
Expand Down Expand Up @@ -591,6 +595,16 @@ def main(self, args=None):
self._add_aliases(command_table, parser)
parsed_args = None
try:
# When --help is present but no command is named, render provider
# help instead of failing on the required ``command`` positional
# argument. When a command is named, fall through so its own
# __call__ renders the more specific help. Kept inside the try so a
# ``--version`` SystemExit(0) from the pre-help slice parse still
# exits 0.
if is_help_option_present(args) and not self._names_a_command(
args, parser
):
args = self._route_to_provider_help(args)
# Because _handle_top_level_args emits events, it's possible
# that exceptions can be raised, which should have the same
# general exception handling logic as calling into the
Expand All @@ -615,6 +629,44 @@ def main(self, args=None):
parsed_globals=parsed_args,
)

def _names_a_command(self, args, parser):
# ``parser`` is the provider-level ``MainArgParser`` (the one built in
# ``main()`` and passed in). Return True if a real command is named
# before the first --help token. Parsing the pre-help slice with that
# parser lets argparse consume the VALUES of value-taking global options
# instead of mistaking them for a command: in ``aws --region ec2
# --help`` the ``ec2`` is --region's value, so no command is named and
# provider help renders.
head = self._tokens_before_help(args)
if head == list(args):
# No --help token present; nothing to decide here.
return True
try:
parsed, _ = parser.parse_known_args(head)
except ArgParseException:
# Only ArgParseException (raised by CLIArgParser.error) means "no
# parseable command", e.g. ``--region ec2`` -> "required: command".
# We deliberately do not catch broader exceptions so a --version
# SystemExit and a user KeyboardInterrupt propagate.
return False
return getattr(parsed, 'command', None) is not None

def _tokens_before_help(self, args):
# The tokens that precede the first --help token. Tokens at or after
# the first help token are ignored for routing, exactly as the
# positional ``help`` token ignores everything after it.
help_index = first_help_option_index(args)
if help_index is None:
return list(args)
return list(args[:help_index])

def _route_to_provider_help(self, args):
# Render provider help: keep the tokens before the first --help and
# append the ``help`` positional argument, so a trailing command is
# ignored.
head = self._tokens_before_help(args)
return head + ['help']

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we avoid converting the help request into a positional help token here? The parse fails because --query has no value, so this rewrites argv to ... --query help, which strips the help intent and lets the command run. Worried about destructive operations like aws s3api delete-object --bucket b --key k --query --help when the intent is --help

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Worth a regression test on a mutating operation specifically

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agreed. Supplying --help should never behave as supplying help as a value to another parameter.


def _emit_session_event(self, parsed_args):
# This event is guaranteed to run after the session has been
# initialized and a profile has been set. This was previously
Expand Down Expand Up @@ -737,10 +789,33 @@ def __call__(self, args, parsed_globals):
# we can go ahead and create the parser for it. We
# can also grab the Service object from botocore.
service_parser = self.create_parser()
parsed_args, remaining = service_parser.parse_known_args(args)
command_table = self._get_command_table()
# Resolve help intent before binding. If --help is present we still
# want to route to a specific operation's help when an operation was
# named (e.g. ``aws ec2 describe-instances --help``); the operation's
# own __call__ renders it. Only when no operation token is present
# (e.g. ``aws ec2 --help``) do we render this service's help here.
if is_help_option_present(args):
operation = self._find_operation_in_args(args, command_table)
if operation is None:
return self.create_help_command()(
strip_help_options(args), parsed_globals
)
parsed_args, remaining = service_parser.parse_known_args(args)
return command_table[parsed_args.operation](remaining, parsed_globals)

def _find_operation_in_args(self, args, command_table):
# Return the first token before the first --help that names an
# operation, or None. An operation named after --help is ignored:
# ``aws ec2 --help describe-instances`` renders EC2 (service) help, the
# same as ``aws ec2 help describe-instances``.
help_index = first_help_option_index(args)
candidates = args if help_index is None else args[:help_index]
for token in candidates:
if not token.startswith('-') and token in command_table:
return token
return None

def _create_command_table(self):
command_table = OrderedDict()
service_model = self._get_service_model()
Expand Down Expand Up @@ -890,6 +965,21 @@ def _parse_potential_subcommand(self, args, subcommand_table):
return parser.parse_known_args(args)
return None

def _subcommand_precedes_help(self, args, maybe_parsed_subcommand):
# True if the subcommand is named before the first --help token,
# decided from what the subcommand parser binds on the pre-help slice
# rather than a value-blind string index. Mirrors
# ``BasicCommand._subcommand_precedes_help``.
help_index = first_help_option_index(args)
if help_index is None:
return True
subcommand_name = maybe_parsed_subcommand[1]
head = args[:help_index]
parsed_head = self._parse_potential_subcommand(
head, self.subcommand_table
)
return parsed_head is not None and parsed_head[1] == subcommand_name

def __call__(self, args, parsed_globals):
# Once we know we're trying to call a particular operation
# of a service we can go ahead and load the parameters.
Expand All @@ -907,9 +997,26 @@ def __call__(self, args, parsed_globals):
maybe_parsed_subcommand = self._parse_potential_subcommand(
args, subcommand_table
)
if maybe_parsed_subcommand is not None:
# Descend into a parsed subcommand only when it is named before the
# first --help token, mirroring the guard in ``BasicCommand.__call__``.
# Otherwise ``--help`` before a subcommand (e.g. a hypothetical
# ``aws myservice myoperation --help mysubcommand``) would render the
# subcommand's help instead of this operation's. No operation
# currently has a subcommand table, so this is a forward-looking
# robustness check that keeps the two dispatch layers symmetric.
if (
maybe_parsed_subcommand is not None
and self._subcommand_precedes_help(args, maybe_parsed_subcommand)
):
new_args, subcommand_name = maybe_parsed_subcommand
return subcommand_table[subcommand_name](new_args, parsed_globals)
# Resolve --help before binding so a preceding value option (e.g.
# ``--instance-ids i-123 --help``) cannot swallow it. The positional
# ``help`` path below still works; this is an additional path.
if is_help_option_present(args):
return self.create_help_command()(
strip_help_options(args), parsed_globals
)
operation_parser = self._create_operation_parser(
self.arg_table, subcommand_table
)
Expand Down
54 changes: 52 additions & 2 deletions awscli/customizations/commands.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import argparse
import copy
import logging
import os
Expand All @@ -7,7 +8,12 @@
from botocore.validate import validate_parameters

import awscli
from awscli.argparser import ArgTableArgParser, SubCommandArgParser
from awscli.argparser import (
ArgTableArgParser,
SubCommandArgParser,
first_help_option_index,
is_help_option_present,
)
from awscli.argprocess import unpack_argument, unpack_cli_arg
from awscli.arguments import CustomArgument, create_argument_model_from_schema
from awscli.bcdoc import docevents
Expand Down Expand Up @@ -148,11 +154,24 @@ def __call__(self, args, parsed_globals):
maybe_parsed_subcommand = self._parse_potential_subcommand(
args, self._subcommand_table
)
if maybe_parsed_subcommand is not None:
# Descend into a subcommand only when it is named before the first
# --help token: ``aws configure get --help`` renders ``get`` help, but
# ``aws configure --help get`` renders configure help (help resolves at
# the depth reached when --help appears; later tokens are ignored).
if (
maybe_parsed_subcommand is not None
and self._subcommand_precedes_help(args, maybe_parsed_subcommand)
):
new_args, subcommand_name = maybe_parsed_subcommand
return self._subcommand_table[subcommand_name](
new_args, parsed_globals
)
# Resolve --help before binding so a preceding positional value (e.g.
# ``aws configure get region --help``) cannot hide it. The positional
# ``help`` path below still works; this is an additional path.
if is_help_option_present(args):
self._display_help(self._build_help_parsed_args(), parsed_globals)
return 0
parser = ArgTableArgParser(self.arg_table, self.subcommand_table)
parsed_args, remaining = parser.parse_known_args(args)

Expand Down Expand Up @@ -249,6 +268,37 @@ def _display_help(self, parsed_args, parsed_globals):
help_command = self.create_help_command()
help_command(parsed_args, parsed_globals)

def _subcommand_precedes_help(self, args, maybe_parsed_subcommand):
# True if the subcommand is named before the first --help token: when
# --help comes first, render this command's help instead of descending
# into the subcommand.
#
# Decide by re-parsing the pre-help slice via
# ``_parse_potential_subcommand`` rather than by
# ``args.index(subcommand_name)``. A naive string index returns the
# first literal occurrence of the name. If an option value equals the
# subcommand name, that occurrence can be the value rather than the
# actual subcommand token. Parsing the slice consumes option values
# correctly, mirroring ``CLIDriver._names_a_command``.
help_index = first_help_option_index(args)
if help_index is None:
return True
subcommand_name = maybe_parsed_subcommand[1]
head = args[:help_index]
parsed_head = self._parse_potential_subcommand(
head, self._subcommand_table
)
return parsed_head is not None and parsed_head[1] == subcommand_name

def _build_help_parsed_args(self):
# BasicHelp.__call__ does not read the parsed args namespace (it
# generates doc events from the command object), so a minimal
# namespace with help='help' is sufficient and mirrors what the
# positional ``help`` path produced.
namespace = argparse.Namespace()
namespace.help = 'help'
return namespace

def create_help_command(self):
command_help_table = {}
if self.SUBCOMMANDS:
Expand Down
Loading
Loading