Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 46 additions & 4 deletions lib/overcommit/hook_context/pre_commit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,30 @@ module Overcommit::HookContext
# This includes staged files, which lines of those files have been modified,
# etc. It is also responsible for saving/restoring the state of the repo so
# hooks only inspect staged changes.
class PreCommit < Base
class PreCommit < Base # rubocop:disable ClassLength
# Returns whether this hook run was triggered by `git commit --amend`
def amendment?
return @amendment unless @amendment.nil?

cmd = Overcommit::Utils.parent_command
amend_pattern = 'commit(\s.*)?\s--amend(\s|$)'

return @amendment if
# True if the command is a commit with the --amend flag
@amendment = !(/\s#{amend_pattern}/ =~ cmd).nil?

# Check for git aliases that call `commit --amend`
`git config --get-regexp '^alias\\.' '#{amend_pattern}'`.
scan(/alias\.([-\w]+)/). # Extract the alias
each do |match|
return @amendment if
# True if the command uses a git alias for `commit --amend`
@amendment = !(/git\s+#{match[0]}/ =~ cmd).nil?
end

@amendment
end

# Stash unstaged contents of files so hooks don't see changes that aren't
# about to be committed.
def setup_environment
Expand Down Expand Up @@ -64,7 +87,16 @@ def cleanup_environment
# Get a list of added, copied, or modified files that have been staged.
# Renames and deletions are ignored, since there should be nothing to check.
def modified_files
@modified_files ||= Overcommit::GitRepo.modified_files(staged: true)
unless @modified_files
@modified_files = Overcommit::GitRepo.modified_files(staged: true)

# Include files modified in last commit if amending
if amendment?
subcmd = 'show --format=%n'
@modified_files += Overcommit::GitRepo.modified_files(subcmd: subcmd)
end
end
@modified_files
end

# @deprecated
Expand All @@ -78,8 +110,18 @@ def modified_lines(file)
# changed in a specified file.
def modified_lines_in_file(file)
@modified_lines ||= {}
@modified_lines[file] ||=
Overcommit::GitRepo.extract_modified_lines(file, staged: true)
unless @modified_lines[file]
@modified_lines[file] =
Overcommit::GitRepo.extract_modified_lines(file, staged: true)

# Include lines modified in last commit if amending
if amendment?
subcmd = 'show --format=%n'
@modified_lines[file] +=
Overcommit::GitRepo.extract_modified_lines(file, subcmd: subcmd)
end
end
@modified_lines[file]
end

private
Expand Down
5 changes: 5 additions & 0 deletions lib/overcommit/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,11 @@ def in_path?(cmd)
false
end

# Return the parent command that triggered this hook run
def parent_command
`ps -ocommand= -p #{Process.ppid}`.chomp
end

# Execute a command in a subprocess, capturing exit status and output from
# both standard and error streams.
#
Expand Down
136 changes: 136 additions & 0 deletions spec/overcommit/hook_context/pre_commit_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,63 @@
let(:input) { double('input') }
let(:context) { described_class.new(config, args, input) }

describe '#amendment?' do
subject { context.amendment? }

before do
Overcommit::Utils.stub(:parent_command).and_return(command)
end

context 'when amending a commit using `git commit --amend`' do
let(:command) { 'git commit --amend' }

it { should == true }
end

context 'when amending a commit using a git alias' do
around do |example|
repo do
`git config alias.amend 'commit --amend'`
`git config alias.other-amend 'commit --amend'`
example.run
end
end

context 'when using one of multiple aliases' do
let(:command) { 'git amend' }

it { should == true }
end

context 'when using another of multiple aliases' do
let(:command) { 'git other-amend' }

it { should == true }
end
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should also add a quick test ensuring we don't match silly aliases like:

[alias]
silly--amend = log

...which is possible if you see my comment above about hypens in aliases.


context 'when not amending a commit' do
context 'using `git commit`' do
let(:command) { 'git commit' }

it { should == false }
end

context 'using a git alias containing "--amend"' do
let(:command) { 'git no--amend' }

around do |example|
repo do
`git config alias.no--amend commit`
example.run
end
end

it { should == false }
end
end
end

describe '#setup_environment' do
subject { context.setup_environment }

Expand Down Expand Up @@ -261,6 +318,10 @@
describe '#modified_files' do
subject { context.modified_files }

before do
context.stub(:amendment?).and_return(false)
end

it 'does not include submodules' do
submodule = repo do
`touch foo`
Expand Down Expand Up @@ -324,5 +385,80 @@

it { should be_empty }
end

context 'when amending last commit' do
around do |example|
repo do
FileUtils.touch('some-file')
`git add some-file`
`git commit -m 'Initial commit'`
FileUtils.touch('other-file')
`git add other-file`
example.run
end
end

before do
context.stub(:amendment?).and_return(true)
end

it { should =~ [File.expand_path('some-file'), File.expand_path('other-file')] }
end
end

describe '#modified_lines_in_file' do
let(:modified_file) { 'some-file' }
subject { context.modified_lines_in_file(modified_file) }

before do
context.stub(:amendment?).and_return(false)
end

context 'when file contains a trailing newline' do
around do |example|
repo do
File.open(modified_file, 'w') { |f| (1..3).each { |i| f.write("#{i}\n") } }
`git add #{modified_file}`
example.run
end
end

it { should == Set.new(1..3) }
end

context 'when file does not contain a trailing newline' do
around do |example|
repo do
File.open(modified_file, 'w') do |f|
(1..2).each { |i| f.write("#{i}\n") }
f.write(3)
end

`git add #{modified_file}`
example.run
end
end

it { should == Set.new(1..3) }
end

context 'when amending last commit' do
around do |example|
repo do
File.open(modified_file, 'w') { |f| (1..3).each { |i| f.write("#{i}\n") } }
`git add #{modified_file}`
`git commit -m "Add files"`
File.open(modified_file, 'a') { |f| f.puts 4 }
`git add #{modified_file}`
example.run
end
end

before do
context.stub(:amendment?).and_return(true)
end

it { should == Set.new(1..4) }
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add tests that ensure we're properly merging the two result sets (already committed files and newly staged files) when we are amending with additional staged files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, done in d2dacb7

end
end
10 changes: 10 additions & 0 deletions spec/overcommit/utils_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,16 @@
# rubocop:enable Metrics/LineLength
end

describe '.parent_command' do
subject { described_class.parent_command }

before do
Process.stub(:ppid) { Process.pid }
end

it { should =~ /rspec/ }
end

describe '.execute' do
let(:arguments) { %w[echo -n Hello World] }
subject { described_class.execute(arguments) }
Expand Down