From 374ef758358ce15b4b4d2706d11ab96741fd3951 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 18:36:14 -0700 Subject: [PATCH 1/4] Convert LoggingHandler to use sys.stdout.write --- pre_commit/logging_handler.py | 11 +++++------ tests/logging_handler_test.py | 4 ++-- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/pre_commit/logging_handler.py b/pre_commit/logging_handler.py index 3226b8c0..87102ca8 100644 --- a/pre_commit/logging_handler.py +++ b/pre_commit/logging_handler.py @@ -1,6 +1,5 @@ -from __future__ import print_function - import logging +import sys from pre_commit import color @@ -14,14 +13,14 @@ LOG_LEVEL_COLORS = { class LoggingHandler(logging.Handler): - def __init__(self, use_color, print_fn=print): + def __init__(self, use_color, write=sys.stdout.write): logging.Handler.__init__(self) self.use_color = use_color - self.__print_fn = print_fn + self.__write = write def emit(self, record): - self.__print_fn( - u'{0}{1}'.format( + self.__write( + u'{0}{1}\n'.format( color.format_color( '[{0}]'.format(record.levelname), LOG_LEVEL_COLORS[record.levelname], diff --git a/tests/logging_handler_test.py b/tests/logging_handler_test.py index 6e8d1835..63d9d21d 100644 --- a/tests/logging_handler_test.py +++ b/tests/logging_handler_test.py @@ -19,7 +19,7 @@ def test_logging_handler_color(): handler = LoggingHandler(True, print_mock) handler.emit(FakeLogRecord('hi', 'WARNING', 30)) print_mock.assert_called_once_with( - color.YELLOW + '[WARNING]' + color.NORMAL + ' hi', + color.YELLOW + '[WARNING]' + color.NORMAL + ' hi\n', ) @@ -28,5 +28,5 @@ def test_logging_handler_no_color(): handler = LoggingHandler(False, print_mock) handler.emit(FakeLogRecord('hi', 'WARNING', 30)) print_mock.assert_called_once_with( - '[WARNING] hi', + '[WARNING] hi\n', ) From 5386b0eea0ed4ab58aab40db48296d82727a9f30 Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 18:43:43 -0700 Subject: [PATCH 2/4] Pass write down from commands to logging handler. --- pre_commit/commands.py | 2 +- pre_commit/logging_handler.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 909f8de4..60fc2c52 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -230,7 +230,7 @@ def _run_hook(runner, hook_id, args, write): def run(runner, args, write=sys.stdout.write): # Set up our logging handler - logger.addHandler(LoggingHandler(args.color)) + logger.addHandler(LoggingHandler(args.color, write=write)) logger.setLevel(logging.INFO) with staged_files_only(runner.cmd_runner): diff --git a/pre_commit/logging_handler.py b/pre_commit/logging_handler.py index 87102ca8..8fed2df3 100644 --- a/pre_commit/logging_handler.py +++ b/pre_commit/logging_handler.py @@ -25,7 +25,7 @@ class LoggingHandler(logging.Handler): '[{0}]'.format(record.levelname), LOG_LEVEL_COLORS[record.levelname], self.use_color, - ) + ' ' if record.levelno >= logging.WARNING else '', + ) + ' ', record.getMessage(), ) ) From 8a8b2241a6719508f7cc25b4caec4daf2f92e62f Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 20:00:10 -0700 Subject: [PATCH 3/4] Add --no-stash option. --- pre_commit/commands.py | 8 ++++++- pre_commit/run.py | 4 ++++ pre_commit/util.py | 6 ++++++ tests/commands_test.py | 47 +++++++++++++++++++++++++++++++++--------- 4 files changed, 54 insertions(+), 11 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 60fc2c52..832bc664 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -21,6 +21,7 @@ from pre_commit.jsonschema_extensions import remove_defaults from pre_commit.logging_handler import LoggingHandler from pre_commit.repository import Repository from pre_commit.staged_files_only import staged_files_only +from pre_commit.util import noop_context logger = logging.getLogger('pre_commit') @@ -233,7 +234,12 @@ def run(runner, args, write=sys.stdout.write): logger.addHandler(LoggingHandler(args.color, write=write)) logger.setLevel(logging.INFO) - with staged_files_only(runner.cmd_runner): + if args.no_stash: + ctx = noop_context() + else: + ctx = staged_files_only(runner.cmd_runner) + + with ctx: if args.hook: return _run_hook(runner, args.hook, args, write=write) else: diff --git a/pre_commit/run.py b/pre_commit/run.py index 2bcb5d86..d4f339d8 100644 --- a/pre_commit/run.py +++ b/pre_commit/run.py @@ -31,6 +31,10 @@ def run(argv): '--color', default='auto', type=color.use_color, help='Whether to use color in output. Defaults to `auto`', ) + run_parser.add_argument( + '--no-stash', default=False, action='store_true', + help='Use this option to prevent auto stashing of unstaged files.', + ) run_parser.add_argument('--verbose', '-v', action='store_true', default=False) help = subparsers.add_parser('help', help='Show help for a specific command.') diff --git a/pre_commit/util.py b/pre_commit/util.py index fb4a8764..5884e4dd 100644 --- a/pre_commit/util.py +++ b/pre_commit/util.py @@ -62,3 +62,9 @@ def clean_path_on_failure(path): if os.path.exists(path): shutil.rmtree(path) raise + + +# TODO: asottile.contextlib this with a forward port of nested +@contextlib.contextmanager +def noop_context(): + yield diff --git a/tests/commands_test.py b/tests/commands_test.py index 1d1cb1ab..d4fd73a6 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -192,20 +192,30 @@ def get_write_mock_output(write_mock): return ''.join(call[0][0] for call in write_mock.call_args_list) +def _get_opts(all_files=False, color=False, verbose=False, hook=None, no_stash=False): + return auto_namedtuple( + all_files=all_files, + color=color, + verbose=verbose, + hook=hook, + no_stash=no_stash, + ) + + +def _do_run(repo, args): + runner = Runner(repo) + write_mock = mock.Mock() + ret = commands.run(runner, args, write=write_mock) + printed = get_write_mock_output(write_mock) + return ret, printed + + def _test_run(repo, options, expected_outputs, expected_ret, stage): if stage: stage_a_file() - runner = Runner(repo) - args = auto_namedtuple( - **dict( - dict(all_files=False, color=False, verbose=False, hook=None), - **options - ) - ) - write_mock = mock.Mock() - ret = commands.run(runner, args, write=write_mock) + args = _get_opts(**options) + ret, printed = _do_run(repo, args) assert ret == expected_ret - printed = get_write_mock_output(write_mock) for expected_output_part in expected_outputs: assert expected_output_part in printed @@ -240,3 +250,20 @@ def test_run_all_hooks_failing(repo_with_failing_hook): ) def test_run(repo_with_passing_hook, options, outputs, expected_ret, stage): _test_run(repo_with_passing_hook, options, outputs, expected_ret, stage) + + +@pytest.mark.parametrize('no_stash', (True, False)) +def test_no_stash(repo_with_passing_hook, no_stash): + stage_a_file() + # Make unstaged changes + with open('foo.py', 'w') as foo_file: + foo_file.write('import os\n') + + args = _get_opts(no_stash=no_stash) + ret, printed = _do_run(repo_with_passing_hook, args) + assert ret == 0 + warning_msg = '[WARNING] Unstaged files detected.' + if no_stash: + assert warning_msg not in printed + else: + assert warning_msg in printed From 3e2b1862ad9c2035fd623171c6bfbae32c9aaaad Mon Sep 17 00:00:00 2001 From: Anthony Sottile Date: Sun, 13 Apr 2014 20:04:58 -0700 Subject: [PATCH 4/4] Don't stash with --all-files. Closes #68. --- pre_commit/commands.py | 2 +- pre_commit/run.py | 2 +- tests/commands_test.py | 20 ++++++++++++++------ 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/pre_commit/commands.py b/pre_commit/commands.py index 832bc664..ac9d4487 100644 --- a/pre_commit/commands.py +++ b/pre_commit/commands.py @@ -234,7 +234,7 @@ def run(runner, args, write=sys.stdout.write): logger.addHandler(LoggingHandler(args.color, write=write)) logger.setLevel(logging.INFO) - if args.no_stash: + if args.no_stash or args.all_files: ctx = noop_context() else: ctx = staged_files_only(runner.cmd_runner) diff --git a/pre_commit/run.py b/pre_commit/run.py index d4f339d8..706d68dd 100644 --- a/pre_commit/run.py +++ b/pre_commit/run.py @@ -25,7 +25,7 @@ def run(argv): run_parser.add_argument('hook', nargs='?', help='A single hook-id to run') run_parser.add_argument( '--all-files', '-a', action='store_true', default=False, - help='Run on all the files in the repo.', + help='Run on all the files in the repo. Implies --no-stash.', ) run_parser.add_argument( '--color', default='auto', type=color.use_color, diff --git a/tests/commands_test.py b/tests/commands_test.py index d4fd73a6..ae4e3b4c 100644 --- a/tests/commands_test.py +++ b/tests/commands_test.py @@ -252,18 +252,26 @@ def test_run(repo_with_passing_hook, options, outputs, expected_ret, stage): _test_run(repo_with_passing_hook, options, outputs, expected_ret, stage) -@pytest.mark.parametrize('no_stash', (True, False)) -def test_no_stash(repo_with_passing_hook, no_stash): +@pytest.mark.parametrize( + ('no_stash', 'all_files', 'expect_stash'), + ( + (True, True, False), + (True, False, False), + (False, True, False), + (False, False, True), + ), +) +def test_no_stash(repo_with_passing_hook, no_stash, all_files, expect_stash): stage_a_file() # Make unstaged changes with open('foo.py', 'w') as foo_file: foo_file.write('import os\n') - args = _get_opts(no_stash=no_stash) + args = _get_opts(no_stash=no_stash, all_files=all_files) ret, printed = _do_run(repo_with_passing_hook, args) assert ret == 0 warning_msg = '[WARNING] Unstaged files detected.' - if no_stash: - assert warning_msg not in printed - else: + if expect_stash: assert warning_msg in printed + else: + assert warning_msg not in printed