From 4f4767c9e07039b2885b8610fec99a1def96e845 Mon Sep 17 00:00:00 2001 From: Mandar Vaze Date: Fri, 31 May 2019 16:42:16 +0530 Subject: [PATCH 1/3] Pass color option to git diff (on failure) Fixes #1007 --- pre_commit/commands/run.py | 9 ++++++++- tests/commands/run_test.py | 7 +++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index d060e186..3c18dd56 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -224,7 +224,14 @@ def _run_hooks(config, hooks, args, environ): '`pre-commit install`.', ) output.write_line('All changes made by hooks:') - subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff')) + if args.color: + subprocess.call(( + 'git', '--no-pager', 'diff', '--no-ext-diff', + '--color={}'.format(args.color), + )) + else: + subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff')) + return retval diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index b465cae6..b4548f6f 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -187,6 +187,13 @@ def test_global_exclude(cap_out, store, tempdir_factory): }, b'All changes made by hooks:', ), + ( + { + 'show_diff_on_failure': True, + 'color': 'auto', + }, + b'All changes made by hooks:', + ), ( { 'show_diff_on_failure': True, From 64f0178b75c7c2ae79d8a7b3962481721856fd71 Mon Sep 17 00:00:00 2001 From: Mandar Vaze Date: Sat, 1 Jun 2019 07:40:20 +0530 Subject: [PATCH 2/3] Pass color option to git diff unconditionally --- pre_commit/commands/run.py | 11 ++++------- tests/commands/run_test.py | 7 +------ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index 3c18dd56..a58e2747 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -224,13 +224,10 @@ def _run_hooks(config, hooks, args, environ): '`pre-commit install`.', ) output.write_line('All changes made by hooks:') - if args.color: - subprocess.call(( - 'git', '--no-pager', 'diff', '--no-ext-diff', - '--color={}'.format(args.color), - )) - else: - subprocess.call(('git', '--no-pager', 'diff', '--no-ext-diff')) + subprocess.call(( + 'git', '--no-pager', 'diff', '--no-ext-diff', + '--color={}'.format(args.color), + )) return retval diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index b4548f6f..a6266fac 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -181,12 +181,6 @@ def test_global_exclude(cap_out, store, tempdir_factory): @pytest.mark.parametrize( ('args', 'expected_out'), [ - ( - { - 'show_diff_on_failure': True, - }, - b'All changes made by hooks:', - ), ( { 'show_diff_on_failure': True, @@ -198,6 +192,7 @@ def test_global_exclude(cap_out, store, tempdir_factory): { 'show_diff_on_failure': True, 'all_files': True, + 'color': 'auto', }, b'reproduce locally with: pre-commit run --all-files', ), From 3d7b374bef1e102b4abe3ecbb7a09a5507e17939 Mon Sep 17 00:00:00 2001 From: Mandar Vaze Date: Sat, 1 Jun 2019 17:33:27 +0530 Subject: [PATCH 3/3] Pass correct value to git color based on args.color --- pre_commit/commands/run.py | 4 +++- tests/commands/run_test.py | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/pre_commit/commands/run.py b/pre_commit/commands/run.py index a58e2747..33c0f10b 100644 --- a/pre_commit/commands/run.py +++ b/pre_commit/commands/run.py @@ -224,9 +224,11 @@ def _run_hooks(config, hooks, args, environ): '`pre-commit install`.', ) output.write_line('All changes made by hooks:') + # args.color is a boolean. + # See user_color function in color.py subprocess.call(( 'git', '--no-pager', 'diff', '--no-ext-diff', - '--color={}'.format(args.color), + '--color={}'.format({True: 'always', False: 'never'}[args.color]), )) return retval diff --git a/tests/commands/run_test.py b/tests/commands/run_test.py index a6266fac..fc2a973c 100644 --- a/tests/commands/run_test.py +++ b/tests/commands/run_test.py @@ -184,7 +184,13 @@ def test_global_exclude(cap_out, store, tempdir_factory): ( { 'show_diff_on_failure': True, - 'color': 'auto', + }, + b'All changes made by hooks:', + ), + ( + { + 'show_diff_on_failure': True, + 'color': True, }, b'All changes made by hooks:', ), @@ -192,7 +198,6 @@ def test_global_exclude(cap_out, store, tempdir_factory): { 'show_diff_on_failure': True, 'all_files': True, - 'color': 'auto', }, b'reproduce locally with: pre-commit run --all-files', ),