Skip to content

Commit 69e87d3

Browse files
sdsShane da Silva
authored and
Shane da Silva
committed
Refactor Overcommit framework
Initial refactor commit. Specs are currently broken, but I'm marking this off as a chunk of work for now, and will start working more atomically from now on. Change-Id: I8a120cb660046e3d7b23e302b0054de932df2bec Reviewed-on: http://gerrit.causes.com/35559 Reviewed-by: Shane da Silva <shane@causes.com> Tested-by: Shane da Silva <shane@causes.com>
1 parent 9dbeb63 commit 69e87d3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1013
-608
lines changed

TODO

+49
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
Handle case where hooks are installed but overcommit isn't
2+
Add user-friendly installation instructions (e.g. run `overcommit` in git
3+
repo and get asked to install if it is not installed)
4+
Symlink all scripts to single Overcommit file
5+
Ask to remove sample files
6+
Parallelize hook checks
7+
Stash non-indexed changes before running checks so real filename can be used
8+
(`git stash save --keep-index --include-untracked`)
9+
Allow hooks to depend on other hooks being run first? (`depends` option)
10+
Auto install hooks on updating overcommit during next hook run
11+
Auto update hook integration script when newer overcommit runs
12+
Allow GIT_TEMPLATE_DIR to be set with `export GIT_TEMPLATE_DIR=`overcommit --template-dir`
13+
Allow hooks to specify required executable (`executable` option, coupled with
14+
install instructions of some sort)
15+
Allow hooks to specify whether they only apply to lines the user modified
16+
Split out Whitespace check into individual checks
17+
Allow SKIP arguments to be case insensitive
18+
19+
20+
Allow features of hooks to be customized (stealth, required, etc.)
21+
22+
`overcommit pre-commit`
23+
`overcommit commit-msg <file-with-message>`
24+
`overcommit post-checkout <prev-HEAD> <new-HEAD> <file-checkout[0/1]>`
25+
26+
Core philosophies:
27+
- Git Hooks are a first-class artifact that don't vary from system to system
28+
(should be stored in source control)
29+
- Copying hooks between multiple repositories is annoying
30+
- Using `git config` as a method of storing hook configuration doesn't persist
31+
- Repository is the fundamental unit of configuration (no global configuration
32+
for multiple repositories stored elsewhere)
33+
- Overcommit is the homebrew of git hooks
34+
35+
36+
37+
WHAT's FIXED:
38+
- Renaming of core concepts
39+
- Control flow fixed to not `exit` in random places
40+
- No longer need temp files for staged files and the various hacks related to it
41+
(e.g. rubocop/scss-lint config loading)
42+
- Easier for developers to test new hooks (just run `bundle exec git commit`,
43+
etc.)
44+
- Removed the HookRegistry
45+
- Allow fine grained control over which checks apply to which files
46+
47+
48+
NICE TO HAVES:
49+
- report status sizes to width of terminal

bin/overcommit

+3-3
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
#!/usr/bin/env ruby
22

3-
$: << File.expand_path('../../lib', __FILE__)
4-
53
require 'overcommit'
64
require 'overcommit/cli'
75

8-
Overcommit::CLI.new(ARGV).tap do |cli|
6+
logger = Overcommit::Logger.new(STDOUT)
7+
8+
Overcommit::CLI.new(ARGV, logger).tap do |cli|
99
cli.parse_arguments
1010
cli.run
1111
end

config/default.yml

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
# Default configuration that all Overcommit configurations inherit from.
2+
#
3+
# This is an opinionated list of which hooks are valuable to run and what their
4+
# out-of-the-box settings should be.
5+
6+
# Where to store hook plugins specific to a repository. These are loaded in
7+
# addition to the default hooks Overcommit comes with. The location is relative
8+
# to the root of the repository.
9+
plugin_directory: '.githooks'
10+
11+
# Hooks that are run after `git commit` is executed, before the commit message
12+
# editor is displayed. These hooks are ideal for syntax checkers, linters, and
13+
# other checks that you want to run before you allow a commit object to be
14+
# created.
15+
pre_commit:
16+
ALL:
17+
requires_files: true
18+
required: false
19+
quiet: false
20+
21+
AuthorName:
22+
requires_files: false
23+
required: true
24+
silent: true
25+
26+
CoffeeLint:
27+
description: 'Analyzing with coffeelint'
28+
include: '**/*.coffee'
29+
30+
CssLint:
31+
description: 'Analyzing with csslint'
32+
include: '**/*.css'
33+
34+
HamlLint:
35+
description: 'Analyzing with haml-lint'
36+
include: '**/*.haml'
37+
38+
JsHint:
39+
description: 'Analyzing with JSHint'
40+
include: '**/*.js'
41+
42+
PythonFlake8:
43+
description: 'Analyzing with flake8'
44+
include: '**/*.py'
45+
46+
Rubocop:
47+
description: 'Analyzing with Rubocop'
48+
include:
49+
- '**/*.gemspec'
50+
- '**/*.rake'
51+
- '**/*.rb'
52+
- '**/Gemfile'
53+
- '**/Rakefile'
54+
55+
ScssLint:
56+
description: 'Analyzing with scss-lint'
57+
include: '**/*.scss'
58+
59+
Whitespace:
60+
description: 'Checking for invalid whitespace'
61+
62+
YamlSyntax:
63+
description: 'Checking YAML syntax'
64+
include: '**/*.yml'
65+
66+
# Hooks that are run after all pre-commit hooks have run, before the user's
67+
# commit message editor is opened.
68+
# These hooks are used to edit the commit message based on the proprosed changes
69+
# (for example, if you want to include additional metadata with the commit
70+
# message).
71+
prepare_commit_msg:
72+
73+
# Hooks that are run against every commit message after a user has written it.
74+
# These hooks are useful for enforcing policies on commit messages written for a
75+
# project.
76+
commit_msg:
77+
ALL:
78+
quiet: false
79+
80+
HardTabs:
81+
description: 'Checking for hard tabs'
82+
83+
RussianNovel:
84+
description: 'Checking length of commit message'
85+
quiet: true
86+
87+
SingleLineSubject:
88+
description: 'Checking subject line'
89+
90+
TextWidth:
91+
description: 'Checking text width'
92+
93+
TrailingPeriod:
94+
description: 'Checking for trailing periods in subject'

lib/overcommit.rb

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
require 'overcommit/constants'
2+
require 'overcommit/exceptions'
13
require 'overcommit/configuration'
2-
require 'overcommit/errors'
3-
require 'overcommit/git_hook'
4-
require 'overcommit/hook_specific_check'
4+
require 'overcommit/configuration_loader'
5+
require 'overcommit/hook/base'
6+
require 'overcommit/hook_runner'
7+
require 'overcommit/hook_runner/base'
58
require 'overcommit/installer'
69
require 'overcommit/logger'
710
require 'overcommit/reporter'
8-
require 'overcommit/staged_file'
911
require 'overcommit/utils'
1012
require 'overcommit/version'

lib/overcommit/cli.rb

+34-64
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,33 @@
11
require 'optparse'
22

33
module Overcommit
4+
# Responsible for parsing command-line options and executing appropriate
5+
# application logic based on those options.
46
class CLI
5-
attr_reader :options
6-
7-
def initialize(arguments = [])
7+
def initialize(arguments, logger)
88
@arguments = arguments
9+
@log = logger
910
@options = {}
1011
end
1112

1213
def parse_arguments
1314
@parser = OptionParser.new do |opts|
14-
opts.banner = "Usage: #{opts.program_name} [options] target"
15+
opts.banner = "Usage: #{opts.program_name} [options] [target-repo]"
1516

1617
opts.on_tail('-h', '--help', 'Show this message') do
1718
print_help opts.help
1819
end
1920

2021
opts.on_tail('-v', '--version', 'Show version') do
21-
log.log VERSION
22-
exit 0
23-
end
24-
25-
opts.on('-l', '--list-templates', 'List built-in templates') do
26-
Overcommit.config.templates.each_pair do |name, configuration|
27-
log.bold name
28-
log.log YAML.dump(configuration), ''
29-
end
30-
exit 0
31-
end
32-
33-
opts.on('-a', '--all', 'Include all git hooks') do
34-
@options[:template] = 'all'
22+
print_version(opts.program_name)
3523
end
3624

37-
opts.on('-t', '--template template',
38-
'Specify a template of hooks') do |template|
39-
@options[:template] = template
40-
end
41-
42-
opts.on('--uninstall', 'Remove overcommit from target') do
25+
opts.on('--uninstall', 'Remove Overcommit hooks from a repository') do
4326
@options[:uninstall] = true
4427
end
4528

46-
opts.on('-e', '--exclude hook_name,...', Array,
47-
'Exclude hooks from installation') do |excludes|
48-
# Transform from:
49-
#
50-
# pre_commit/test_history,commit_msg/change_id
51-
#
52-
# Into:
53-
#
54-
# {
55-
# 'commit_msg' => ['change_id'],
56-
# 'pre_commit' => ['test_history']
57-
# }
58-
@options[:excludes] = excludes.inject({}) do |memo, exclude|
59-
parts = exclude.split(%r{[:/.]})
60-
next memo unless parts.size == 2
61-
62-
memo[parts.first] ||= []
63-
memo[parts.first] << parts.last
64-
65-
memo
66-
end
29+
opts.on('--install', 'Install Overcommit hooks in a repository') do
30+
@options[:install] = true
6731
end
6832
end
6933

@@ -78,37 +42,43 @@ def parse_arguments
7842
end
7943

8044
def run
81-
if @options[:targets].nil? || @options[:targets].empty?
82-
log.warning 'You must supply at least one directory'
83-
log.log @parser.help
84-
exit 2
45+
if Array(@options[:targets]).empty?
46+
@options[:targets] = [Overcommit::Utils.repo_root].compact
47+
if @options[:targets].empty?
48+
log.warning 'You are not in a git repository.'
49+
log.log 'You must either specify the path to a repository or ' <<
50+
'change your current directory to a repository.'
51+
halt 64 # EX_USAGE
52+
end
8553
end
8654

8755
@options[:targets].each do |target|
8856
begin
89-
Installer.new(@options, target).run
90-
rescue NotAGitRepoError => e
91-
log.warning "Skipping #{target}: #{e}"
57+
Installer.new(@options, target, log).run
58+
rescue Overcommit::Exceptions::InvalidGitRepo => error
59+
log.warning "Skipping #{target}: #{error}"
9260
end
9361
end
94-
95-
log.success "#{@options[:uninstall] ? 'Removal' : 'Installation'} complete"
96-
97-
rescue ArgumentError => ex
98-
error "Installation failed: #{ex}"
99-
exit 3
10062
end
10163

10264
private
10365

104-
def log
105-
Logger.instance
106-
end
66+
attr_reader :log
10767

108-
def print_help(message, ex = nil)
109-
log.error ex.to_s + "\n" if ex
68+
def print_help(message, error = nil)
69+
log.error "#{error}\n" if error
11070
log.log message
111-
exit 0
71+
halt(error ? 64 : 0) # 64 = EX_USAGE
72+
end
73+
74+
def print_version(program_name)
75+
log.log "#{program_name} #{Overcommit::VERSION}"
76+
halt
77+
end
78+
79+
# Used for ease of stubbing in tests
80+
def halt(status = 0)
81+
exit status
11282
end
11383
end
11484
end

0 commit comments

Comments
 (0)