From c40e132143a593519c4015b18a17d78ba8af7efb Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 8 Mar 2018 13:35:17 -0800 Subject: [PATCH 1/8] Alias 0.2.0 --- src/alias/azext_alias/_const.py | 14 +- src/alias/azext_alias/alias.py | 108 +++++------- src/alias/azext_alias/argument.py | 163 +++++++++++++++++++ src/alias/azext_alias/tests/_const.py | 32 ++-- src/alias/azext_alias/tests/test_alias.py | 43 ++--- src/alias/azext_alias/tests/test_argument.py | 110 +++++++++++++ src/alias/azext_alias/version.py | 2 +- src/alias/setup.py | 4 +- src/index.json | 16 +- 9 files changed, 368 insertions(+), 124 deletions(-) create mode 100644 src/alias/azext_alias/argument.py create mode 100644 src/alias/azext_alias/tests/test_argument.py diff --git a/src/alias/azext_alias/_const.py b/src/alias/azext_alias/_const.py index c78f1eaf04a..2e19ac67f4d 100644 --- a/src/alias/azext_alias/_const.py +++ b/src/alias/azext_alias/_const.py @@ -11,11 +11,15 @@ COLLIDED_ALIAS_FILE_NAME = 'collided_alias' COLLISION_CHECK_LEVEL_DEPTH = 4 -PLACEHOLDER_REGEX = r'\s+{\d+}' +POS_ARG_NAME_REGEX = r'{{\s*([a-zA-Z0-9_]+)\s*}}' +EXPRESSION_REGEX = r'{{\s*([a-zA-Z0-9_]+.*?)\s*}}' +NUMBER_PLACEHOLDER_REGEX = r'{{\s*\d+\s*}}' -INCONSISTENT_INDEXING_ERROR = 'alias: Placeholder indexing should be zero-indexed, but {} is missing in "{}"' -INSUFFICIENT_POS_ARG_ERROR = 'alias: "{}" takes exactly {} argument(s) ({} given)' -CONFIG_PARSING_ERROR = 'alias: Error parsing the configuration file - %s. Please fix the problem manually.' +INSUFFICIENT_POS_ARG_ERROR = 'alias: "{}" takes exactly {} positional argument{} ({} given)' +CONFIG_PARSING_ERROR = 'alias: Error parsing the configuration file - {}. Please fix the problem manually.' DEBUG_MSG = 'Alias Manager: Transforming "%s" to "%s"' DEBUG_MSG_WITH_TIMING = 'Alias Manager: Transformed args to %s in %.3fms' -POS_ARG_DEBUG_MSG = 'Alias Manager: Transforming "{}" to "{}", with the following positional arguments: ' +POS_ARG_DEBUG_MSG = 'Alias Manager: Transforming "%s" to "%s", with the following positional arguments: %s' +DUPLICATED_PLACEHOLDER_ERROR = 'alias: Duplicated placeholders found when transforming "{}"' +RENDER_TEMPLATE_ERROR = 'alias: Encounted the following error when injecting positional arguments to "{}" - {}' +PLACEHOLDER_EVAL_ERROR = 'alias: Encounted the following error when evaluating "{}" - {}' diff --git a/src/alias/azext_alias/alias.py b/src/alias/azext_alias/alias.py index 1ae87ced3e3..4aed8f32c66 100644 --- a/src/alias/azext_alias/alias.py +++ b/src/alias/azext_alias/alias.py @@ -5,12 +5,14 @@ import os import re -import hashlib +import sys import json +import shlex +import hashlib +from collections import defaultdict from six.moves import configparser from knack.log import get_logger -from knack.util import CLIError from azext_alias import telemetry from azext_alias._const import ( @@ -18,14 +20,16 @@ ALIAS_FILE_NAME, ALIAS_HASH_FILE_NAME, COLLIDED_ALIAS_FILE_NAME, - PLACEHOLDER_REGEX, - INCONSISTENT_INDEXING_ERROR, CONFIG_PARSING_ERROR, - INSUFFICIENT_POS_ARG_ERROR, DEBUG_MSG, - POS_ARG_DEBUG_MSG, - COLLISION_CHECK_LEVEL_DEPTH + COLLISION_CHECK_LEVEL_DEPTH, + POS_ARG_DEBUG_MSG ) +from azext_alias.argument import ( + build_pos_args_table, + render_template +) + GLOBAL_ALIAS_PATH = os.path.join(GLOBAL_CONFIG_DIR, ALIAS_FILE_NAME) GLOBAL_ALIAS_HASH_PATH = os.path.join(GLOBAL_CONFIG_DIR, ALIAS_HASH_FILE_NAME) @@ -34,12 +38,24 @@ logger = get_logger(__name__) +def get_config_parser(): + """ + Disable configparser's interpolation function and return an instance of config parser. + + Returns: + An instance of config parser with interpolation disabled. + """ + if sys.version_info.major == 3: + return configparser.ConfigParser(interpolation=None) # pylint: disable=unexpected-keyword-arg + return configparser.ConfigParser() + + class AliasManager(object): def __init__(self, **kwargs): - self.alias_table = configparser.ConfigParser() + self.alias_table = get_config_parser() self.kwargs = kwargs - self.collided_alias = dict() + self.collided_alias = defaultdict(list) self.reserved_commands = [] self.alias_config_str = '' self.alias_config_hash = '' @@ -81,7 +97,7 @@ def load_collided_alias(self): collided_alias_str = collided_alias_file.read() try: self.collided_alias = json.loads(collided_alias_str if collided_alias_str else '{}') - except Exception: # pylint: disable=broad-except + except Exception: # pylint: disable=broad-except self.collided_alias = {} def detect_alias_config_change(self): @@ -136,34 +152,25 @@ def transform(self, args): continue full_alias = self.get_full_alias(alias) - num_pos_args = AliasManager.count_positional_args(full_alias) if self.alias_table.has_option(full_alias, 'command'): cmd_derived_from_alias = self.alias_table.get(full_alias, 'command') - if not num_pos_args: - logger.debug(DEBUG_MSG, alias, cmd_derived_from_alias) telemetry.set_alias_hit(full_alias) else: transformed_commands.append(alias) continue - if num_pos_args: - # Take arguments indexed from alias_index to alias_index + num_pos_args and inject - # them as positional arguments into the command - pos_args_iter = AliasManager.pos_args_iter(alias, args, alias_index, num_pos_args) - pos_arg_debug_msg = POS_ARG_DEBUG_MSG.format(alias, cmd_derived_from_alias) - for placeholder, pos_arg in pos_args_iter: - if placeholder not in full_alias: - raise CLIError(INCONSISTENT_INDEXING_ERROR.format(placeholder, full_alias)) - - cmd_derived_from_alias = cmd_derived_from_alias.replace(placeholder, pos_arg) - pos_arg_debug_msg += "({}: {}) ".format(placeholder, pos_arg) - # Skip the next arg because it has been already consumed as a positional argument above - next(alias_iter) - logger.debug(pos_arg_debug_msg) + pos_args_table = build_pos_args_table(full_alias, args, alias_index) + if pos_args_table: + logger.debug(POS_ARG_DEBUG_MSG, full_alias, cmd_derived_from_alias, pos_args_table) + transformed_commands += render_template(cmd_derived_from_alias, pos_args_table) - # Invoke split() because the command derived from the alias might contain spaces - transformed_commands += cmd_derived_from_alias.split() + # Skip the next arg(s) because they have been already consumed as a positional argument above + for pos_arg in pos_args_table: # pylint: disable=unused-variable + next(alias_iter) + else: + logger.debug(DEBUG_MSG, full_alias, cmd_derived_from_alias) + transformed_commands += shlex.split(cmd_derived_from_alias) return self.post_transform(transformed_commands) @@ -171,7 +178,7 @@ def build_collision_table(self, levels=COLLISION_CHECK_LEVEL_DEPTH): """ Build the collision table according to the alias configuration file against the entire command table. - if the word collided with a reserved command. self.collided_alias is structured as: + self.collided_alias is structured as: { 'collided_alias': [the command level at which collision happens] } @@ -193,8 +200,6 @@ def build_collision_table(self, levels=COLLISION_CHECK_LEVEL_DEPTH): for level in range(1, levels + 1): collision_regex = r'^{}{}($|\s)'.format(r'([a-z\-]*\s)' * (level - 1), word.lower()) if list(filter(re.compile(collision_regex).match, self.reserved_commands)): - if word not in self.collided_alias: - self.collided_alias[word] = [] self.collided_alias[word].append(level) telemetry.set_collided_aliases(list(self.collided_alias.keys())) @@ -210,6 +215,7 @@ def get_full_alias(self, query): """ if query in self.alias_table.sections(): return query + return next((section for section in self.alias_table.sections() if section.split()[0] == query), '') def load_full_command_table(self): @@ -217,7 +223,7 @@ def load_full_command_table(self): Perform a full load of the command table to get all the reserved command words. """ load_cmd_tbl_func = self.kwargs.get('load_cmd_tbl_func', None) - if load_cmd_tbl_func: + if callable(load_cmd_tbl_func): self.reserved_commands = list(load_cmd_tbl_func([]).keys()) telemetry.set_full_command_table_loaded() @@ -233,9 +239,6 @@ def post_transform(self, args): post_transform_commands = [] for arg in args: - # Trim leading and trailing quotes - if arg and arg[0] == arg[-1] and arg[0] in '\'"': - arg = arg[1:-1] post_transform_commands.append(os.path.expandvars(arg)) self.write_alias_config_hash() @@ -291,36 +294,3 @@ def process_exception_message(exception): for replace_char in ['\t', '\n', '\\n']: exception_message = exception_message.replace(replace_char, '' if replace_char != '\t' else ' ') return exception_message.replace('section', 'alias') - - @staticmethod - def pos_args_iter(alias, args, start_index, num_pos_args): - """ - Generate an tuple iterator ([0], [1]) where the [0] is the positional argument - placeholder and [1] is the argument value. e.g. ('{0}', pos_arg_1) -> ('{1}', pos_arg_2) -> ... - - Args: - alias: The current alias we are processing. - args: The list of input commands. - start_index: The index where we start selecting the positional arguments - (one-index instead of zero-index). - num_pos_args: The number of positional arguments that this alias has. - """ - pos_args = args[start_index: start_index + num_pos_args] - if len(pos_args) != num_pos_args: - raise CLIError(INSUFFICIENT_POS_ARG_ERROR.format(alias, num_pos_args, len(pos_args))) - - for i, pos_arg in enumerate(pos_args): - yield ('{{{}}}'.format(i), pos_arg) - - @staticmethod - def count_positional_args(arg): - """ - Count how many positional arguments ({0}, {1} ...) there are. - - Args: - arg: The word which this function performs counting on. - - Returns: - The number of placeholders in arg. - """ - return len(re.findall(PLACEHOLDER_REGEX, arg)) diff --git a/src/alias/azext_alias/argument.py b/src/alias/azext_alias/argument.py new file mode 100644 index 00000000000..288c7c9fc9a --- /dev/null +++ b/src/alias/azext_alias/argument.py @@ -0,0 +1,163 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +import re +import shlex + +from knack.util import CLIError + +from jinja2 import Template +from azext_alias._const import ( + POS_ARG_NAME_REGEX, + NUMBER_PLACEHOLDER_REGEX, + DUPLICATED_PLACEHOLDER_ERROR, + RENDER_TEMPLATE_ERROR, + INSUFFICIENT_POS_ARG_ERROR, + PLACEHOLDER_EVAL_ERROR, + EXPRESSION_REGEX +) + + +def get_pos_args_names(arg): + """ + Get all the positional arguments' names in order. + + Args: + arg: The word which this function performs searching on. + + Returns: + A list of positional arguments' names. + """ + pos_args_name = list(map(str.strip, re.findall(POS_ARG_NAME_REGEX, arg))) + + # Make sure there is no duplicated positional argument + if len(pos_args_name) != len(set(pos_args_name)): + raise CLIError(DUPLICATED_PLACEHOLDER_ERROR.format(arg)) + + for i, pos_arg_name in enumerate(pos_args_name): + if pos_arg_name.isdigit(): + pos_args_name[i] = '_{}'.format(pos_arg_name) + + return pos_args_name + + +def stringify_placeholder_expr(arg): + """ + Jinja does not accept numbers as placeholder names, + so add a "_" before the number to make them valid placeholder names. + Surround placeholders expressions with "" so we can preserve spaces inside the positional arguments. + + Args: + arg: The string to process. + + Returns: + A processed string where placeholders are surrounded by "" and + numbered placeholders are prepended with "_". + """ + number_placeholders = list(map(str.strip, re.findall(NUMBER_PLACEHOLDER_REGEX, arg))) + for number_placeholder in number_placeholders: + number = re.search(r'\d+', number_placeholder).group() + arg = arg.replace(number_placeholder, '{{_' + number + '}}') + + return arg.replace('{{', '"{{').replace('}}', '}}"') + + +def build_pos_args_table(full_alias, args, start_index): + """ + Build a dictionary of position argument. + + Args: + full_alias: The full alias (including any placeholders). + args: The arguments that the user inputs in the terminal. + start_index: The index at which we start taking position arguments. + Returns: + A dictionary with the key beign the name of the placeholder and its value + being the respective positional argument. + """ + pos_args_placeholder = get_pos_args_names(full_alias) + pos_args = args[start_index: start_index + len(pos_args_placeholder)] + + if len(pos_args_placeholder) != len(pos_args): + error_msg = INSUFFICIENT_POS_ARG_ERROR.format(full_alias, + len(pos_args_placeholder), + '' if len(pos_args_placeholder) == 1 else 's', + len(pos_args)) + raise CLIError(error_msg) + + # Escape '"' because we are using "" to surround placeholder expressions + for i, pos_arg in enumerate(pos_args): + pos_args[i] = pos_arg.replace('"', '\\"') + + return dict(zip(pos_args_placeholder, pos_args)) + + +def render_template(cmd_derived_from_alias, pos_args_table): + """ + Render cmd_derived_from_alias as a Jinja template with pos_args_table as the arguments. + + Args: + cmd_derived_from_alias: The string to be injected with positional arguemnts. + pos_args_table: The dictionary used to rendered. + Returns: + A processed string with positional arguments injected. + """ + try: + cmd_derived_from_alias = stringify_placeholder_expr(cmd_derived_from_alias) + template = Template(cmd_derived_from_alias) + + # Shlex.split allows us to split a string by spaces while preserving quoted substrings + # (positional arguments in this case) + rendered = shlex.split(template.render(pos_args_table)) + + # Manually check if there is any runtime error (such as index out of range) + # since jinja2 only checks for compile error + # Only check for runtime errors if there is an empty string in rendered + if '' in rendered: + check_runtime_errors(cmd_derived_from_alias, pos_args_table) + + return rendered + except Exception as exception: + if isinstance(exception, CLIError): + raise + + # Template has compile error + split_exception_message = str(exception).split() + + # Check if the error message provides the index of the erroneous character + error_index = split_exception_message[-1] + if error_index.isdigit(): + split_exception_message.insert(-1, 'index') + error_msg = RENDER_TEMPLATE_ERROR.format(' '.join(split_exception_message), cmd_derived_from_alias) + + # Calculate where to put an arrow (^) char so that it is exactly below the erroneous character + # e.g. ... "{{a.split('|)}}" + # ^ + error_msg += '\n{}^'.format(' ' * (len(error_msg) - len(cmd_derived_from_alias) + int(error_index) - 1)) + else: + exception_str = str(exception).replace('"{{', '}}').replace('}}"', '}}') + error_msg = RENDER_TEMPLATE_ERROR.format(cmd_derived_from_alias, exception_str) + + raise CLIError(error_msg) + + +def check_runtime_errors(cmd_derived_from_alias, pos_args_table): + """ + Validate placeholders and their expressions in cmd_derived_from_alias to make sure + that there is no runtime error (such as index out of range). + + Args: + cmd_derived_from_alias: + pos_args_table: + """ + for placeholder, value in pos_args_table.items(): + exec('{} = "{}"'.format(placeholder, value)) # pylint: disable=exec-used + + expressions = list(map(str.strip, re.findall(EXPRESSION_REGEX, cmd_derived_from_alias))) + for expression in expressions: + try: + exec(expression) # pylint: disable=exec-used + except Exception as exception: # pylint: disable=broad-except + error_msg = PLACEHOLDER_EVAL_ERROR.format(expression, exception) + raise CLIError(error_msg) diff --git a/src/alias/azext_alias/tests/_const.py b/src/alias/azext_alias/tests/_const.py index c98e5656380..f63450ee210 100644 --- a/src/alias/azext_alias/tests/_const.py +++ b/src/alias/azext_alias/tests/_const.py @@ -22,23 +22,17 @@ [create-vm] command = vm create -g test-group -n test-vm -[pos-arg-1 {0} {1}] -command = iot {0}test {1}test +[pos-arg-1 {{ 0 }} {{ 1 }}] +command = iot {{ 0 }}test {{ 1 }}test -[pos-arg-2] -command = ad {0} {1} +[pos-arg-2 {{ 0 }} {{ arg_1 }}] +command = sf {{ 0 }} {{ 0 }} {{ arg_1 }} {{ arg_1 }} -[pos-arg-3 {0} {1}] -command = sf {0} {0} {1} {1} +[pos-arg-json {{ 0 }}] +command = test --json {{ 0 }} -[cp {0} {1}] -command = storage blob copy start-batch --source-uri {0} --destination-container {1} - -[show-ext-1 {0}] -command = extension show -n {1} - -[show-ext-2 {1}] -command = extension show -n {0} +[cp {{ arg_1 }} {{ arg_2 }}] +command = storage blob copy start-batch --source-uri {{ arg_1 }} --destination-container {{ arg_2 }} [ac-ls] command = ac ls @@ -46,8 +40,14 @@ [-h] command = account -[storage-connect {0} {1}] -command = az storage account connection-string -g {0} -n {1} -otsv +[storage-connect {{ arg_1 }} {{ arg_2 }}] +command = az storage account connection-string -g {{ arg_1 }} -n {{ arg_2 }} -otsv + +[storage-ls {{ arg_1 }}] +command = storage blob list --account-name {{ arg_1.split(".")[0] }} --container-name {{ arg_1.split("/")[1] }} + +[storage-ls-2 {{ arg_1 }}] +command = storage blob list --account-name {{ arg_1.replace('https://', '').split('.')[0] }} --container-name {{ arg_1.replace("https://", "").split("/")[1] }} ''' COLLISION_MOCK_ALIAS_STRING = ''' diff --git a/src/alias/azext_alias/tests/test_alias.py b/src/alias/azext_alias/tests/test_alias.py index 42bee8f13c0..3af1b93460d 100644 --- a/src/alias/azext_alias/tests/test_alias.py +++ b/src/alias/azext_alias/tests/test_alias.py @@ -7,6 +7,7 @@ import sys import os +import shlex import unittest from six.moves import configparser @@ -24,7 +25,6 @@ TEST_TRANSFORM_ALIAS = 'test_transform_alias' TEST_TRANSFORM_COLLIDED_ALIAS = 'test_transform_collided_alias' TEST_TRANSFORM_EMPTY_STRING = 'test_transform_empty_string' -TEST_POST_TRANSFORM_REMOVE_QUOTES = 'test_post_transform_remove_quotes' TEST_POST_TRANSFORM_ENV_VAR = 'test_post_transform_env_var' TEST_INCONSISTENT_PLACEHOLDER_INDEX = 'test_inconsistent_placeholder_index' TEST_PARSE_ERROR_PYTHON_3 = 'test_parse_error_python_3' @@ -41,16 +41,18 @@ ('-h', '-h'), ('storage-connect test1 test2', 'storage account connection-string -g test1 -n test2 -otsv'), ('', ''), + ('test --json \'{"test": "arg"}\'', 'test --json \'{"test": "arg"}\''), ('ac set -s test', 'account set -s test'), ('vm ls -g test -otable', 'vm list -otable -g test -otable'), ('cp test1 test2', 'storage blob copy start-batch --source-uri test1 --destination-container test2'), ('pos-arg-1 test1 test2', 'iot test1test test2test'), - ('pos-arg-2', 'ad {0} {1}'), - ('pos-arg-3 test1 test2', 'sf test1 test1 test2 test2'), - ('show-ext-1 test-ext', 'extension show -n {1}'), + ('pos-arg-2 test1 test2', 'sf test1 test1 test2 test2'), + ('pos-arg-json \'{"test": "arg"}\'', 'test --json \'{"test": "arg"}\''), ('cp test1 test2 -o tsv', 'storage blob copy start-batch --source-uri test1 --destination-container test2 -o tsv'), ('create-vm --image ubtuntults --generate-ssh-key --no-wait', 'vm create -g test-group -n test-vm --image ubtuntults --generate-ssh-key --no-wait'), - ('cp mn diag', 'storage blob copy start-batch --source-uri mn --destination-container diag') + ('cp mn diag', 'storage blob copy start-batch --source-uri mn --destination-container diag'), + ('storage-ls azurecliprod.blob.core.windows.net/cli-extensions', 'storage blob list --account-name azurecliprod --container-name cli-extensions'), + ('storage-ls-2 https://azurecliprod.blob.core.windows.net/cli-extensions', 'storage blob list --account-name azurecliprod --container-name cli-extensions') ], TEST_TRANSFORM_COLLIDED_ALIAS: [ ('account list -otable', 'account list -otable'), @@ -61,20 +63,14 @@ ], TEST_TRANSFORM_EMPTY_STRING: [ ('network vnet update -g test -n test --dns-servers ""', 'network vnet update -g test -n test --dns-servers'), - ('test1 test2 --query \'\'', 'test1 test2 --query') - ], - TEST_POST_TRANSFORM_REMOVE_QUOTES: [ - (['test', '--json', '\'{"parameters": {"location": {"value": "westus"}, "name": {"value": "azure-cli-deploy-test-nsg1"}}}\''], ['test', '--json', '{"parameters": {"location": {"value": "westus"}, "name": {"value": "azure-cli-deploy-test-nsg1"}}}']), - (['test', '--query', '"query with spaces"'], ['test', '--query', 'query with spaces']), - (['test', '--query', '"[].id"'], ['test', '--query', '[].id'], ['test', '--query', '\'[].id\''], ['test', '--query', '[].id']) + ('test1 test2 --query ""', 'test1 test2 --query') ], TEST_POST_TRANSFORM_ENV_VAR: [ ('group create -n test --tags tag1=$tag1 tag2=$tag2 tag3=$non-existing-env-var', 'group create -n test --tags tag1=test-env-var-1 tag2=test-env-var-2 tag3=$non-existing-env-var') ], TEST_INCONSISTENT_PLACEHOLDER_INDEX: [ ['cp'], - ['cp', 'test'], - ['show-ext-2', 'test-ext'] + ['cp', 'test'] ], TEST_PARSE_ERROR_PYTHON_3: [ DUP_SECTION_MOCK_ALIAS_STRING, @@ -94,23 +90,17 @@ def test_transform_alias(self, test_case): def test_transform_collided_alias(self, test_case): alias_manager = self.get_alias_manager(COLLISION_MOCK_ALIAS_STRING, TEST_RESERVED_COMMANDS) alias_manager.build_collision_table() - self.assertEqual(test_case[1].split(), alias_manager.transform(test_case[0].split())) + self.assertEqual(shlex.split(test_case[1]), alias_manager.transform(shlex.split(test_case[0]))) def test_transform_empty_string(self, test_case): alias_manager = self.get_alias_manager() - transformed_args = alias_manager.transform(test_case[0].split()) - expected_args = test_case[1].split() + transformed_args = alias_manager.transform(shlex.split(test_case[0])) + expected_args = shlex.split(test_case[1]) self.assertEqual(expected_args, transformed_args[:-1]) self.assertEqual('', transformed_args[-1]) -def test_post_transform_remove_quotes(self, test_case): - alias_manager = self.get_alias_manager() - transformed_args = alias_manager.post_transform(test_case[0]) - self.assertListEqual(test_case[1], transformed_args) - - def test_post_transform_env_var(self, test_case): os.environ['tag1'] = 'test-env-var-1' os.environ['tag2'] = 'test-env-var-2' @@ -144,7 +134,6 @@ def test(self): TEST_TRANSFORM_ALIAS: test_transform_alias, TEST_TRANSFORM_COLLIDED_ALIAS: test_transform_collided_alias, TEST_TRANSFORM_EMPTY_STRING: test_transform_empty_string, - TEST_POST_TRANSFORM_REMOVE_QUOTES: test_post_transform_remove_quotes, TEST_POST_TRANSFORM_ENV_VAR: test_post_transform_env_var, TEST_INCONSISTENT_PLACEHOLDER_INDEX: test_inconsistent_placeholder_index, TEST_PARSE_ERROR_PYTHON_3: test_parse_error_python_3, @@ -188,12 +177,11 @@ def get_alias_manager(self, mock_alias_str=DEFAULT_MOCK_ALIAS_STRING, reserved_c def assertAlias(self, value): """ Assert the alias with the default alias config file """ alias_manager = self.get_alias_manager() - self.assertEqual(value[1].split(), alias_manager.transform(value[0].split())) + self.assertEqual(shlex.split(value[1]), alias_manager.transform(shlex.split(value[0]))) def assertPostTransform(self, value, mock_alias_str=DEFAULT_MOCK_ALIAS_STRING): alias_manager = self.get_alias_manager(mock_alias_str=mock_alias_str) - self.assertEqual(value[1].split(), - alias_manager.post_transform(value[0].split())) + self.assertEqual(shlex.split(value[1]), alias_manager.post_transform(shlex.split(value[0]))) class MockAliasManager(alias.AliasManager): @@ -233,5 +221,4 @@ def write_collided_alias(self): if __name__ == '__main__': - test_suite = unittest.TestLoader().loadTestsFromTestCase(TestAlias) - unittest.TextTestRunner(verbosity=2).run(test_suite) + unittest.main() diff --git a/src/alias/azext_alias/tests/test_argument.py b/src/alias/azext_alias/tests/test_argument.py new file mode 100644 index 00000000000..43ab320fdc5 --- /dev/null +++ b/src/alias/azext_alias/tests/test_argument.py @@ -0,0 +1,110 @@ +# -------------------------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for license information. +# -------------------------------------------------------------------------------------------- + +# pylint: disable=line-too-long,no-self-use + +import unittest + +from knack.util import CLIError + +from azext_alias.argument import ( + get_pos_args_names, + stringify_placeholder_expr, + build_pos_args_table, + render_template, + check_runtime_errors +) + + +class TestArgument(unittest.TestCase): + + def test_get_pos_args_names(self): + self.assertListEqual(['arg_1', 'arg_2'], get_pos_args_names('{{ arg_1 }} {{ arg_2 }}')) + + def test_get_pos_args_names_with_numbers(self): + self.assertListEqual(['_0', '_1'], get_pos_args_names('{{ 0 }} {{ 1 }}')) + + def test_get_pos_args_names_duplicate(self): + with self.assertRaises(CLIError): + get_pos_args_names('{{ arg_1 }} {{ arg_1 }}') + + def test_stringify_placeholder_expr(self): + self.assertEqual('"{{ arg_1 }}" "{{ arg_2 }}"', stringify_placeholder_expr('{{ arg_1 }} {{ arg_2 }}')) + + def test_stringify_placeholder_expr_number(self): + self.assertEqual('"{{_0}}" "{{_1}}"', stringify_placeholder_expr('{{ 0 }} {{ 1 }}')) + + def test_build_pos_args_table(self): + expected = { + 'arg_1': 'test_1', + 'arg_2': 'test_2' + } + self.assertDictEqual(expected, build_pos_args_table('{{ arg_1 }} {{ arg_2 }}', ['test_1', 'test_2'], 0)) + + def test_build_pos_args_table_with_spaces(self): + expected = { + '_0': '{\\"test\\": \\"test\\"}', + 'arg_1': 'test1 test2', + 'arg_2': 'arg with spaces', + 'arg_3': '\\"azure cli\\"' + } + self.assertDictEqual(expected, build_pos_args_table('{{ 0 }} {{ arg_1 }} {{ arg_2 }} {{ arg_3 }}', ['{"test": "test"}', 'test1 test2', 'arg with spaces', '"azure cli"'], 0)) + + def test_build_pos_args_table_not_enough_arguments(self): + with self.assertRaises(CLIError): + build_pos_args_table('{{ arg_1 }} {{ arg_2 }}', ['test_1', 'test_2'], 1) + + def test_render_template(self): + pos_args_table = { + 'arg_1': 'test_1', + 'arg_2': 'test_2' + } + self.assertListEqual(['test_1', 'test_2'], render_template('{{ arg_1 }} {{ arg_2 }}', pos_args_table)) + + def test_render_template_pos_arg_with_spaces(self): + pos_args_table = { + 'arg_1': '{\\"test\\": \\"test\\"}', + 'arg_2': 'argument with spaces' + } + self.assertListEqual(['{"test": "test"}', 'argument with spaces'], render_template('{{ arg_1 }} {{ arg_2 }}', pos_args_table)) + + def test_render_template_split_arg(self): + pos_args_table = { + 'arg_1': 'argument with spaces' + } + self.assertListEqual(['argument'], render_template('{{ arg_1.split()[0] }}', pos_args_table)) + + def test_render_template_upper(self): + pos_args_table = { + 'arg_1': 'argument with spaces' + } + self.assertListEqual(['argument with spaces'.upper()], render_template('{{ arg_1.upper() }}', pos_args_table)) + + def test_render_template_error(self): + with self.assertRaises(CLIError): + pos_args_table = { + 'arg_1': 'test_1', + 'arg_2': 'test_2' + } + render_template('{{ arg_1 }} {{ arg_2 }', pos_args_table) + + def test_check_runtime_errors_no_error(self): + pos_args_table = { + 'arg_1': 'test_1', + 'arg_2': 'test_2' + } + check_runtime_errors('{{ arg_1.split("_")[0] }} {{ arg_2.split("_")[1] }}', pos_args_table) + + def test_check_runtime_errors_has_error(self): + with self.assertRaises(CLIError): + pos_args_table = { + 'arg_1': 'test_1', + 'arg_2': 'test_2' + } + check_runtime_errors('{{ arg_1.split("_")[2] }} {{ arg_2.split("_")[1] }}', pos_args_table) + + +if __name__ == '__main__': + unittest.main() diff --git a/src/alias/azext_alias/version.py b/src/alias/azext_alias/version.py index 60641a40d32..7f76f4a5955 100644 --- a/src/alias/azext_alias/version.py +++ b/src/alias/azext_alias/version.py @@ -3,4 +3,4 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -VERSION = '0.1.1' +VERSION = '0.2.0' diff --git a/src/alias/setup.py b/src/alias/setup.py index bd0e8aaea1c..a189ec47cb2 100644 --- a/src/alias/setup.py +++ b/src/alias/setup.py @@ -29,7 +29,9 @@ 'License :: OSI Approved :: MIT License', ] -DEPENDENCIES = [] +DEPENDENCIES = [ + 'jinja2~=2.10' +] setup( name='alias', diff --git a/src/index.json b/src/index.json index b536a29170d..b38f1a6f834 100644 --- a/src/index.json +++ b/src/index.json @@ -550,9 +550,9 @@ ], "alias": [ { - "filename": "alias-0.1.1-py2.py3-none-any.whl", - "sha256Digest": "e096bef74b6ce29046e91330d9a543d4292adc434df2152b3b25a1d129011d90", - "downloadUrl": "https://azurecliprod.blob.core.windows.net/cli-extensions/alias-0.1.1-py2.py3-none-any.whl", + "filename": "alias-0.2.0-py2.py3-none-any.whl", + "sha256Digest": "2bb3736c8477cfd2cd4cc8dd79bc1d670d9dc3f6922c7ce13af62d62495edc0d", + "downloadUrl": "https://azurecliprod.blob.core.windows.net/cli-extensions/alias-0.2.0-py2.py3-none-any.whl", "metadata": { "azext.minCliCoreVersion": "2.0.28", "classifiers": [ @@ -585,12 +585,20 @@ } } }, + "extras": [], "generator": "bdist_wheel (0.29.0)", "license": "MIT", "metadata_version": "2.0", "name": "alias", + "run_requires": [ + { + "requires": [ + "jinja2 (~=2.10)" + ] + } + ], "summary": "Azure CLI Alias Extension", - "version": "0.1.1" + "version": "0.2.0" } } ], From 50c7b71a0568e04391376dde23986a587d1c8fc3 Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 8 Mar 2018 17:29:11 -0800 Subject: [PATCH 2/8] Run tests against temp folder --- scripts/ci/test_source.py | 2 ++ src/alias/azext_alias/argument.py | 2 ++ src/index.json | 2 +- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index a9985afa640..93a30e09b6e 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -40,6 +40,8 @@ def test(self): pip_args = [sys.executable, '-m', 'pip', 'install', '--upgrade', '--target', os.path.join(self.ext_dir, 'ext'), ext_path] check_call(pip_args) + install_dependencies_args = [sys.executable, '-m', 'pip', 'install', '-e', ext_path] + check_call(install_dependencies_args) unittest_args = [sys.executable, '-m', 'unittest', 'discover', '-v', ext_path] check_call(unittest_args) return test diff --git a/src/alias/azext_alias/argument.py b/src/alias/azext_alias/argument.py index 288c7c9fc9a..ea8b5b1a92b 100644 --- a/src/alias/azext_alias/argument.py +++ b/src/alias/azext_alias/argument.py @@ -3,6 +3,8 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- +# pylint: disable=import-error + import re import shlex diff --git a/src/index.json b/src/index.json index b38f1a6f834..20214726f8a 100644 --- a/src/index.json +++ b/src/index.json @@ -551,7 +551,7 @@ "alias": [ { "filename": "alias-0.2.0-py2.py3-none-any.whl", - "sha256Digest": "2bb3736c8477cfd2cd4cc8dd79bc1d670d9dc3f6922c7ce13af62d62495edc0d", + "sha256Digest": "86e6b2daa94dbfaf881ab6f9204cab792a3dbef3932ebe00a45b3d51c5e55ac7", "downloadUrl": "https://azurecliprod.blob.core.windows.net/cli-extensions/alias-0.2.0-py2.py3-none-any.whl", "metadata": { "azext.minCliCoreVersion": "2.0.28", From 0782dc0b2fba3ff194581ac7a4e02fd9a28665c2 Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Fri, 9 Mar 2018 10:49:08 -0800 Subject: [PATCH 3/8] Add test_requirements.txt in ext dir --- scripts/ci/test_source.py | 7 +++++-- src/alias/test_requirements.txt | 1 + 2 files changed, 6 insertions(+), 2 deletions(-) create mode 100644 src/alias/test_requirements.txt diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index 93a30e09b6e..fa26c9490a9 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -40,8 +40,11 @@ def test(self): pip_args = [sys.executable, '-m', 'pip', 'install', '--upgrade', '--target', os.path.join(self.ext_dir, 'ext'), ext_path] check_call(pip_args) - install_dependencies_args = [sys.executable, '-m', 'pip', 'install', '-e', ext_path] - check_call(install_dependencies_args) + test_requirements = os.path.join(ext_path, 'test_requirements.txt') + if os.path.isfile(test_requirements): + install_test_dep_args = [sys.executable, '-m', 'pip', 'install', '--upgrade', + '-r', test_requirements, '--target', ext_path] + check_call(install_test_dep_args) unittest_args = [sys.executable, '-m', 'unittest', 'discover', '-v', ext_path] check_call(unittest_args) return test diff --git a/src/alias/test_requirements.txt b/src/alias/test_requirements.txt new file mode 100644 index 00000000000..b60338e30ad --- /dev/null +++ b/src/alias/test_requirements.txt @@ -0,0 +1 @@ +jinja2==2.10 From 9b61a4dd4500872e66106b7b5d5fa60fb410e415 Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Tue, 13 Mar 2018 15:15:01 -0700 Subject: [PATCH 4/8] Address PR comments --- src/alias/azext_alias/_const.py | 5 +- src/alias/azext_alias/alias.py | 2 +- src/alias/azext_alias/argument.py | 84 +++++++++++++------- src/alias/azext_alias/tests/test_argument.py | 51 +++++++++--- src/index.json | 2 +- 5 files changed, 95 insertions(+), 49 deletions(-) diff --git a/src/alias/azext_alias/_const.py b/src/alias/azext_alias/_const.py index 2e19ac67f4d..be8fd5c4629 100644 --- a/src/alias/azext_alias/_const.py +++ b/src/alias/azext_alias/_const.py @@ -11,10 +11,6 @@ COLLIDED_ALIAS_FILE_NAME = 'collided_alias' COLLISION_CHECK_LEVEL_DEPTH = 4 -POS_ARG_NAME_REGEX = r'{{\s*([a-zA-Z0-9_]+)\s*}}' -EXPRESSION_REGEX = r'{{\s*([a-zA-Z0-9_]+.*?)\s*}}' -NUMBER_PLACEHOLDER_REGEX = r'{{\s*\d+\s*}}' - INSUFFICIENT_POS_ARG_ERROR = 'alias: "{}" takes exactly {} positional argument{} ({} given)' CONFIG_PARSING_ERROR = 'alias: Error parsing the configuration file - {}. Please fix the problem manually.' DEBUG_MSG = 'Alias Manager: Transforming "%s" to "%s"' @@ -23,3 +19,4 @@ DUPLICATED_PLACEHOLDER_ERROR = 'alias: Duplicated placeholders found when transforming "{}"' RENDER_TEMPLATE_ERROR = 'alias: Encounted the following error when injecting positional arguments to "{}" - {}' PLACEHOLDER_EVAL_ERROR = 'alias: Encounted the following error when evaluating "{}" - {}' +PLACEHOLDER_BRACKETS_ERROR = 'alias: Brackets in "{}" are not enclosed properly' diff --git a/src/alias/azext_alias/alias.py b/src/alias/azext_alias/alias.py index 4aed8f32c66..397e56c7d26 100644 --- a/src/alias/azext_alias/alias.py +++ b/src/alias/azext_alias/alias.py @@ -223,7 +223,7 @@ def load_full_command_table(self): Perform a full load of the command table to get all the reserved command words. """ load_cmd_tbl_func = self.kwargs.get('load_cmd_tbl_func', None) - if callable(load_cmd_tbl_func): + if load_cmd_tbl_func is not None: self.reserved_commands = list(load_cmd_tbl_func([]).keys()) telemetry.set_full_command_table_loaded() diff --git a/src/alias/azext_alias/argument.py b/src/alias/azext_alias/argument.py index ea8b5b1a92b..de55806f38a 100644 --- a/src/alias/azext_alias/argument.py +++ b/src/alias/azext_alias/argument.py @@ -10,60 +10,80 @@ from knack.util import CLIError -from jinja2 import Template +import jinja2 as jinja from azext_alias._const import ( - POS_ARG_NAME_REGEX, - NUMBER_PLACEHOLDER_REGEX, DUPLICATED_PLACEHOLDER_ERROR, RENDER_TEMPLATE_ERROR, INSUFFICIENT_POS_ARG_ERROR, PLACEHOLDER_EVAL_ERROR, - EXPRESSION_REGEX + PLACEHOLDER_BRACKETS_ERROR ) -def get_pos_args_names(arg): +def get_placeholders(arg, check_duplicates=False): """ - Get all the positional arguments' names in order. + Get all the placeholders' names in order. + Use the regex below to locate all the opening ({{) and closing brackets (}}). + After that, extract "stuff" inside the brackets. Args: arg: The word which this function performs searching on. + check_duplicates: True if we want to check for duplicated positional arguments. Returns: - A list of positional arguments' names. + A list of positional arguments in order. """ - pos_args_name = list(map(str.strip, re.findall(POS_ARG_NAME_REGEX, arg))) - - # Make sure there is no duplicated positional argument - if len(pos_args_name) != len(set(pos_args_name)): + placeholders = [] + last_match = None + arg = normalize_placeholders(arg) + for cur_match in re.finditer(r'\s*{{|}}\s*', arg): + matched_text = cur_match.group().strip() + if not last_match and matched_text == '{{': + last_match = cur_match + continue + + last_matched_text = '' if not last_match else last_match.group().strip() + # Check if the positional argument is enclosed with {{ }} properly + if (not last_matched_text and matched_text == '}}') or (last_matched_text == '{{' and matched_text != '}}'): + raise CLIError(PLACEHOLDER_BRACKETS_ERROR.format(arg)) + elif last_matched_text == '{{' and matched_text == '}}': + # Extract start and end index of the placeholder name + start_index, end_index = last_match.span()[1], cur_match.span()[0] + placeholders.append(arg[start_index: end_index].strip()) + last_match = None + + # last_match did not reset - that means brackets are not enclosed properly + if last_match: + raise CLIError(PLACEHOLDER_BRACKETS_ERROR.format(arg)) + + # Make sure there is no duplicated placeholder names + if check_duplicates and len(placeholders) != len(set(placeholders)): raise CLIError(DUPLICATED_PLACEHOLDER_ERROR.format(arg)) - for i, pos_arg_name in enumerate(pos_args_name): - if pos_arg_name.isdigit(): - pos_args_name[i] = '_{}'.format(pos_arg_name) - - return pos_args_name + return placeholders -def stringify_placeholder_expr(arg): +def normalize_placeholders(arg, inject_quotes=False): """ - Jinja does not accept numbers as placeholder names, - so add a "_" before the number to make them valid placeholder names. - Surround placeholders expressions with "" so we can preserve spaces inside the positional arguments. + Normalize names so that the template can be ingested into Jinja template engine + - Jinja does not accept numbers as placeholder names, so add a "_" + before the number to make them valid placeholder names. + - Surround placeholders expressions with "" so we can preserve spaces inside the positional arguments. Args: arg: The string to process. + inject_qoutes: True if we want to surround placeholders with a pair of quotes Returns: A processed string where placeholders are surrounded by "" and numbered placeholders are prepended with "_". """ - number_placeholders = list(map(str.strip, re.findall(NUMBER_PLACEHOLDER_REGEX, arg))) + number_placeholders = re.findall(r'{{\s*\d+\s*}}', arg) for number_placeholder in number_placeholders: number = re.search(r'\d+', number_placeholder).group() arg = arg.replace(number_placeholder, '{{_' + number + '}}') - return arg.replace('{{', '"{{').replace('}}', '}}"') + return arg.replace('{{', '"{{').replace('}}', '}}"') if inject_quotes else arg def build_pos_args_table(full_alias, args, start_index): @@ -74,11 +94,12 @@ def build_pos_args_table(full_alias, args, start_index): full_alias: The full alias (including any placeholders). args: The arguments that the user inputs in the terminal. start_index: The index at which we start taking position arguments. + Returns: A dictionary with the key beign the name of the placeholder and its value being the respective positional argument. """ - pos_args_placeholder = get_pos_args_names(full_alias) + pos_args_placeholder = get_placeholders(full_alias, check_duplicates=True) pos_args = args[start_index: start_index + len(pos_args_placeholder)] if len(pos_args_placeholder) != len(pos_args): @@ -102,29 +123,31 @@ def render_template(cmd_derived_from_alias, pos_args_table): Args: cmd_derived_from_alias: The string to be injected with positional arguemnts. pos_args_table: The dictionary used to rendered. + Returns: A processed string with positional arguments injected. """ try: - cmd_derived_from_alias = stringify_placeholder_expr(cmd_derived_from_alias) - template = Template(cmd_derived_from_alias) + cmd_derived_from_alias = normalize_placeholders(cmd_derived_from_alias, inject_quotes=True) + template = jinja.Template(cmd_derived_from_alias) # Shlex.split allows us to split a string by spaces while preserving quoted substrings # (positional arguments in this case) rendered = shlex.split(template.render(pos_args_table)) # Manually check if there is any runtime error (such as index out of range) - # since jinja2 only checks for compile error + # since Jinja template engine only checks for compile error # Only check for runtime errors if there is an empty string in rendered if '' in rendered: check_runtime_errors(cmd_derived_from_alias, pos_args_table) return rendered except Exception as exception: + # Exception raised from runtime error if isinstance(exception, CLIError): raise - # Template has compile error + # Template has some sort of compile errors split_exception_message = str(exception).split() # Check if the error message provides the index of the erroneous character @@ -150,13 +173,14 @@ def check_runtime_errors(cmd_derived_from_alias, pos_args_table): that there is no runtime error (such as index out of range). Args: - cmd_derived_from_alias: - pos_args_table: + cmd_derived_from_alias: The command derived from the alias + (include any positional argument placehodlers) + pos_args_table: The positional argument table. """ for placeholder, value in pos_args_table.items(): exec('{} = "{}"'.format(placeholder, value)) # pylint: disable=exec-used - expressions = list(map(str.strip, re.findall(EXPRESSION_REGEX, cmd_derived_from_alias))) + expressions = get_placeholders(cmd_derived_from_alias) for expression in expressions: try: exec(expression) # pylint: disable=exec-used diff --git a/src/alias/azext_alias/tests/test_argument.py b/src/alias/azext_alias/tests/test_argument.py index 43ab320fdc5..3913d73360d 100644 --- a/src/alias/azext_alias/tests/test_argument.py +++ b/src/alias/azext_alias/tests/test_argument.py @@ -3,15 +3,15 @@ # Licensed under the MIT License. See License.txt in the project root for license information. # -------------------------------------------------------------------------------------------- -# pylint: disable=line-too-long,no-self-use +# pylint: disable=line-too-long,no-self-use,too-many-public-methods import unittest from knack.util import CLIError from azext_alias.argument import ( - get_pos_args_names, - stringify_placeholder_expr, + get_placeholders, + normalize_placeholders, build_pos_args_table, render_template, check_runtime_errors @@ -20,21 +20,46 @@ class TestArgument(unittest.TestCase): - def test_get_pos_args_names(self): - self.assertListEqual(['arg_1', 'arg_2'], get_pos_args_names('{{ arg_1 }} {{ arg_2 }}')) + def test_get_placeholders(self): + self.assertListEqual(['arg_1', 'arg_2'], get_placeholders('{{ arg_1 }} {{ arg_2 }}')) - def test_get_pos_args_names_with_numbers(self): - self.assertListEqual(['_0', '_1'], get_pos_args_names('{{ 0 }} {{ 1 }}')) + def test_get_placeholders_with_numbers(self): + self.assertListEqual(['_0', '_1'], get_placeholders('{{ 0 }} {{ 1 }}')) - def test_get_pos_args_names_duplicate(self): + def test_get_placeholders_with_strings_and_numbers(self): + self.assertListEqual(['_0', '_1', 'arg_1', 'arg_2'], get_placeholders('{{ 0 }} {{ 1 }} {{ arg_1 }} {{ arg_2 }}')) + + def test_get_placeholders_duplicate(self): + with self.assertRaises(CLIError): + get_placeholders('{{ arg_1 }} {{ arg_1 }}', check_duplicates=True) + + def test_get_placeholders_no_opening_bracket(self): with self.assertRaises(CLIError): - get_pos_args_names('{{ arg_1 }} {{ arg_1 }}') + get_placeholders('arg_1 }}') + + def test_get_placeholders_double_opening_bracket(self): + with self.assertRaises(CLIError): + get_placeholders('{{ {{ arg_1') + + def test_get_placeholders_double_closing_bracket(self): + with self.assertRaises(CLIError): + get_placeholders('{{ arg_1 }} }}') + + def test_get_placeholders_no_closing_bracket(self): + with self.assertRaises(CLIError): + get_placeholders('{{ arg_1 ') + + def test_normalize_placeholders(self): + self.assertEqual('"{{ arg_1 }}" "{{ arg_2 }}"', normalize_placeholders('{{ arg_1 }} {{ arg_2 }}', inject_quotes=True)) + + def test_normalize_placeholders_number(self): + self.assertEqual('"{{_0}}" "{{_1}}"', normalize_placeholders('{{ 0 }} {{ 1 }}', inject_quotes=True)) - def test_stringify_placeholder_expr(self): - self.assertEqual('"{{ arg_1 }}" "{{ arg_2 }}"', stringify_placeholder_expr('{{ arg_1 }} {{ arg_2 }}')) + def test_normalize_placeholders_no_quotes(self): + self.assertEqual('{{_0}} {{_1}}', normalize_placeholders('{{ 0 }} {{ 1 }}')) - def test_stringify_placeholder_expr_number(self): - self.assertEqual('"{{_0}}" "{{_1}}"', stringify_placeholder_expr('{{ 0 }} {{ 1 }}')) + def test_normalize_placeholders_number_no_quotes(self): + self.assertEqual('{{_0}} {{_1}}', normalize_placeholders('{{ 0 }} {{ 1 }}')) def test_build_pos_args_table(self): expected = { diff --git a/src/index.json b/src/index.json index 20214726f8a..559ab9d23e3 100644 --- a/src/index.json +++ b/src/index.json @@ -551,7 +551,7 @@ "alias": [ { "filename": "alias-0.2.0-py2.py3-none-any.whl", - "sha256Digest": "86e6b2daa94dbfaf881ab6f9204cab792a3dbef3932ebe00a45b3d51c5e55ac7", + "sha256Digest": "d330dcba6c0fcbaadf7fb9e6b5d4e9a63c5431649e3945d5317cf460fef55bde", "downloadUrl": "https://azurecliprod.blob.core.windows.net/cli-extensions/alias-0.2.0-py2.py3-none-any.whl", "metadata": { "azext.minCliCoreVersion": "2.0.28", From bcfab3ae48d0a91f01cd2be2649cf02cab076b6a Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 15 Mar 2018 09:56:22 -0700 Subject: [PATCH 5/8] Address PR comments --- src/alias/azext_alias/alias.py | 7 +++---- src/alias/azext_alias/argument.py | 16 ++++++++-------- src/index.json | 2 +- 3 files changed, 12 insertions(+), 13 deletions(-) diff --git a/src/alias/azext_alias/alias.py b/src/alias/azext_alias/alias.py index 397e56c7d26..7569fd0ad12 100644 --- a/src/alias/azext_alias/alias.py +++ b/src/alias/azext_alias/alias.py @@ -222,10 +222,9 @@ def load_full_command_table(self): """ Perform a full load of the command table to get all the reserved command words. """ - load_cmd_tbl_func = self.kwargs.get('load_cmd_tbl_func', None) - if load_cmd_tbl_func is not None: - self.reserved_commands = list(load_cmd_tbl_func([]).keys()) - telemetry.set_full_command_table_loaded() + load_cmd_tbl_func = self.kwargs.get('load_cmd_tbl_func', lambda _: {}) + self.reserved_commands = list(load_cmd_tbl_func([]).keys()) + telemetry.set_full_command_table_loaded() def post_transform(self, args): """ diff --git a/src/alias/azext_alias/argument.py b/src/alias/azext_alias/argument.py index de55806f38a..ca5a3d8d11f 100644 --- a/src/alias/azext_alias/argument.py +++ b/src/alias/azext_alias/argument.py @@ -65,14 +65,14 @@ def get_placeholders(arg, check_duplicates=False): def normalize_placeholders(arg, inject_quotes=False): """ - Normalize names so that the template can be ingested into Jinja template engine + Normalize placeholders' names so that the template can be ingested into Jinja template engine. - Jinja does not accept numbers as placeholder names, so add a "_" - before the number to make them valid placeholder names. + before the numbers to make them valid placeholder names. - Surround placeholders expressions with "" so we can preserve spaces inside the positional arguments. Args: arg: The string to process. - inject_qoutes: True if we want to surround placeholders with a pair of quotes + inject_qoutes: True if we want to surround placeholders with a pair of quotes. Returns: A processed string where placeholders are surrounded by "" and @@ -88,12 +88,12 @@ def normalize_placeholders(arg, inject_quotes=False): def build_pos_args_table(full_alias, args, start_index): """ - Build a dictionary of position argument. + Build a dictionary where the key is placeholder name and the value is the position argument value. Args: full_alias: The full alias (including any placeholders). args: The arguments that the user inputs in the terminal. - start_index: The index at which we start taking position arguments. + start_index: The index at which we start ingesting position arguments. Returns: A dictionary with the key beign the name of the placeholder and its value @@ -136,8 +136,8 @@ def render_template(cmd_derived_from_alias, pos_args_table): rendered = shlex.split(template.render(pos_args_table)) # Manually check if there is any runtime error (such as index out of range) - # since Jinja template engine only checks for compile error - # Only check for runtime errors if there is an empty string in rendered + # since Jinja template engine only checks for compile time error. + # Only check for runtime errors if there is an empty string in rendered. if '' in rendered: check_runtime_errors(cmd_derived_from_alias, pos_args_table) @@ -147,7 +147,7 @@ def render_template(cmd_derived_from_alias, pos_args_table): if isinstance(exception, CLIError): raise - # Template has some sort of compile errors + # The template has some sort of compile time errors split_exception_message = str(exception).split() # Check if the error message provides the index of the erroneous character diff --git a/src/index.json b/src/index.json index 559ab9d23e3..3c797b352c3 100644 --- a/src/index.json +++ b/src/index.json @@ -551,7 +551,7 @@ "alias": [ { "filename": "alias-0.2.0-py2.py3-none-any.whl", - "sha256Digest": "d330dcba6c0fcbaadf7fb9e6b5d4e9a63c5431649e3945d5317cf460fef55bde", + "sha256Digest": "3f01195ad2ce32d4332276d63243b064da8df6490509e103c2bb0295a7735425", "downloadUrl": "https://azurecliprod.blob.core.windows.net/cli-extensions/alias-0.2.0-py2.py3-none-any.whl", "metadata": { "azext.minCliCoreVersion": "2.0.28", From 0b5312aff59d7f9906d6c69644ae078365b9127d Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 15 Mar 2018 12:58:58 -0700 Subject: [PATCH 6/8] Set PYTHONPATH to temp folder when executing tests --- scripts/ci/test_source.py | 10 +++------- src/alias/test_requirements.txt | 1 - 2 files changed, 3 insertions(+), 8 deletions(-) delete mode 100644 src/alias/test_requirements.txt diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index fa26c9490a9..b381203e963 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -37,16 +37,12 @@ def __new__(mcs, name, bases, _dict): def gen_test(ext_path): def test(self): + ext_install_dir = os.path.join(self.ext_dir, 'ext') pip_args = [sys.executable, '-m', 'pip', 'install', '--upgrade', '--target', - os.path.join(self.ext_dir, 'ext'), ext_path] + ext_install_dir, ext_path] check_call(pip_args) - test_requirements = os.path.join(ext_path, 'test_requirements.txt') - if os.path.isfile(test_requirements): - install_test_dep_args = [sys.executable, '-m', 'pip', 'install', '--upgrade', - '-r', test_requirements, '--target', ext_path] - check_call(install_test_dep_args) unittest_args = [sys.executable, '-m', 'unittest', 'discover', '-v', ext_path] - check_call(unittest_args) + check_call(unittest_args, env={'PYTHONPATH': ext_install_dir}) return test for tname, ext_path in ALL_TESTS: diff --git a/src/alias/test_requirements.txt b/src/alias/test_requirements.txt deleted file mode 100644 index b60338e30ad..00000000000 --- a/src/alias/test_requirements.txt +++ /dev/null @@ -1 +0,0 @@ -jinja2==2.10 From 955505667e30e1095d1404ce9134dcf8ecb441c3 Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 15 Mar 2018 14:09:19 -0700 Subject: [PATCH 7/8] Concatenate existing PYHTONPATH with the temp folder --- scripts/ci/test_source.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index b381203e963..0e78bc96d35 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -42,7 +42,9 @@ def test(self): ext_install_dir, ext_path] check_call(pip_args) unittest_args = [sys.executable, '-m', 'unittest', 'discover', '-v', ext_path] - check_call(unittest_args, env={'PYTHONPATH': ext_install_dir}) + check_call(unittest_args, env={ + 'PYTHONPATH': '{}{}:'.format(os.environ.get('PYTHONPATH', ''), ext_install_dir) + }) return test for tname, ext_path in ALL_TESTS: From 6f71339993ec1992b28fb5c346bd17bb1fcc9c05 Mon Sep 17 00:00:00 2001 From: Ernest Wong Date: Thu, 15 Mar 2018 14:21:37 -0700 Subject: [PATCH 8/8] Use os.environ.copy() --- scripts/ci/test_source.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/scripts/ci/test_source.py b/scripts/ci/test_source.py index 0e78bc96d35..42d323878d5 100755 --- a/scripts/ci/test_source.py +++ b/scripts/ci/test_source.py @@ -42,9 +42,9 @@ def test(self): ext_install_dir, ext_path] check_call(pip_args) unittest_args = [sys.executable, '-m', 'unittest', 'discover', '-v', ext_path] - check_call(unittest_args, env={ - 'PYTHONPATH': '{}{}:'.format(os.environ.get('PYTHONPATH', ''), ext_install_dir) - }) + env = os.environ.copy() + env['PYTHONPATH'] = ext_install_dir + check_call(unittest_args, env=env) return test for tname, ext_path in ALL_TESTS: