From 68510596d31963c078bcec94e2b28b8b03b795c3 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Fri, 24 Jul 2020 15:30:36 -0700 Subject: [PATCH] warn on old list-style configuration --- pre_commit/clientlib.py | 45 +++++++++++++++++++++++++---------------- pre_commit/color.py | 10 +++++++++ pre_commit/main.py | 35 ++++++++++++-------------------- tests/clientlib_test.py | 9 ++++++++- 4 files changed, 59 insertions(+), 40 deletions(-) diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 56ec0dd1b..8dfa9473e 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -12,8 +12,10 @@ from identify.identify import ALL_TAGS import pre_commit.constants as C +from pre_commit.color import add_color_option from pre_commit.error_handler import FatalError from pre_commit.languages.all import all_languages +from pre_commit.logging_handler import logging_handler from pre_commit.util import parse_version from pre_commit.util import yaml_load @@ -43,6 +45,7 @@ def _make_argparser(filenames_help: str) -> argparse.ArgumentParser: parser = argparse.ArgumentParser() parser.add_argument('filenames', nargs='*', help=filenames_help) parser.add_argument('-V', '--version', action='version', version=C.VERSION) + add_color_option(parser) return parser @@ -92,14 +95,16 @@ class InvalidManifestError(FatalError): def validate_manifest_main(argv: Optional[Sequence[str]] = None) -> int: parser = _make_argparser('Manifest filenames.') args = parser.parse_args(argv) - ret = 0 - for filename in args.filenames: - try: - load_manifest(filename) - except InvalidManifestError as e: - print(e) - ret = 1 - return ret + + with logging_handler(args.color): + ret = 0 + for filename in args.filenames: + try: + load_manifest(filename) + except InvalidManifestError as e: + print(e) + ret = 1 + return ret LOCAL = 'local' @@ -290,7 +295,11 @@ class InvalidConfigError(FatalError): def ordered_load_normalize_legacy_config(contents: str) -> Dict[str, Any]: data = yaml_load(contents) if isinstance(data, list): - # TODO: Once happy, issue a deprecation warning and instructions + logger.warning( + 'normalizing pre-commit configuration to a top-level map. ' + 'support for top level list will be removed in a future version. ' + 'run: `pre-commit migrate-config` to automatically fix this.', + ) return {'repos': data} else: return data @@ -307,11 +316,13 @@ def ordered_load_normalize_legacy_config(contents: str) -> Dict[str, Any]: def validate_config_main(argv: Optional[Sequence[str]] = None) -> int: parser = _make_argparser('Config filenames.') args = parser.parse_args(argv) - ret = 0 - for filename in args.filenames: - try: - load_config(filename) - except InvalidConfigError as e: - print(e) - ret = 1 - return ret + + with logging_handler(args.color): + ret = 0 + for filename in args.filenames: + try: + load_config(filename) + except InvalidConfigError as e: + print(e) + ret = 1 + return ret diff --git a/pre_commit/color.py b/pre_commit/color.py index eb906b78f..4ddfdf5b3 100644 --- a/pre_commit/color.py +++ b/pre_commit/color.py @@ -1,3 +1,4 @@ +import argparse import os import sys @@ -95,3 +96,12 @@ def use_color(setting: str) -> bool: os.getenv('TERM') != 'dumb' ) ) + + +def add_color_option(parser: argparse.ArgumentParser) -> None: + parser.add_argument( + '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'), + type=use_color, + metavar='{' + ','.join(COLOR_CHOICES) + '}', + help='Whether to use color in output. Defaults to `%(default)s`.', + ) diff --git a/pre_commit/main.py b/pre_commit/main.py index ffcc2e875..86479607c 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -8,8 +8,8 @@ from typing import Union import pre_commit.constants as C -from pre_commit import color from pre_commit import git +from pre_commit.color import add_color_option from pre_commit.commands.autoupdate import autoupdate from pre_commit.commands.clean import clean from pre_commit.commands.gc import gc @@ -41,15 +41,6 @@ COMMANDS_NO_GIT = {'clean', 'gc', 'init-templatedir', 'sample-config'} -def _add_color_option(parser: argparse.ArgumentParser) -> None: - parser.add_argument( - '--color', default=os.environ.get('PRE_COMMIT_COLOR', 'auto'), - type=color.use_color, - metavar='{' + ','.join(color.COLOR_CHOICES) + '}', - help='Whether to use color in output. Defaults to `%(default)s`.', - ) - - def _add_config_option(parser: argparse.ArgumentParser) -> None: parser.add_argument( '-c', '--config', default=C.CONFIG_FILE, @@ -195,7 +186,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: 'autoupdate', help="Auto-update pre-commit config to the latest repos' versions.", ) - _add_color_option(autoupdate_parser) + add_color_option(autoupdate_parser) _add_config_option(autoupdate_parser) autoupdate_parser.add_argument( '--bleeding-edge', action='store_true', @@ -216,11 +207,11 @@ def main(argv: Optional[Sequence[str]] = None) -> int: clean_parser = subparsers.add_parser( 'clean', help='Clean out pre-commit files.', ) - _add_color_option(clean_parser) + add_color_option(clean_parser) _add_config_option(clean_parser) hook_impl_parser = subparsers.add_parser('hook-impl') - _add_color_option(hook_impl_parser) + add_color_option(hook_impl_parser) _add_config_option(hook_impl_parser) hook_impl_parser.add_argument('--hook-type') hook_impl_parser.add_argument('--hook-dir') @@ -230,7 +221,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: hook_impl_parser.add_argument(dest='rest', nargs=argparse.REMAINDER) gc_parser = subparsers.add_parser('gc', help='Clean unused cached repos.') - _add_color_option(gc_parser) + add_color_option(gc_parser) _add_config_option(gc_parser) init_templatedir_parser = subparsers.add_parser( @@ -240,7 +231,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: '`git config init.templateDir`.' ), ) - _add_color_option(init_templatedir_parser) + add_color_option(init_templatedir_parser) _add_config_option(init_templatedir_parser) init_templatedir_parser.add_argument( 'directory', help='The directory in which to write the hook script.', @@ -256,7 +247,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: install_parser = subparsers.add_parser( 'install', help='Install the pre-commit script.', ) - _add_color_option(install_parser) + add_color_option(install_parser) _add_config_option(install_parser) install_parser.add_argument( '-f', '--overwrite', action='store_true', @@ -286,32 +277,32 @@ def main(argv: Optional[Sequence[str]] = None) -> int: 'useful.' ), ) - _add_color_option(install_hooks_parser) + add_color_option(install_hooks_parser) _add_config_option(install_hooks_parser) migrate_config_parser = subparsers.add_parser( 'migrate-config', help='Migrate list configuration to new map configuration.', ) - _add_color_option(migrate_config_parser) + add_color_option(migrate_config_parser) _add_config_option(migrate_config_parser) run_parser = subparsers.add_parser('run', help='Run hooks.') - _add_color_option(run_parser) + add_color_option(run_parser) _add_config_option(run_parser) _add_run_options(run_parser) sample_config_parser = subparsers.add_parser( 'sample-config', help=f'Produce a sample {C.CONFIG_FILE} file', ) - _add_color_option(sample_config_parser) + add_color_option(sample_config_parser) _add_config_option(sample_config_parser) try_repo_parser = subparsers.add_parser( 'try-repo', help='Try the hooks in a repository, useful for developing new hooks.', ) - _add_color_option(try_repo_parser) + add_color_option(try_repo_parser) _add_config_option(try_repo_parser) try_repo_parser.add_argument( 'repo', help='Repository to source hooks from.', @@ -328,7 +319,7 @@ def main(argv: Optional[Sequence[str]] = None) -> int: uninstall_parser = subparsers.add_parser( 'uninstall', help='Uninstall the pre-commit script.', ) - _add_color_option(uninstall_parser) + add_color_option(uninstall_parser) _add_config_option(uninstall_parser) _add_hook_type_option(uninstall_parser) diff --git a/tests/clientlib_test.py b/tests/clientlib_test.py index c48adbde9..2e2f738c9 100644 --- a/tests/clientlib_test.py +++ b/tests/clientlib_test.py @@ -30,6 +30,10 @@ def test_check_type_tag_failures(value): check_type_tag(value) +def test_check_type_tag_success(): + check_type_tag('file') + + @pytest.mark.parametrize( ('config_obj', 'expected'), ( ( @@ -110,15 +114,18 @@ def test_validate_config_main_ok(): assert not validate_config_main(('.pre-commit-config.yaml',)) -def test_validate_config_old_list_format_ok(tmpdir): +def test_validate_config_old_list_format_ok(tmpdir, cap_out): f = tmpdir.join('cfg.yaml') f.write('- {repo: meta, hooks: [{id: identity}]}') assert not validate_config_main((f.strpath,)) + start = '[WARNING] normalizing pre-commit configuration to a top-level map' + assert cap_out.get().startswith(start) def test_validate_warn_on_unknown_keys_at_repo_level(tmpdir, caplog): f = tmpdir.join('cfg.yaml') f.write( + 'repos:\n' '- repo: https://gitlab.com/pycqa/flake8\n' ' rev: 3.7.7\n' ' hooks:\n'