Merge pull request #561 from pre-commit/upgrade_add_trailing_comma

Upgrade add-trailing-comma to 0.4.0
This commit is contained in:
Anthony Sottile 2017-07-15 14:17:53 -07:00 committed by GitHub
commit 412bb7c718
9 changed files with 216 additions and 199 deletions

View file

@ -22,6 +22,6 @@
- id: reorder-python-imports - id: reorder-python-imports
language_version: python2.7 language_version: python2.7
- repo: https://github.com/asottile/add-trailing-comma - repo: https://github.com/asottile/add-trailing-comma
sha: v0.3.0 sha: v0.4.0
hooks: hooks:
- id: add-trailing-comma - id: add-trailing-comma

View file

@ -46,29 +46,6 @@ variable `slowtests=false`.
With the environment activated simply run `pre-commit install`. With the environment activated simply run `pre-commit install`.
## Style
This repository follows pep8 (and enforces it with flake8). There are a few
nitpicky things I also like that I'll outline below.
### Multi-line method invocation
Multiple line method invocation should look as follows
```python
function_call(
argument,
argument,
argument,
)
```
Some notable features:
- The initial parenthesis is at the end of the line
- Parameters are indented one indentation level further than the function name
- The last parameter contains a trailing comma (This helps make `git blame`
more accurate and reduces merge conflicts when adding / removing parameters).
## Documentation ## Documentation
Documentation is hosted at http://pre-commit.com Documentation is hosted at http://pre-commit.com

View file

@ -24,11 +24,13 @@ def get_env_patch(venv, language_version): # pragma: windows no cover
('GEM_HOME', os.path.join(venv, 'gems')), ('GEM_HOME', os.path.join(venv, 'gems')),
('RBENV_ROOT', venv), ('RBENV_ROOT', venv),
('BUNDLE_IGNORE_CONFIG', '1'), ('BUNDLE_IGNORE_CONFIG', '1'),
('PATH', ( (
'PATH', (
os.path.join(venv, 'gems', 'bin'), os.pathsep, os.path.join(venv, 'gems', 'bin'), os.pathsep,
os.path.join(venv, 'shims'), os.pathsep, os.path.join(venv, 'shims'), os.pathsep,
os.path.join(venv, 'bin'), os.pathsep, Var('PATH'), os.path.join(venv, 'bin'), os.pathsep, Var('PATH'),
)), ),
),
) )
if language_version != 'default': if language_version != 'default':
patches += (('RBENV_VERSION', language_version),) patches += (('RBENV_VERSION', language_version),)

View file

@ -69,13 +69,15 @@ def modify_config(path='.', commit=True):
def config_with_local_hooks(): def config_with_local_hooks():
return OrderedDict(( return OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', [OrderedDict(( (
'hooks', [OrderedDict((
('id', 'do_not_commit'), ('id', 'do_not_commit'),
('name', 'Block if "DO NOT COMMIT" is found'), ('name', 'Block if "DO NOT COMMIT" is found'),
('entry', 'DO NOT COMMIT'), ('entry', 'DO NOT COMMIT'),
('language', 'pcre'), ('language', 'pcre'),
('files', '^(.*)$'), ('files', '^(.*)$'),
))]), ))],
),
)) ))

View file

@ -56,7 +56,8 @@ def test_validate_config_main(args, expected_output):
assert validate_config_main(args) == expected_output assert validate_config_main(args) == expected_output
@pytest.mark.parametrize(('config_obj', 'expected'), ( @pytest.mark.parametrize(
('config_obj', 'expected'), (
([], False), ([], False),
( (
[{ [{
@ -96,13 +97,15 @@ def test_validate_config_main(args, expected_output):
}], }],
False, False,
), ),
)) ),
)
def test_config_valid(config_obj, expected): def test_config_valid(config_obj, expected):
ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA) ret = is_valid_according_to_schema(config_obj, CONFIG_SCHEMA)
assert ret is expected assert ret is expected
@pytest.mark.parametrize('config_obj', ( @pytest.mark.parametrize(
'config_obj', (
[{ [{
'repo': 'local', 'repo': 'local',
'sha': 'foo', 'sha': 'foo',
@ -114,13 +117,15 @@ def test_config_valid(config_obj, expected):
'files': '^(.*)$', 'files': '^(.*)$',
}], }],
}], }],
)) ),
)
def test_config_with_local_hooks_definition_fails(config_obj): def test_config_with_local_hooks_definition_fails(config_obj):
with pytest.raises(schema.ValidationError): with pytest.raises(schema.ValidationError):
schema.validate(config_obj, CONFIG_SCHEMA) schema.validate(config_obj, CONFIG_SCHEMA)
@pytest.mark.parametrize('config_obj', ( @pytest.mark.parametrize(
'config_obj', (
[{ [{
'repo': 'local', 'repo': 'local',
'hooks': [{ 'hooks': [{
@ -143,7 +148,8 @@ def test_config_with_local_hooks_definition_fails(config_obj):
'args': ['hello', 'world'], 'args': ['hello', 'world'],
}] }]
}], }],
)) ),
)
def test_config_with_local_hooks_definition_passes(config_obj): def test_config_with_local_hooks_definition_passes(config_obj):
schema.validate(config_obj, CONFIG_SCHEMA) schema.validate(config_obj, CONFIG_SCHEMA)

View file

@ -11,10 +11,12 @@ from pre_commit.color import InvalidColorSetting
from pre_commit.color import use_color from pre_commit.color import use_color
@pytest.mark.parametrize(('in_text', 'in_color', 'in_use_color', 'expected'), ( @pytest.mark.parametrize(
('in_text', 'in_color', 'in_use_color', 'expected'), (
('foo', GREEN, True, '{}foo\033[0m'.format(GREEN)), ('foo', GREEN, True, '{}foo\033[0m'.format(GREEN)),
('foo', GREEN, False, 'foo'), ('foo', GREEN, False, 'foo'),
)) ),
)
def test_format_color(in_text, in_color, in_use_color, expected): def test_format_color(in_text, in_color, in_use_color, expected):
ret = format_color(in_text, in_color, in_use_color) ret = format_color(in_text, in_color, in_use_color)
assert ret == expected assert ret == expected

View file

@ -86,8 +86,10 @@ def _do_run(cap_out, repo, args, environ={}, config_file=C.CONFIG_FILE):
return ret, printed return ret, printed
def _test_run(cap_out, repo, opts, expected_outputs, expected_ret, stage, def _test_run(
config_file=C.CONFIG_FILE): cap_out, repo, opts, expected_outputs, expected_ret, stage,
config_file=C.CONFIG_FILE,
):
if stage: if stage:
stage_a_file() stage_a_file()
args = _get_opts(**opts) args = _get_opts(**opts)
@ -571,18 +573,24 @@ def test_lots_of_files(mock_out_store_directory, tempdir_factory):
@pytest.mark.parametrize( @pytest.mark.parametrize(
('hook_stage', 'stage_for_first_hook', 'stage_for_second_hook', (
'expected_output'), 'hook_stage', 'stage_for_first_hook', 'stage_for_second_hook',
'expected_output',
),
( (
('push', ['commit'], ['commit'], [b'', b'']), ('push', ['commit'], ['commit'], [b'', b'']),
('push', ['commit', 'push'], ['commit', 'push'], (
[b'hook 1', b'hook 2']), 'push', ['commit', 'push'], ['commit', 'push'],
[b'hook 1', b'hook 2'],
),
('push', [], [], [b'hook 1', b'hook 2']), ('push', [], [], [b'hook 1', b'hook 2']),
('push', [], ['commit'], [b'hook 1', b'']), ('push', [], ['commit'], [b'hook 1', b'']),
('push', ['push'], ['commit'], [b'hook 1', b'']), ('push', ['push'], ['commit'], [b'hook 1', b'']),
('push', ['commit'], ['push'], [b'', b'hook 2']), ('push', ['commit'], ['push'], [b'', b'hook 2']),
('commit', ['commit', 'push'], ['commit', 'push'], (
[b'hook 1', b'hook 2']), 'commit', ['commit', 'push'], ['commit', 'push'],
[b'hook 1', b'hook 2'],
),
('commit', ['commit'], ['commit'], [b'hook 1', b'hook 2']), ('commit', ['commit'], ['commit'], [b'hook 1', b'hook 2']),
('commit', [], [], [b'hook 1', b'hook 2']), ('commit', [], [], [b'hook 1', b'hook 2']),
('commit', [], ['commit'], [b'hook 1', b'hook 2']), ('commit', [], ['commit'], [b'hook 1', b'hook 2']),
@ -600,7 +608,9 @@ def test_local_hook_for_stages(
): ):
config = OrderedDict(( config = OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', (OrderedDict(( (
'hooks', (
OrderedDict((
('id', 'flake8'), ('id', 'flake8'),
('name', 'hook 1'), ('name', 'hook 1'),
('entry', 'python -m flake8.__main__'), ('entry', 'python -m flake8.__main__'),
@ -614,7 +624,9 @@ def test_local_hook_for_stages(
('language', 'pcre'), ('language', 'pcre'),
('files', '^(.*)$'), ('files', '^(.*)$'),
('stages', stage_for_second_hook), ('stages', stage_for_second_hook),
)))), )),
),
),
)) ))
add_config_to_repo(repo_with_passing_hook, config) add_config_to_repo(repo_with_passing_hook, config)
@ -637,7 +649,9 @@ def test_local_hook_passes(
): ):
config = OrderedDict(( config = OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', (OrderedDict(( (
'hooks', (
OrderedDict((
('id', 'flake8'), ('id', 'flake8'),
('name', 'flake8'), ('name', 'flake8'),
('entry', 'python -m flake8.__main__'), ('entry', 'python -m flake8.__main__'),
@ -649,7 +663,9 @@ def test_local_hook_passes(
('entry', 'DO NOT COMMIT'), ('entry', 'DO NOT COMMIT'),
('language', 'pcre'), ('language', 'pcre'),
('files', '^(.*)$'), ('files', '^(.*)$'),
)))), )),
),
),
)) ))
add_config_to_repo(repo_with_passing_hook, config) add_config_to_repo(repo_with_passing_hook, config)
@ -672,13 +688,15 @@ def test_local_hook_fails(
): ):
config = OrderedDict(( config = OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', [OrderedDict(( (
'hooks', [OrderedDict((
('id', 'no-todo'), ('id', 'no-todo'),
('name', 'No TODO'), ('name', 'No TODO'),
('entry', 'sh -c "! grep -iI todo $@" --'), ('entry', 'sh -c "! grep -iI todo $@" --'),
('language', 'system'), ('language', 'system'),
('files', ''), ('files', ''),
))]), ))],
),
)) ))
add_config_to_repo(repo_with_passing_hook, config) add_config_to_repo(repo_with_passing_hook, config)

View file

@ -54,13 +54,15 @@ def makedirs_mock():
return mock.Mock(spec=os.makedirs) return mock.Mock(spec=os.makedirs)
@pytest.mark.parametrize(('input', 'expected_prefix'), ( @pytest.mark.parametrize(
('input', 'expected_prefix'), (
norm_slash(('.', './')), norm_slash(('.', './')),
norm_slash(('foo', 'foo/')), norm_slash(('foo', 'foo/')),
norm_slash(('bar/', 'bar/')), norm_slash(('bar/', 'bar/')),
norm_slash(('foo/bar', 'foo/bar/')), norm_slash(('foo/bar', 'foo/bar/')),
norm_slash(('foo/bar/', 'foo/bar/')), norm_slash(('foo/bar/', 'foo/bar/')),
)) ),
)
def test_init_normalizes_path_endings(input, expected_prefix): def test_init_normalizes_path_endings(input, expected_prefix):
input = input.replace('/', os.sep) input = input.replace('/', os.sep)
expected_prefix = expected_prefix.replace('/', os.sep) expected_prefix = expected_prefix.replace('/', os.sep)

View file

@ -57,7 +57,9 @@ def test_repositories(tempdir_factory, mock_out_store_directory):
def test_local_hooks(tempdir_factory, mock_out_store_directory): def test_local_hooks(tempdir_factory, mock_out_store_directory):
config = OrderedDict(( config = OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', (OrderedDict(( (
'hooks', (
OrderedDict((
('id', 'arg-per-line'), ('id', 'arg-per-line'),
('name', 'Args per line hook'), ('name', 'Args per line hook'),
('entry', 'bin/hook.sh'), ('entry', 'bin/hook.sh'),
@ -70,7 +72,9 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory):
('entry', 'DO NOT COMMIT'), ('entry', 'DO NOT COMMIT'),
('language', 'pcre'), ('language', 'pcre'),
('files', '^(.*)$'), ('files', '^(.*)$'),
)))), )),
),
),
)) ))
git_path = git_dir(tempdir_factory) git_path = git_dir(tempdir_factory)
add_config_to_repo(git_path, config) add_config_to_repo(git_path, config)
@ -82,7 +86,9 @@ def test_local_hooks(tempdir_factory, mock_out_store_directory):
def test_local_hooks_alt_config(tempdir_factory, mock_out_store_directory): def test_local_hooks_alt_config(tempdir_factory, mock_out_store_directory):
config = OrderedDict(( config = OrderedDict((
('repo', 'local'), ('repo', 'local'),
('hooks', (OrderedDict(( (
'hooks', (
OrderedDict((
('id', 'arg-per-line'), ('id', 'arg-per-line'),
('name', 'Args per line hook'), ('name', 'Args per line hook'),
('entry', 'bin/hook.sh'), ('entry', 'bin/hook.sh'),
@ -101,7 +107,9 @@ def test_local_hooks_alt_config(tempdir_factory, mock_out_store_directory):
('entry', 'DO NOT COMMIT'), ('entry', 'DO NOT COMMIT'),
('language', 'pcre'), ('language', 'pcre'),
('files', '^(.*)$'), ('files', '^(.*)$'),
)))), )),
),
),
)) ))
git_path = git_dir(tempdir_factory) git_path = git_dir(tempdir_factory)
alt_config_file = 'alternate_config.yaml' alt_config_file = 'alternate_config.yaml'