diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py index 1fcce4eaf..bf4e2e455 100644 --- a/pre_commit/clientlib.py +++ b/pre_commit/clientlib.py @@ -336,6 +336,11 @@ def check(self, dct: dict[str, Any]) -> None: 'Config', None, cfgv.RequiredRecurse('repos', cfgv.Array(CONFIG_REPO_DICT)), + cfgv.Optional( + 'default_install_hook_types', + cfgv.check_array(cfgv.check_one_of(C.HOOK_TYPES)), + ['pre-commit'], + ), cfgv.OptionalRecurse( 'default_language_version', DEFAULT_LANGUAGE_VERSION, {}, ), @@ -355,6 +360,7 @@ def check(self, dct: dict[str, Any]) -> None: cfgv.WarnAdditionalKeys( ( 'repos', + 'default_install_hook_types', 'default_language_version', 'default_stages', 'files', diff --git a/pre_commit/commands/init_templatedir.py b/pre_commit/commands/init_templatedir.py index 004e8ccfc..08af6561e 100644 --- a/pre_commit/commands/init_templatedir.py +++ b/pre_commit/commands/init_templatedir.py @@ -2,7 +2,6 @@ import logging import os.path -from typing import Sequence from pre_commit.commands.install_uninstall import install from pre_commit.store import Store @@ -16,7 +15,7 @@ def init_templatedir( config_file: str, store: Store, directory: str, - hook_types: Sequence[str], + hook_types: list[str] | None, skip_on_missing_config: bool = True, ) -> int: install( diff --git a/pre_commit/commands/install_uninstall.py b/pre_commit/commands/install_uninstall.py index b1fd8d490..5ff6cba6e 100644 --- a/pre_commit/commands/install_uninstall.py +++ b/pre_commit/commands/install_uninstall.py @@ -5,10 +5,10 @@ import shlex import shutil import sys -from typing import Sequence from pre_commit import git from pre_commit import output +from pre_commit.clientlib import InvalidConfigError from pre_commit.clientlib import load_config from pre_commit.repository import all_hooks from pre_commit.repository import install_hook_envs @@ -32,6 +32,18 @@ TEMPLATE_END = '# end templated\n' +def _hook_types(cfg_filename: str, hook_types: list[str] | None) -> list[str]: + if hook_types is not None: + return hook_types + else: + try: + cfg = load_config(cfg_filename) + except InvalidConfigError: + return ['pre-commit'] + else: + return cfg['default_install_hook_types'] + + def _hook_paths( hook_type: str, git_dir: str | None = None, @@ -103,7 +115,7 @@ def _install_hook_script( def install( config_file: str, store: Store, - hook_types: Sequence[str], + hook_types: list[str] | None, overwrite: bool = False, hooks: bool = False, skip_on_missing_config: bool = False, @@ -116,7 +128,7 @@ def install( ) return 1 - for hook_type in hook_types: + for hook_type in _hook_types(config_file, hook_types): _install_hook_script( config_file, hook_type, overwrite=overwrite, @@ -150,7 +162,7 @@ def _uninstall_hook_script(hook_type: str) -> None: output.write_line(f'Restored previous hooks to {hook_path}') -def uninstall(hook_types: Sequence[str]) -> int: - for hook_type in hook_types: +def uninstall(config_file: str, hook_types: list[str] | None) -> int: + for hook_type in _hook_types(config_file, hook_types): _uninstall_hook_script(hook_type) return 0 diff --git a/pre_commit/constants.py b/pre_commit/constants.py index 40127a052..5bc4ae98b 100644 --- a/pre_commit/constants.py +++ b/pre_commit/constants.py @@ -24,4 +24,10 @@ 'post-rewrite', ) +HOOK_TYPES = ( + 'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg', + 'commit-msg', 'post-commit', 'post-checkout', 'post-merge', + 'post-rewrite', +) + DEFAULT = 'default' diff --git a/pre_commit/main.py b/pre_commit/main.py index f3c551885..645e97f74 100644 --- a/pre_commit/main.py +++ b/pre_commit/main.py @@ -4,7 +4,6 @@ import logging import os import sys -from typing import Any from typing import Sequence import pre_commit.constants as C @@ -46,34 +45,10 @@ def _add_config_option(parser: argparse.ArgumentParser) -> None: ) -class AppendReplaceDefault(argparse.Action): - def __init__(self, *args: Any, **kwargs: Any) -> None: - super().__init__(*args, **kwargs) - self.appended = False - - def __call__( - self, - parser: argparse.ArgumentParser, - namespace: argparse.Namespace, - values: str | Sequence[str] | None, - option_string: str | None = None, - ) -> None: - if not self.appended: - setattr(namespace, self.dest, []) - self.appended = True - getattr(namespace, self.dest).append(values) - - def _add_hook_type_option(parser: argparse.ArgumentParser) -> None: parser.add_argument( - '-t', '--hook-type', choices=( - 'pre-commit', 'pre-merge-commit', 'pre-push', 'prepare-commit-msg', - 'commit-msg', 'post-commit', 'post-checkout', 'post-merge', - 'post-rewrite', - ), - action=AppendReplaceDefault, - default=['pre-commit'], - dest='hook_types', + '-t', '--hook-type', + choices=C.HOOK_TYPES, action='append', dest='hook_types', ) @@ -399,7 +374,10 @@ def main(argv: Sequence[str] | None = None) -> int: elif args.command == 'try-repo': return try_repo(args) elif args.command == 'uninstall': - return uninstall(hook_types=args.hook_types) + return uninstall( + config_file=args.config, + hook_types=args.hook_types, + ) else: raise NotImplementedError( f'Command {args.command} not implemented.', diff --git a/tests/commands/install_uninstall_test.py b/tests/commands/install_uninstall_test.py index 703ba03b7..ae668ac9f 100644 --- a/tests/commands/install_uninstall_test.py +++ b/tests/commands/install_uninstall_test.py @@ -7,6 +7,7 @@ import pre_commit.constants as C from pre_commit import git +from pre_commit.commands.install_uninstall import _hook_types from pre_commit.commands.install_uninstall import CURRENT_HASH from pre_commit.commands.install_uninstall import install from pre_commit.commands.install_uninstall import install_hooks @@ -27,6 +28,36 @@ from testing.util import git_commit +def test_hook_types_explicitly_listed(): + assert _hook_types(os.devnull, ['pre-push']) == ['pre-push'] + + +def test_hook_types_default_value_when_not_specified(): + assert _hook_types(os.devnull, None) == ['pre-commit'] + + +def test_hook_types_configured(tmpdir): + cfg = tmpdir.join('t.cfg') + cfg.write('default_install_hook_types: [pre-push]\nrepos: []\n') + + assert _hook_types(str(cfg), None) == ['pre-push'] + + +def test_hook_types_configured_nonsense(tmpdir): + cfg = tmpdir.join('t.cfg') + cfg.write('default_install_hook_types: []\nrepos: []\n') + + # hopefully the user doesn't do this, but the code allows it! + assert _hook_types(str(cfg), None) == [] + + +def test_hook_types_configuration_has_error(tmpdir): + cfg = tmpdir.join('t.cfg') + cfg.write('[') + + assert _hook_types(str(cfg), None) == ['pre-commit'] + + def test_is_not_script(): assert is_our_script('setup.py') is False @@ -61,7 +92,7 @@ def test_install_multiple_hooks_at_once(in_git_dir, store): install(C.CONFIG_FILE, store, hook_types=['pre-commit', 'pre-push']) assert in_git_dir.join('.git/hooks/pre-commit').exists() assert in_git_dir.join('.git/hooks/pre-push').exists() - uninstall(hook_types=['pre-commit', 'pre-push']) + uninstall(C.CONFIG_FILE, hook_types=['pre-commit', 'pre-push']) assert not in_git_dir.join('.git/hooks/pre-commit').exists() assert not in_git_dir.join('.git/hooks/pre-push').exists() @@ -79,14 +110,14 @@ def test_install_hooks_dead_symlink(in_git_dir, store): def test_uninstall_does_not_blow_up_when_not_there(in_git_dir): - assert uninstall(hook_types=['pre-commit']) == 0 + assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0 def test_uninstall(in_git_dir, store): assert not in_git_dir.join('.git/hooks/pre-commit').exists() install(C.CONFIG_FILE, store, hook_types=['pre-commit']) assert in_git_dir.join('.git/hooks/pre-commit').exists() - uninstall(hook_types=['pre-commit']) + uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) assert not in_git_dir.join('.git/hooks/pre-commit').exists() @@ -416,7 +447,7 @@ def test_uninstall_restores_legacy_hooks(tempdir_factory, store): # Now install and uninstall pre-commit assert install(C.CONFIG_FILE, store, hook_types=['pre-commit']) == 0 - assert uninstall(hook_types=['pre-commit']) == 0 + assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0 # Make sure we installed the "old" hook correctly ret, output = _get_commit_output(tempdir_factory, touch_file='baz') @@ -451,7 +482,7 @@ def test_uninstall_doesnt_remove_not_our_hooks(in_git_dir): pre_commit.write('#!/usr/bin/env bash\necho 1\n') make_executable(pre_commit.strpath) - assert uninstall(hook_types=['pre-commit']) == 0 + assert uninstall(C.CONFIG_FILE, hook_types=['pre-commit']) == 0 assert pre_commit.exists() @@ -1007,3 +1038,16 @@ def test_install_temporarily_allow_mising_config(tempdir_factory, store): 'Skipping `pre-commit`.' ) assert expected in output + + +def test_install_uninstall_default_hook_types(in_git_dir, store): + cfg_src = 'default_install_hook_types: [pre-commit, pre-push]\nrepos: []\n' + in_git_dir.join(C.CONFIG_FILE).write(cfg_src) + + assert not install(C.CONFIG_FILE, store, hook_types=None) + assert os.access(in_git_dir.join('.git/hooks/pre-commit').strpath, os.X_OK) + assert os.access(in_git_dir.join('.git/hooks/pre-push').strpath, os.X_OK) + + assert not uninstall(C.CONFIG_FILE, hook_types=None) + assert not in_git_dir.join('.git/hooks/pre-commit').exists() + assert not in_git_dir.join('.git/hooks/pre-push').exists() diff --git a/tests/main_test.py b/tests/main_test.py index 64b26a00c..a645300ab 100644 --- a/tests/main_test.py +++ b/tests/main_test.py @@ -14,20 +14,6 @@ from testing.util import cwd -@pytest.mark.parametrize( - ('argv', 'expected'), - ( - ((), ['f']), - (('--f', 'x'), ['x']), - (('--f', 'x', '--f', 'y'), ['x', 'y']), - ), -) -def test_append_replace_default(argv, expected): - parser = argparse.ArgumentParser() - parser.add_argument('--f', action=main.AppendReplaceDefault, default=['f']) - assert parser.parse_args(argv).f == expected - - def _args(**kwargs): kwargs.setdefault('command', 'help') kwargs.setdefault('config', C.CONFIG_FILE) @@ -172,7 +158,7 @@ def test_init_templatedir(mock_store_dir): assert patch.call_count == 1 assert 'tdir' in patch.call_args[0] - assert patch.call_args[1]['hook_types'] == ['pre-commit'] + assert patch.call_args[1]['hook_types'] is None assert patch.call_args[1]['skip_on_missing_config'] is True