Don't display hook skip message unless hook would run

Previously, we would always display "Skipping <hook name>" when skipping
via the `SKIP` environment variable, even if that hook would never have
run (e.g. due to no applicable files).

This would get annoying when specifying `SKIP=all` as it would display a
large list of hooks that were "skipped"--most of which would never
actually have run.

Change the behavior to only display the message if the hook would have
actually run. In the process, we were able to simplify some boolean
expressions in `Hook#run?` and `HookRunner#run_hooks` as we realized
they didn't need to be as complicated.

Change-Id: I9f503b0d58bd6e3dd5c317f122c8694f3fb05072
Reviewed-on: http://gerrit.causes.com/45273
Tested-by: jenkins <jenkins@brigade.com>
Reviewed-by: Shane da Silva <shane.dasilva@brigade.com>

Author: sds

CHANGELOG.md

@@ -5,6 +5,8 @@
 * Add `--run` flag which runs all configured pre-commit hooks against the
   entire repository
 * Fix installer to work with Overcommit hooks created via `GIT_TEMPLATE_DIR`
+* Fix hook runner to not display skip message unless hook would have actually
+  run
 
 ## 0.19.0
 * Add `--no-pngout` flag for `image_optim` command on `:fail` message

lib/overcommit/hook/base.rb

@@ -100,7 +100,6 @@ def skip?
 
     def run?
       enabled? &&
-        (!skip? || required?) &&
         !(@config['requires_files'] && applicable_files.empty?)
     end
 

lib/overcommit/hook_runner.rb

@@ -36,7 +36,7 @@ def run
     attr_reader :log
 
     def run_hooks
-      if @hooks.any? { |hook| hook.run? || hook.skip? }
+      if @hooks.any?(&:enabled?)
         @printer.start_run
 
         interrupted = false
@@ -109,7 +109,8 @@ def should_skip?(hook)
         if hook.required?
           @printer.required_hook_not_skipped(hook)
         else
-          @printer.hook_skipped(hook)
+          # Tell user if hook was skipped only if it actually would have run
+          @printer.hook_skipped(hook) if hook.run?
           return true
         end
       end