diff --git a/.gitignore b/.gitignore index 2bb7f841..7ea8f1f7 100644 --- a/.gitignore +++ b/.gitignore @@ -3,3 +3,5 @@ coverage/ pkg/ .bundle .idea +.history/ +.vscode/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c586f60..1711cb54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Overcommit Changelog +## 0.53.0 + +* Improve performance in `PhpCs` pre-commit hook +* Add `Pronto` pre-push hook +* Remove erroneous extra newline in replacement string for `ReplaceBranch` prepare-commit-msg hook +* Add note about potentially checking your stash when hook is interrupted +* Add support for skipping hooks based on command result using the `skip_if` option + ## 0.52.1 * Fix case where no standard input is provided to `pre-push` hooks diff --git a/README.md b/README.md index f1076df6..af718032 100644 --- a/README.md +++ b/README.md @@ -241,6 +241,7 @@ Option | Description `processors` | The number of processing units to reserve for this hook. This does not reserve CPUs, but indicates that out of the total number of possible concurrent hooks allowed by the global `concurrency` option, this hook requires the specified number. Thus in the typical case where `concurrency` is set to the number of available cores (default), and you have a hook that executes an application which itself creates 2 threads (or is otherwise scheduled on 2 cores), you can indicate that Overcommit should allocate 2 `processors` to the hook. Ideally this means your hooks won't put undue load on your available cores. `install_command` | Command the user can run to install the `required_executable` (or alternately the specified `required_libraries`). This is intended for documentation purposes, as Overcommit does not install software on your behalf since there are too many edge cases where such behavior would result in incorrectly configured installations (e.g. installing a Python package in the global package space instead of in a virtual environment). `skip_file_checkout` | Whether to skip this hook for file checkouts (e.g. `git checkout some-ref -- file`). Only applicable to `PostCheckout` hooks. +`skip_if` | Array of arguments to be executed to determine whether or not the hook should run. For example, setting this to a value of `['bash', '-c', '! which my-executable']` would allow you to skip running this hook if `my-executable` was not in the bin path. In addition to the built-in configuration options, each hook can expose its own unique configuration options. The `AuthorEmail` hook, for example, allows @@ -582,6 +583,7 @@ aborted. * [Brakeman](lib/overcommit/hook/pre_push/brakeman.rb) * [Minitest](lib/overcommit/hook/pre_push/minitest.rb) * [PhpUnit](lib/overcommit/hook/pre_push/php_unit.rb) +* [Pronto](lib/overcommit/hook/pre_push/pronto.rb) * [ProtectedBranches](lib/overcommit/hook/pre_push/protected_branches.rb) * [Pytest](lib/overcommit/hook/pre_push/pytest.rb) * [PythonNose](lib/overcommit/hook/pre_push/python_nose.rb) diff --git a/config/default.yml b/config/default.yml index 5a6887f4..74503e8e 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1297,6 +1297,13 @@ PrePush: flags: ['--bootstrap', 'vendor/autoload.php', 'tests'] install_command: 'composer require --dev phpunit/phpunit' + Pronto: + enabled: false + description: 'Analyzing with pronto' + required_executable: 'pronto' + install_command: 'gem install pronto' + flags: ['run', '--exit-code'] + ProtectedBranches: enabled: false description: 'Check for illegal pushes to protected branches' diff --git a/lib/overcommit/hook/base.rb b/lib/overcommit/hook/base.rb index 4f48d770..37696d08 100644 --- a/lib/overcommit/hook/base.rb +++ b/lib/overcommit/hook/base.rb @@ -84,7 +84,8 @@ def excluded? end def skip? - @config['skip'] + @config['skip'] || + (@config['skip_if'] ? execute(@config['skip_if']).success? : false) end def run? diff --git a/lib/overcommit/hook/pre_commit/php_cs.rb b/lib/overcommit/hook/pre_commit/php_cs.rb index 6204c8dd..8e837406 100644 --- a/lib/overcommit/hook/pre_commit/php_cs.rb +++ b/lib/overcommit/hook/pre_commit/php_cs.rb @@ -12,21 +12,14 @@ class PhpCs < Base def run messages = [] - applicable_files.each do |file| - result = execute(command, args: [file]) - if result.status - rows = result.stdout.split("\n") - - # Discard the csv header - rows.shift - - # Push each of the errors in the particular file into the array - rows.map do |row| - messages << row - end - end + result = execute(command, args: applicable_files) + if result.status + messages = result.stdout.split("\n") + # Discard the csv header + messages.shift end + return :fail if messages.empty? && !result.success? return :pass if messages.empty? parse_messages(messages) diff --git a/lib/overcommit/hook/pre_commit/pronto.rb b/lib/overcommit/hook/pre_commit/pronto.rb index da016e8f..664b4ef8 100644 --- a/lib/overcommit/hook/pre_commit/pronto.rb +++ b/lib/overcommit/hook/pre_commit/pronto.rb @@ -1,23 +1,12 @@ # frozen_string_literal: true +require 'overcommit/hook/shared/pronto' + module Overcommit::Hook::PreCommit # Runs `pronto` # # @see https://github.com/mmozuras/pronto class Pronto < Base - MESSAGE_TYPE_CATEGORIZER = lambda do |type| - type.include?('E') ? :error : :warning - end - - def run - result = execute(command) - return :pass if result.success? - - extract_messages( - result.stdout.split("\n"), - /^(?(?:\w:)?[^:]+):(?\d+) (?[^ ]+)/, - MESSAGE_TYPE_CATEGORIZER, - ) - end + include Overcommit::Hook::Shared::Pronto end end diff --git a/lib/overcommit/hook/pre_push/base.rb b/lib/overcommit/hook/pre_push/base.rb index 72c89d0c..35e2b3a8 100644 --- a/lib/overcommit/hook/pre_push/base.rb +++ b/lib/overcommit/hook/pre_push/base.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'forwardable' +require 'overcommit/utils/messages_utils' module Overcommit::Hook::PrePush # Functionality common to all pre-push hooks. @@ -17,6 +18,10 @@ def run? private + def extract_messages(*args) + Overcommit::Utils::MessagesUtils.extract_messages(*args) + end + def exclude_remotes @config['exclude_remotes'] || [] end diff --git a/lib/overcommit/hook/pre_push/pronto.rb b/lib/overcommit/hook/pre_push/pronto.rb new file mode 100644 index 00000000..93a0724e --- /dev/null +++ b/lib/overcommit/hook/pre_push/pronto.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +require 'overcommit/hook/shared/pronto' + +module Overcommit::Hook::PrePush + # Runs `pronto` + # + # @see https://github.com/mmozuras/pronto + class Pronto < Base + include Overcommit::Hook::Shared::Pronto + end +end diff --git a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb index ce490163..c64f4c22 100644 --- a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb +++ b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb @@ -17,7 +17,7 @@ def run Overcommit::Utils.log.debug("Writing #{commit_message_filename} with #{new_template}") modify_commit_message do |old_contents| - "#{new_template}\n#{old_contents}" + "#{new_template} #{old_contents}" end :pass diff --git a/lib/overcommit/hook/shared/pronto.rb b/lib/overcommit/hook/shared/pronto.rb new file mode 100644 index 00000000..b1d71442 --- /dev/null +++ b/lib/overcommit/hook/shared/pronto.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Overcommit::Hook::Shared + # Shared code used by all Pronto hooks. Runs pronto linter. + module Pronto + MESSAGE_TYPE_CATEGORIZER = lambda do |type| + type.include?('E') ? :error : :warning + end + + def run + result = execute(command) + return :pass if result.success? + + extract_messages( + result.stdout.split("\n"), + /^(?(?:\w:)?[^:]+):(?\d+) (?[^ ]+)/, + MESSAGE_TYPE_CATEGORIZER, + ) + end + end +end diff --git a/lib/overcommit/printer.rb b/lib/overcommit/printer.rb index aebb8326..02d0995d 100644 --- a/lib/overcommit/printer.rb +++ b/lib/overcommit/printer.rb @@ -51,6 +51,7 @@ def interrupt_triggered def run_interrupted log.newline log.warning '⚠ Hook run interrupted by user' + log.warning '⚠ If files appear modified/missing, check your stash to recover them' log.newline end diff --git a/lib/overcommit/version.rb b/lib/overcommit/version.rb index f6c31248..d4659a8b 100644 --- a/lib/overcommit/version.rb +++ b/lib/overcommit/version.rb @@ -2,5 +2,5 @@ # Defines the gem version. module Overcommit - VERSION = '0.52.1' + VERSION = '0.53.0' end diff --git a/spec/overcommit/hook/base_spec.rb b/spec/overcommit/hook/base_spec.rb index 8a0e5d46..0c8c7af7 100644 --- a/spec/overcommit/hook/base_spec.rb +++ b/spec/overcommit/hook/base_spec.rb @@ -122,4 +122,69 @@ end end end + + context '#skip?' do + before do + config.stub(:for_hook).and_return(hook_config) + end + + subject { hook.skip? } + + context 'with skip_if not specified' do + let(:hook_config) do + { 'skip' => skip } + end + + context 'with skip true' do + let(:skip) { true } + + it { subject.should == true } + end + + context 'with skip false' do + let(:skip) { false } + + it { subject.should == false } + end + end + + context 'with skip_if specified' do + before do + result = Overcommit::Subprocess::Result.new(success ? 0 : 1, '', '') + allow(Overcommit::Utils).to receive(:execute).and_return(result) + end + + let(:hook_config) do + { 'skip' => skip, 'skip_if' => ['bash', '-c', '! which my-executable'] } + end + + context 'with skip true and skip_if returning true' do + let(:skip) { true } + let(:success) { true } + + it { subject.should == true } + end + + context 'with skip true and skip_if returning false' do + let(:skip) { true } + let(:success) { false } + + it { subject.should == true } + end + + context 'with skip false and skip_if returning true' do + let(:skip) { false } + let(:success) { true } + + it { subject.should == true } + end + + context 'with skip false and skip_if returning false' do + let(:skip) { false } + let(:success) { false } + + it { subject.should == false } + end + end + end end diff --git a/spec/overcommit/hook/pre_push/pronto_spec.rb b/spec/overcommit/hook/pre_push/pronto_spec.rb new file mode 100644 index 00000000..2851a860 --- /dev/null +++ b/spec/overcommit/hook/pre_push/pronto_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Overcommit::Hook::PrePush::Pronto do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + before do + subject.stub(:applicable_files).and_return(%w[file1.rb file2.rb]) + end + + context 'when pronto exits successfully' do + before do + result = double('result') + result.stub(:success?).and_return(true) + subject.stub(:execute).and_return(result) + end + + it { should pass } + end + + context 'when pronto exits unsucessfully' do + let(:result) { double('result') } + + before do + result.stub(:success?).and_return(false) + subject.stub(:execute).and_return(result) + end + + context 'and it reports an error' do + before do + result.stub(:stdout).and_return([ + 'file2.rb:10 E: IDENTICAL code found in :iter.', + ].join("\n")) + end + + it { should fail_hook } + end + + context 'and it reports a warning' do + before do + result.stub(:stdout).and_return([ + 'file1.rb:12 W: Line is too long. [107/80]', + 'file2.rb:14 I: Prefer single-quoted strings' + ].join("\n")) + end + + it { should warn } + end + end +end