Skip to content

Refactor Git::Diff to separate classes for DiffStats and DiffPathStatus #815

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 2, 2025
Merged
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
21 changes: 21 additions & 0 deletions lib/git/base.rb
Original file line number Diff line number Diff line change
Expand Up @@ -782,6 +782,27 @@ def merge_base(*args)
shas.map { |sha| gcommit(sha) }
end

# Returns a Git::Diff::Stats object for accessing diff statistics.
#
# @param objectish [String] The first commit or object to compare. Defaults to 'HEAD'.
# @param obj2 [String, nil] The second commit or object to compare.
# @return [Git::Diff::Stats]
def diff_stats(objectish = 'HEAD', obj2 = nil)
Git::DiffStats.new(self, objectish, obj2)
end

# Returns a Git::Diff::PathStatus object for accessing the name-status report.
#
# @param objectish [String] The first commit or object to compare. Defaults to 'HEAD'.
# @param obj2 [String, nil] The second commit or object to compare.
# @return [Git::Diff::PathStatus]
def diff_path_status(objectish = 'HEAD', obj2 = nil)
Git::DiffPathStatus.new(self, objectish, obj2)
end

# Provided for backwards compatibility
alias diff_name_status diff_path_status

private

# Normalize options before they are sent to Git::Base.new
Expand Down
159 changes: 79 additions & 80 deletions lib/git/diff.rb
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
# frozen_string_literal: true

module Git
require_relative 'diff_path_status'
require_relative 'diff_stats'

# object that holds the last X commits on given branch
module Git
# object that holds the diff between two commits
class Diff
include Enumerable

Expand All @@ -12,63 +14,68 @@ def initialize(base, from = nil, to = nil)
@to = to && to.to_s

@path = nil
@full_diff = nil
@full_diff_files = nil
@stats = nil
end
attr_reader :from, :to

def name_status
cache_name_status
end

def path(path)
@path = path
self
end

def size
cache_stats
@stats[:total][:files]
def patch
@base.lib.diff_full(@from, @to, { path_limiter: @path })
end
alias_method :to_s, :patch

def lines
cache_stats
@stats[:total][:lines]
def [](key)
process_full
@full_diff_files.assoc(key)[1]
end

def deletions
cache_stats
@stats[:total][:deletions]
def each(&block)
process_full
@full_diff_files.map { |file| file[1] }.each(&block)
end

def insertions
cache_stats
@stats[:total][:insertions]
#
# DEPRECATED METHODS
#

def name_status
Git::Deprecation.warn("Git::Diff#name_status is deprecated. Use Git::Base#diff_path_status instead.")
path_status_provider.to_h
end

def stats
cache_stats
@stats
def size
Git::Deprecation.warn("Git::Diff#size is deprecated. Use Git::Base#diff_stats(...).total[:files] instead.")
stats_provider.total[:files]
end

# if file is provided and is writable, it will write the patch into the file
def patch(file = nil)
cache_full
@full_diff


def lines
Git::Deprecation.warn("Git::Diff#lines is deprecated. Use Git::Base#diff_stats(...).lines instead.")
stats_provider.lines
end
alias_method :to_s, :patch

# enumerable methods
def deletions
Git::Deprecation.warn("Git::Diff#deletions is deprecated. Use Git::Base#diff_stats(...).deletions instead.")
stats_provider.deletions
end

def [](key)
process_full
@full_diff_files.assoc(key)[1]
def insertions
Git::Deprecation.warn("Git::Diff#insertions is deprecated. Use Git::Base#diff_stats(...).insertions instead.")
stats_provider.insertions
end

def each(&block) # :yields: each Git::DiffFile in turn
process_full
@full_diff_files.map { |file| file[1] }.each(&block)
def stats
Git::Deprecation.warn("Git::Diff#stats is deprecated. Use Git::Base#diff_stats instead.")
# CORRECTED: Re-create the original hash structure for backward compatibility
{
files: stats_provider.files,
total: stats_provider.total
}
end

class DiffFile
Expand Down Expand Up @@ -102,56 +109,48 @@ def blob(type = :dst)

private

def cache_full
@full_diff ||= @base.lib.diff_full(@from, @to, {:path_limiter => @path})
end

def process_full
return if @full_diff_files
cache_full
@full_diff_files = process_full_diff
end
def process_full
return if @full_diff_files
@full_diff_files = process_full_diff
end

def cache_stats
@stats ||= @base.lib.diff_stats(@from, @to, {:path_limiter => @path})
end
# CORRECTED: Pass the @path variable to the new objects
def path_status_provider
@path_status_provider ||= Git::DiffPathStatus.new(@base, @from, @to, @path)
end

def cache_name_status
@name_status ||= @base.lib.diff_name_status(@from, @to, {:path => @path})
end
# CORRECTED: Pass the @path variable to the new objects
def stats_provider
@stats_provider ||= Git::DiffStats.new(@base, @from, @to, @path)
end

# break up @diff_full
def process_full_diff
defaults = {
:mode => '',
:src => '',
:dst => '',
:type => 'modified'
}
final = {}
current_file = nil
@full_diff.split("\n").each do |line|
if m = %r{\Adiff --git ("?)a/(.+?)\1 ("?)b/(.+?)\3\z}.match(line)
current_file = Git::EscapedPath.new(m[2]).unescape
final[current_file] = defaults.merge({:patch => line, :path => current_file})
else
if m = /^index ([0-9a-f]{4,40})\.\.([0-9a-f]{4,40})( ......)*/.match(line)
final[current_file][:src] = m[1]
final[current_file][:dst] = m[2]
final[current_file][:mode] = m[3].strip if m[3]
end
if m = /^([[:alpha:]]*?) file mode (......)/.match(line)
final[current_file][:type] = m[1]
final[current_file][:mode] = m[2]
end
if m = /^Binary files /.match(line)
final[current_file][:binary] = true
end
final[current_file][:patch] << "\n" + line
def process_full_diff
defaults = {
mode: '', src: '', dst: '', type: 'modified'
}
final = {}
current_file = nil
patch.split("\n").each do |line|
if m = %r{\Adiff --git ("?)a/(.+?)\1 ("?)b/(.+?)\3\z}.match(line)
current_file = Git::EscapedPath.new(m[2]).unescape
final[current_file] = defaults.merge({ patch: line, path: current_file })
else
if m = /^index ([0-9a-f]{4,40})\.\.([0-9a-f]{4,40})( ......)*/.match(line)
final[current_file][:src] = m[1]
final[current_file][:dst] = m[2]
final[current_file][:mode] = m[3].strip if m[3]
end
if m = /^([[:alpha:]]*?) file mode (......)/.match(line)
final[current_file][:type] = m[1]
final[current_file][:mode] = m[2]
end
if m = /^Binary files /.match(line)
final[current_file][:binary] = true
end
final[current_file][:patch] << "\n" + line
end
final.map { |e| [e[0], DiffFile.new(@base, e[1])] }
end

final.map { |e| [e[0], DiffFile.new(@base, e[1])] }
end
end
end
45 changes: 45 additions & 0 deletions lib/git/diff_path_status.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

module Git
class DiffPathStatus
include Enumerable

# @private
def initialize(base, from, to, path_limiter = nil)
# Eagerly check for invalid arguments
[from, to].compact.each do |arg|
raise ArgumentError, "Invalid argument: '#{arg}'" if arg.start_with?('-')
end

@base = base
@from = from
@to = to
@path_limiter = path_limiter
@path_status = nil
end

# Iterates over each file's status.
#
# @yield [path, status]
def each(&block)
fetch_path_status.each(&block)
end

# Returns the name-status report as a Hash.
#
# @return [Hash<String, String>] A hash where keys are file paths
# and values are their status codes.
def to_h
fetch_path_status
end

private

# Lazily fetches and caches the path status from the git lib.
def fetch_path_status
@path_status ||= @base.lib.diff_path_status(
@from, @to, { path: @path_limiter }
)
end
end
end
59 changes: 59 additions & 0 deletions lib/git/diff_stats.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# frozen_string_literal: true

module Git
# Provides access to the statistics of a diff between two commits,
# including insertions, deletions, and file-level details.
class DiffStats
# @private
def initialize(base, from, to, path_limiter = nil)
# Eagerly check for invalid arguments
[from, to].compact.each do |arg|
raise ArgumentError, "Invalid argument: '#{arg}'" if arg.start_with?('-')
end

@base = base
@from = from
@to = to
@path_limiter = path_limiter
@stats = nil
end

# Returns the total number of lines deleted.
def deletions
fetch_stats[:total][:deletions]
end

# Returns the total number of lines inserted.
def insertions
fetch_stats[:total][:insertions]
end

# Returns the total number of lines changed (insertions + deletions).
def lines
fetch_stats[:total][:lines]
end

# Returns a hash of statistics for each file in the diff.
#
# @return [Hash<String, {insertions: Integer, deletions: Integer}>]
def files
fetch_stats[:files]
end

# Returns a hash of the total statistics for the diff.
#
# @return [{insertions: Integer, deletions: Integer, lines: Integer, files: Integer}]
def total
fetch_stats[:total]
end

private

# Lazily fetches and caches the stats from the git lib.
def fetch_stats
@stats ||= @base.lib.diff_stats(
@from, @to, { path_limiter: @path_limiter }
)
end
end
end
2 changes: 1 addition & 1 deletion lib/git/lib.rb
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,7 @@ def diff_stats(obj1 = 'HEAD', obj2 = nil, opts = {})
hsh
end

def diff_name_status(reference1 = nil, reference2 = nil, opts = {})
def diff_path_status(reference1 = nil, reference2 = nil, opts = {})
assert_args_are_not_options('commit or commit range', reference1, reference2)

opts_arr = ['--name-status']
Expand Down
2 changes: 1 addition & 1 deletion tests/units/test_diff.rb
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_diff_patch_with_bad_commit
end
end

def test_diff_name_status_with_bad_commit
def test_diff_path_status_with_bad_commit
assert_raise(ArgumentError) do
@git.diff('-s').name_status
end
Expand Down
Loading