diff --git a/config/default.yml b/config/default.yml index 55fa74f6..27794ffe 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1237,9 +1237,14 @@ PrepareCommitMsg: ReplaceBranch: enabled: false description: 'Prepends the commit message with text based on the branch name' - branch_pattern: '\A.*\w+[-_](\d+).*\z' + branch_pattern: '\A(\d+)-(\w+).*\z' replacement_text: '[#\1]' - skipped_commit_types: ['message', 'template', 'merge', 'squash'] + skipped_commit_types: + - 'message' # if message is given via `-m`, `-F` + - 'template' # if `-t` is given or `commit.template` is set + - 'commit' # if `-c`, `-C`, or `--amend` is given + - 'merge' # if merging + - 'squash' # if squashing on_fail: warn # Hooks that run during `git push`, after remote refs have been updated but diff --git a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb index c64f4c22..5739d124 100644 --- a/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb +++ b/lib/overcommit/hook/prepare_commit_msg/replace_branch.rb @@ -2,11 +2,37 @@ module Overcommit::Hook::PrepareCommitMsg # Prepends the commit message with a message based on the branch name. + # + # === What to prepend + # # It's possible to reference parts of the branch name through the captures in # the `branch_pattern` regex. + # + # For instance, if your current branch is `123-topic` then this config + # + # branch_pattern: '(\d+)-(\w+)' + # replacement_text: '[#\1]' + # + # would make this hook prepend commit messages with `[#123]`. + # + # Similarly, a replacement text of `[\1][\2]` would result in `[123][topic]`. + # + # == When to run this hook + # + # You can configure this to run only for specific types of commits by setting + # the `skipped_commit_types`. The allowed types are + # + # - 'message' - if message is given via `-m`, `-F` + # - 'template' - if `-t` is given or `commit.template` is set + # - 'commit' - if `-c`, `-C`, or `--amend` is given + # - 'merge' - if merging + # - 'squash' - if squashing + # class ReplaceBranch < Base + DEFAULT_BRANCH_PATTERN = /\A(\d+)-(\w+).*\z/ + def run - return :pass if skipped_commit_types.include? commit_message_source + return :pass if skip? Overcommit::Utils.log.debug( "Checking if '#{Overcommit::GitRepo.current_branch}' matches #{branch_pattern}" @@ -17,21 +43,25 @@ def run Overcommit::Utils.log.debug("Writing #{commit_message_filename} with #{new_template}") modify_commit_message do |old_contents| - "#{new_template} #{old_contents}" + "#{new_template}#{old_contents}" end :pass end def new_template - @new_template ||= Overcommit::GitRepo.current_branch.gsub(branch_pattern, replacement_text) + @new_template ||= + begin + curr_branch = Overcommit::GitRepo.current_branch + curr_branch.gsub(branch_pattern, replacement_text).strip + end end def branch_pattern @branch_pattern ||= begin pattern = config['branch_pattern'] - Regexp.new((pattern || '').empty? ? '\A.*\w+[-_](\d+).*\z' : pattern) + Regexp.new((pattern || '').empty? ? DEFAULT_BRANCH_PATTERN : pattern) end end @@ -53,5 +83,9 @@ def replacement_text_config def skipped_commit_types @skipped_commit_types ||= config['skipped_commit_types'].map(&:to_sym) end + + def skip? + skipped_commit_types.include?(commit_message_source) + end end end diff --git a/spec/overcommit/hook/prepare_commit_msg/replace_branch_spec.rb b/spec/overcommit/hook/prepare_commit_msg/replace_branch_spec.rb index 71eb7c13..3844f77a 100644 --- a/spec/overcommit/hook/prepare_commit_msg/replace_branch_spec.rb +++ b/spec/overcommit/hook/prepare_commit_msg/replace_branch_spec.rb @@ -4,77 +4,127 @@ require 'overcommit/hook_context/prepare_commit_msg' describe Overcommit::Hook::PrepareCommitMsg::ReplaceBranch do - let(:config) { Overcommit::ConfigurationLoader.default_configuration } - let(:context) do - Overcommit::HookContext::PrepareCommitMsg.new( - config, [prepare_commit_message_file, 'commit'], StringIO.new + def checkout_branch(branch) + allow(Overcommit::GitRepo).to receive(:current_branch).and_return(branch) + end + + def new_config(opts = {}) + default = Overcommit::ConfigurationLoader.default_configuration + + return default if opts.empty? + + default.merge( + Overcommit::Configuration.new( + 'PrepareCommitMsg' => { + 'ReplaceBranch' => opts.merge('enabled' => true) + } + ) ) end - let(:prepare_commit_message_file) { 'prepare_commit_message_file.txt' } + def new_context(config, argv) + Overcommit::HookContext::PrepareCommitMsg.new(config, argv, StringIO.new) + end - subject(:hook) { described_class.new(config, context) } + def hook_for(config, context) + described_class.new(config, context) + end - before do - File.open(prepare_commit_message_file, 'w') - allow(Overcommit::Utils).to receive_message_chain(:log, :debug) - allow(Overcommit::GitRepo).to receive(:current_branch).and_return(new_head) + def add_file(name, contents) + File.open(name, 'w') { |f| f.puts contents } end - after do - File.delete(prepare_commit_message_file) unless ENV['APPVEYOR'] + def remove_file(name) + File.delete(name) end - let(:new_head) { 'userbeforeid-12345-branch-description' } + before { allow(Overcommit::Utils).to receive_message_chain(:log, :debug) } + + let(:config) { new_config } + let(:normal_context) { new_context(config, ['COMMIT_EDITMSG']) } + let(:message_context) { new_context(config, %w[COMMIT_EDITMSG message]) } + let(:commit_context) { new_context(config, %w[COMMIT_EDITMSG commit HEAD]) } + let(:merge_context) { new_context(config, %w[MERGE_MSG merge]) } + let(:squash_context) { new_context(config, %w[SQUASH_MSG squash]) } + let(:template_context) { new_context(config, ['template.txt', 'template']) } + subject(:hook) { hook_for(config, normal_context) } describe '#run' do + before { add_file 'COMMIT_EDITMSG', '' } + after { remove_file 'COMMIT_EDITMSG' } + context 'when the checked out branch matches the pattern' do + before { checkout_branch '123-topic' } + before { hook.run } + it { is_expected.to pass } - context 'template contents' do - subject(:template) { hook.new_template } + it 'prepends the replacement text' do + expect(File.read('COMMIT_EDITMSG')).to eq("[#123]\n") + end + end - before do - hook.stub(:replacement_text).and_return('Id is: \1') - end + context "when the checked out branch doesn't matches the pattern" do + before { checkout_branch 'topic-123' } + before { hook.run } - it { is_expected.to eq('Id is: 12345') } + context 'with the default `skipped_commit_types`' do + it { is_expected.to warn } end - end - context 'when the checked out branch does not match the pattern' do - let(:new_head) { "this shouldn't match the default pattern" } + context 'when merging, and `skipped_commit_types` includes `merge`' do + let(:config) { new_config('skipped_commit_types' => ['merge']) } + subject(:hook) { hook_for(config, merge_context) } - context 'when the commit type is in `skipped_commit_types`' do - let(:context) do - Overcommit::HookContext::PrepareCommitMsg.new( - config, [prepare_commit_message_file, 'template'], StringIO.new - ) - end + it { is_expected.to pass } + end + + context 'when merging, and `skipped_commit_types` includes `template`' do + let(:config) { new_config('skipped_commit_types' => ['template']) } + subject(:hook) { hook_for(config, template_context) } it { is_expected.to pass } end - context 'when the commit type is not in `skipped_commit_types`' do - it { is_expected.to warn } + context 'when merging, and `skipped_commit_types` includes `message`' do + let(:config) { new_config('skipped_commit_types' => ['message']) } + subject(:hook) { hook_for(config, message_context) } + + it { is_expected.to pass } end - end - end - describe '#replacement_text' do - subject(:replacement_text) { hook.replacement_text } - let(:replacement_template_file) { 'valid_filename.txt' } - let(:replacement) { 'Id is: \1' } + context 'when merging, and `skipped_commit_types` includes `commit`' do + let(:config) { new_config('skipped_commit_types' => ['commit']) } + subject(:hook) { hook_for(config, commit_context) } - context 'when the replacement text points to a valid filename' do - before do - hook.stub(:replacement_text_config).and_return(replacement_template_file) - File.stub(:exist?).and_return(true) - File.stub(:read).with(replacement_template_file).and_return(replacement) + it { is_expected.to pass } + end + + context 'when merging, and `skipped_commit_types` includes `squash`' do + let(:config) { new_config('skipped_commit_types' => ['squash']) } + subject(:hook) { hook_for(config, squash_context) } + + it { is_expected.to pass } end + end + + context 'when the replacement text points to a valid filename' do + before { checkout_branch '123-topic' } + before { add_file 'replacement_text.txt', 'FOO' } + after { remove_file 'replacement_text.txt' } + + let(:config) { new_config('replacement_text' => 'replacement_text.txt') } + let(:normal_context) { new_context(config, ['COMMIT_EDITMSG']) } + subject(:hook) { hook_for(config, normal_context) } + + before { hook.run } + + it { is_expected.to pass } + + let(:commit_msg) { File.read('COMMIT_EDITMSG') } - describe 'it reads it as the replacement template' do - it { is_expected.to eq(replacement) } + it 'uses the file contents as the replacement text' do + expect(commit_msg).to eq(File.read('replacement_text.txt')) end end end