From 4e61c25b0bdaed7cfc4a8c3d6b01652f5d23971a Mon Sep 17 00:00:00 2001 From: James Couball Date: Sun, 6 Jul 2025 13:04:32 -0700 Subject: [PATCH 1/3] fix: fix Rubocop Metrics/ClassLength offense (exclude tests) Exclude tests from the Metrics/ClassLength Rubocop offfense since the plan is to rewrite the test suite in RSpec. At that time many of the Rubocop exclusions will be removed. --- .rubocop.yml | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index f12a62d1..60a81291 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -3,13 +3,18 @@ inherit_from: .rubocop_todo.yml inherit_gem: main_branch_shared_rubocop_config: config/rubocop.yml -# Don't care about CyclomaticComplexity or AbcSize in TestUnit tests This should go -# away when we switch to RSpec. +# Don't care about complexity offenses in the TestUnit tests This exclusions +# will be removed when we switch to RSpec. Metrics/CyclomaticComplexity: Exclude: - "tests/test_helper.rb" - "tests/units/**/*" +Metrics/ClassLength: + Exclude: + - "tests/test_helper.rb" + - "tests/units/**/*" + Metrics/AbcSize: Exclude: - "tests/test_helper.rb" From 0e9d62db06365959365296a5a9b7d35fb3c7631b Mon Sep 17 00:00:00 2001 From: James Couball Date: Sun, 6 Jul 2025 13:09:33 -0700 Subject: [PATCH 2/3] fix: fix Rubocop Metrics/ClassLength offense (refactor Git::Status) This refactoring streamlines the Git::Status class by decomposing its responsibilities, resulting in cleaner, more focused components: * `StatusFile`: The inner data responsibilities was moved to its own class. * `StatusFileFactory`: A new private factory was created to encapsulate all the logic for executing git commands and parsing their output into StatusFile objects. I think the result is more readable and maintainable code. --- .rubocop_todo.yml | 4 +- lib/git/status.rb | 368 ++++++++++++++-------------------------------- 2 files changed, 111 insertions(+), 261 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d85c7f54..7a979d4a 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,12 +1,12 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-07-06 05:52:16 UTC using RuboCop version 1.77.0. +# on 2025-07-06 20:05:03 UTC using RuboCop version 1.77.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 21 +# Offense count: 3 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: Max: 1032 diff --git a/lib/git/status.rb b/lib/git/status.rb index 544f861a..4a63b266 100644 --- a/lib/git/status.rb +++ b/lib/git/status.rb @@ -1,115 +1,48 @@ # frozen_string_literal: true +# These would be required by the main `git.rb` file + module Git - # The status class gets the status of a git repository - # - # This identifies which files have been modified, added, or deleted from the - # worktree. Untracked files are also identified. - # - # The Status object is an Enumerable that contains StatusFile objects. + # The Status class gets the status of a git repository. It identifies which + # files have been modified, added, or deleted, including untracked files. + # The Status object is an Enumerable of StatusFile objects. # # @api public # class Status include Enumerable + # @param base [Git::Base] The base git object def initialize(base) @base = base - construct_status - end - - # - # Returns an Enumerable containing files that have changed from the - # git base directory - # - # @return [Enumerable] - def changed - @changed ||= @files.select { |_k, f| f.type == 'M' } - end - - # - # Determines whether the given file has been changed. - # File path starts at git base directory - # - # @param file [String] The name of the file. - # @example Check if lib/git.rb has changed. - # changed?('lib/git.rb') - # @return [Boolean] - def changed?(file) - case_aware_include?(:changed, :lc_changed, file) - end - - # Returns an Enumerable containing files that have been added. - # File path starts at git base directory - # - # @return [Enumerable] - def added - @added ||= @files.select { |_k, f| f.type == 'A' } - end - - # Determines whether the given file has been added to the repository - # - # File path starts at git base directory - # - # @param file [String] The name of the file. - # @example Check if lib/git.rb is added. - # added?('lib/git.rb') - # @return [Boolean] - def added?(file) - case_aware_include?(:added, :lc_added, file) + # The factory returns a hash of file paths to StatusFile objects. + @files = StatusFileFactory.new(base).construct_files end - # - # Returns an Enumerable containing files that have been deleted. - # File path starts at git base directory - # - # @return [Enumerable] - def deleted - @deleted ||= @files.select { |_k, f| f.type == 'D' } - end + # File status collections, memoized for performance. + def changed = @changed ||= select_files { |f| f.type == 'M' } + def added = @added ||= select_files { |f| f.type == 'A' } + def deleted = @deleted ||= select_files { |f| f.type == 'D' } + # This works with `true` or `nil` + def untracked = @untracked ||= select_files(&:untracked) - # - # Determines whether the given file has been deleted from the repository - # File path starts at git base directory - # - # @param file [String] The name of the file. - # @example Check if lib/git.rb is deleted. - # deleted?('lib/git.rb') - # @return [Boolean] - def deleted?(file) - case_aware_include?(:deleted, :lc_deleted, file) - end - - # - # Returns an Enumerable containing files that are not tracked in git. - # File path starts at git base directory - # - # @return [Enumerable] - def untracked - @untracked ||= @files.select { |_k, f| f.untracked } - end + # Predicate methods to check the status of a specific file. + def changed?(file) = file_in_collection?(:changed, file) + def added?(file) = file_in_collection?(:added, file) + def deleted?(file) = file_in_collection?(:deleted, file) + def untracked?(file) = file_in_collection?(:untracked, file) - # - # Determines whether the given file is tracked by git. - # File path starts at git base directory - # - # @param file [String] The name of the file. - # @example Check if lib/git.rb is an untracked file. - # untracked?('lib/git.rb') - # @return [Boolean] - def untracked?(file) - case_aware_include?(:untracked, :lc_untracked, file) - end + # Access a status file by path, or iterate over all status files. + def [](file) = @files[file] + def each(&) = @files.values.each(&) + # Returns a formatted string representation of the status. def pretty - out = +'' - each do |file| - out << pretty_file(file) - end - out << "\n" - out + map { |file| pretty_file(file) }.join << "\n" end + private + def pretty_file(file) <<~FILE #{file.path} @@ -121,198 +54,115 @@ def pretty_file(file) FILE end - # enumerable method - - def [](file) - @files[file] + def select_files(&block) + @files.select { |_path, file| block.call(file) } end - def each(&) - @files.values.each(&) + def file_in_collection?(collection_name, file_path) + collection = public_send(collection_name) + if ignore_case? + downcased_keys(collection_name).include?(file_path.downcase) + else + collection.key?(file_path) + end end - # subclass that does heavy lifting - class StatusFile - # @!attribute [r] path - # The path of the file relative to the project root directory - # @return [String] - attr_accessor :path - - # @!attribute [r] type - # The type of change - # - # * 'M': modified - # * 'A': added - # * 'D': deleted - # * nil: ??? - # - # @return [String] - attr_accessor :type - - # @!attribute [r] mode_index - # The mode of the file in the index - # @return [String] - # @example 100644 - # - attr_accessor :mode_index - - # @!attribute [r] mode_repo - # The mode of the file in the repo - # @return [String] - # @example 100644 - # - attr_accessor :mode_repo - - # @!attribute [r] sha_index - # The sha of the file in the index - # @return [String] - # @example 123456 - # - attr_accessor :sha_index + def downcased_keys(collection_name) + @_downcased_keys ||= {} + @_downcased_keys[collection_name] ||= + public_send(collection_name).keys.to_set(&:downcase) + end - # @!attribute [r] sha_repo - # The sha of the file in the repo - # @return [String] - # @example 123456 - attr_accessor :sha_repo + def ignore_case? + return @_ignore_case if defined?(@_ignore_case) - # @!attribute [r] untracked - # Whether the file is untracked - # @return [Boolean] - attr_accessor :untracked + @_ignore_case = (@base.config('core.ignoreCase') == 'true') + rescue Git::FailedError + @_ignore_case = false + end + end +end - # @!attribute [r] stage - # The stage of the file - # - # * '0': the unmerged state - # * '1': the common ancestor (or original) version - # * '2': "our version" from the current branch head - # * '3': "their version" from the other branch head - # @return [String] - attr_accessor :stage +module Git + class Status + # Represents a single file's status in the git repository. Each instance + # holds information about a file's state in the index and working tree. + class StatusFile + attr_reader :path, :type, :stage, :mode_index, :mode_repo, + :sha_index, :sha_repo, :untracked def initialize(base, hash) - @base = base - @path = hash[:path] - @type = hash[:type] - @stage = hash[:stage] + @base = base + @path = hash[:path] + @type = hash[:type] + @stage = hash[:stage] @mode_index = hash[:mode_index] - @mode_repo = hash[:mode_repo] - @sha_index = hash[:sha_index] - @sha_repo = hash[:sha_repo] - @untracked = hash[:untracked] + @mode_repo = hash[:mode_repo] + @sha_index = hash[:sha_index] + @sha_repo = hash[:sha_repo] + @untracked = hash[:untracked] end + # Returns a Git::Object::Blob for either the index or repo version of the file. def blob(type = :index) - if type == :repo - @base.object(@sha_repo) - else - begin - @base.object(@sha_index) - rescue StandardError - @base.object(@sha_repo) - end - end - end - end - - private - - def construct_status - # Lists all files in the index and the worktree - # git ls-files --stage - # { file => { path: file, mode_index: '100644', sha_index: 'dd4fc23', stage: '0' } } - @files = @base.lib.ls_files - - # Lists files in the worktree that are not in the index - # Add untracked files to @files - fetch_untracked - - # Lists files that are different between the index vs. the worktree - fetch_modified - - # Lists files that are different between the repo HEAD vs. the worktree - fetch_added - - @files.each do |k, file_hash| - @files[k] = StatusFile.new(@base, file_hash) + sha = type == :repo ? sha_repo : (sha_index || sha_repo) + @base.object(sha) if sha end end + end +end - def fetch_untracked - # git ls-files --others --exclude-standard, chdir: @git_work_dir) - # { file => { path: file, untracked: true } } - @base.lib.untracked_files.each do |file| - @files[file] = { path: file, untracked: true } +module Git + class Status + # A factory class responsible for fetching git status data and building + # a hash of StatusFile objects. + # @api private + class StatusFileFactory + def initialize(base) + @base = base + @lib = base.lib end - end - def fetch_modified - # Files changed between the index vs. the worktree - # git diff-files - # { - # file => { - # path: file, type: 'M', mode_index: '100644', mode_repo: '100644', - # sha_index: '0000000', :sha_repo: '52c6c4e' - # } - # } - @base.lib.diff_files.each do |path, data| - @files[path] ? @files[path].merge!(data) : @files[path] = data + # Gathers all status data and builds a hash of file paths to + # StatusFile objects. + def construct_files + files_data = fetch_all_files_data + files_data.transform_values do |data| + StatusFile.new(@base, data) + end end - end - def fetch_added - return if @base.lib.empty? + private - # Files changed between the repo HEAD vs. the worktree - # git diff-index HEAD - # { - # file => { - # path: file, type: 'M', mode_index: '100644', mode_repo: '100644', - # sha_index: '0000000', :sha_repo: '52c6c4e' - # } - # } - @base.lib.diff_index('HEAD').each do |path, data| - @files[path] ? @files[path].merge!(data) : @files[path] = data + # Fetches and merges status information from multiple git commands. + def fetch_all_files_data + files = @lib.ls_files # Start with files tracked in the index. + merge_untracked_files(files) + merge_modified_files(files) + merge_head_diffs(files) + files end - end - - # It's worth noting that (like git itself) this gem will not behave well if - # ignoreCase is set inconsistently with the file-system itself. For details: - # https://git-scm.com/docs/git-config#Documentation/git-config.txt-coreignoreCase - def ignore_case? - return @_ignore_case if defined?(@_ignore_case) - - @_ignore_case = @base.config('core.ignoreCase') == 'true' - rescue Git::FailedError - @_ignore_case = false - end - - def downcase_keys(hash) - hash.transform_keys(&:downcase) - end - - def lc_changed - @lc_changed ||= changed.transform_keys(&:downcase) - end - def lc_added - @lc_added ||= added.transform_keys(&:downcase) - end + def merge_untracked_files(files) + @lib.untracked_files.each do |file| + files[file] = { path: file, untracked: true } + end + end - def lc_deleted - @lc_deleted ||= deleted.transform_keys(&:downcase) - end + def merge_modified_files(files) + # Merge changes between the index and the working directory. + @lib.diff_files.each do |path, data| + (files[path] ||= {}).merge!(data) + end + end - def lc_untracked - @lc_untracked ||= untracked.transform_keys(&:downcase) - end + def merge_head_diffs(files) + return if @lib.empty? - def case_aware_include?(cased_hash, downcased_hash, file) - if ignore_case? - send(downcased_hash).include?(file.downcase) - else - send(cased_hash).include?(file) + # Merge changes between HEAD and the index. + @lib.diff_index('HEAD').each do |path, data| + (files[path] ||= {}).merge!(data) + end end end end From b00ca5266e8b561bd05cb6de2f878668b8679b1e Mon Sep 17 00:00:00 2001 From: James Couball Date: Sun, 6 Jul 2025 14:11:02 -0700 Subject: [PATCH 3/3] fix: fix Rubocop Metrics/ClassLength offense (refactor Git::Log) The Git::Log class had grown to over 150 lines, making it difficult to read and maintain. This refactoring simplifies the internal implementation to improve its structure and reduce its size to under 100 lines, without altering the public API. The primary motivations were to enhance readability and make the class easier to extend in the future. The key changes include: - Consolidated all query parameters from individual instance variables into a single `@options` hash to simplify state management. - Replaced repetitive builder methods with a concise `set_option` private helper, reducing code duplication. - Centralized the command execution logic into a single `run_log_if_dirty` method to ensure consistent behavior. - Simplified the deprecated Enumerable methods by using a helper for warnings and safe navigation (`&.`) for cleaner error handling. - Modernized the nested `Git::Log::Result` class to use `Data.define`, leveraging Ruby 3.2+ features to create a more concise and immutable result object. --- .rubocop_todo.yml | 4 +- lib/git/log.rb | 293 +++++++++++----------------------------------- 2 files changed, 73 insertions(+), 224 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 7a979d4a..1333d28e 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,12 +1,12 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2025-07-06 20:05:03 UTC using RuboCop version 1.77.0. +# on 2025-07-06 21:08:14 UTC using RuboCop version 1.77.0. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new # versions of RuboCop, may require this file to be generated again. -# Offense count: 3 +# Offense count: 2 # Configuration parameters: CountComments, CountAsOne. Metrics/ClassLength: Max: 1032 diff --git a/lib/git/log.rb b/lib/git/log.rb index 1dbfc8d8..c5b3c6da 100644 --- a/lib/git/log.rb +++ b/lib/git/log.rb @@ -1,78 +1,34 @@ # frozen_string_literal: true module Git - # Return the last n commits that match the specified criteria + # Builds and executes a `git log` query. # - # @example The last (default number) of commits - # git = Git.open('.') - # Git::Log.new(git).execute #=> Enumerable of the last 30 commits + # This class provides a fluent interface for building complex `git log` queries. + # The query is lazily executed when results are requested either via the modern + # `#execute` method or the deprecated Enumerable methods. # - # @example The last n commits - # Git::Log.new(git).max_commits(50).execute #=> Enumerable of last 50 commits - # - # @example All commits returned by `git log` - # Git::Log.new(git).max_count(:all).execute #=> Enumerable of all commits - # - # @example All commits that match complex criteria - # Git::Log.new(git) - # .max_count(:all) - # .object('README.md') - # .since('10 years ago') - # .between('v1.0.7', 'HEAD') - # .execute + # @example Using the modern `execute` API + # log = git.log.max_count(50).between('v1.0', 'v1.1').author('Scott') + # results = log.execute + # puts "Found #{results.size} commits." + # results.each { |commit| puts commit.sha } # # @api public # class Log include Enumerable - # An immutable collection of commits returned by Git::Log#execute - # - # This object is an Enumerable that contains Git::Object::Commit objects. - # It provides methods to access the commit data without executing any - # further git commands. - # + # An immutable, Enumerable collection of `Git::Object::Commit` objects. + # Returned by `Git::Log#execute`. # @api public - class Result + Result = Data.define(:commits) do include Enumerable - # @private - def initialize(commits) - @commits = commits - end - - # @return [Integer] the number of commits in the result set - def size - @commits.size - end - - # Iterates over each commit in the result set - # - # @yield [Git::Object::Commit] - def each(&) - @commits.each(&) - end - - # @return [Git::Object::Commit, nil] the first commit in the result set - def first - @commits.first - end - - # @return [Git::Object::Commit, nil] the last commit in the result set - def last - @commits.last - end - - # @param index [Integer] the index of the commit to return - # @return [Git::Object::Commit, nil] the commit at the given index - def [](index) - @commits[index] - end - - # @return [String] a string representation of the log - def to_s - map(&:to_s).join("\n") - end + def each(&block) = commits.each(&block) + def last = commits.last + def [](index) = commits[index] + def to_s = map(&:to_s).join("\n") + def size = commits.size end # Create a new Git::Log object @@ -88,12 +44,29 @@ def to_s # Passing max_count to {#initialize} is equivalent to calling {#max_count} on the object. # def initialize(base, max_count = 30) - dirty_log @base = base - max_count(max_count) + @options = {} + @dirty = true + self.max_count(max_count) end - # Executes the git log command and returns an immutable result object. + # Set query options using a fluent interface. + # Each method returns `self` to allow for chaining. + # + def max_count(num) = set_option(:count, num == :all ? nil : num) + def all = set_option(:all, true) + def object(objectish) = set_option(:object, objectish) + def author(regex) = set_option(:author, regex) + def grep(regex) = set_option(:grep, regex) + def path(path) = set_option(:path_limiter, path) + def skip(num) = set_option(:skip, num) + def since(date) = set_option(:since, date) + def until(date) = set_option(:until, date) + def between(val1, val2 = nil) = set_option(:between, [val1, val2]) + def cherry = set_option(:cherry, true) + def merges = set_option(:merges, true) + + # Executes the git log command and returns an immutable result object # # This is the preferred way to get log data. It separates the query # building from the execution, making the API more predictable. @@ -107,188 +80,64 @@ def initialize(base, max_count = 30) # end # # @return [Git::Log::Result] an object containing the log results + # def execute - run_log + run_log_if_dirty Result.new(@commits) end - # The maximum number of commits to return - # - # @example All commits returned by `git log` - # git = Git.open('.') - # Git::Log.new(git).max_count(:all) - # - # @param num_or_all [Integer, Symbol, nil] the number of commits to return, or - # `:all` or `nil` to return all - # - # @return [self] - # - def max_count(num_or_all) - dirty_log - @max_count = num_or_all == :all ? nil : num_or_all - self - end - - # Adds the --all flag to the git log command - # - # This asks for the logs of all refs (basically all commits reachable by HEAD, - # branches, and tags). This does not control the maximum number of commits - # returned. To control how many commits are returned, call {#max_count}. - # - # @example Return the last 50 commits reachable by all refs - # git = Git.open('.') - # Git::Log.new(git).max_count(50).all - # - # @return [self] - # - def all - dirty_log - @all = true - self - end - - def object(objectish) - dirty_log - @object = objectish - self - end - - def author(regex) - dirty_log - @author = regex - self - end - - def grep(regex) - dirty_log - @grep = regex - self - end - - def path(path) - dirty_log - @path = path - self - end - - def skip(num) - dirty_log - @skip = num - self - end - - def since(date) - dirty_log - @since = date - self - end - - def until(date) - dirty_log - @until = date - self - end - - def between(sha1, sha2 = nil) - dirty_log - @between = [sha1, sha2] - self - end - - def cherry - dirty_log - @cherry = true - self - end - - def merges - dirty_log - @merges = true - self - end - - def to_s - deprecate_method(__method__) - check_log - @commits.map(&:to_s).join("\n") - end - - # forces git log to run - - def size - deprecate_method(__method__) - check_log - begin - @commits.size - rescue StandardError - nil - end - end + # @!group Deprecated Enumerable Interface + # @deprecated Use {#execute} and call `each` on the result. def each(&) - deprecate_method(__method__) - check_log + deprecate_and_run @commits.each(&) end - def first - deprecate_method(__method__) - check_log - begin - @commits.first - rescue StandardError - nil - end + # @deprecated Use {#execute} and call `size` on the result. + def size + deprecate_and_run + @commits&.size end - def last - deprecate_method(__method__) - check_log - begin - @commits.last - rescue StandardError - nil - end + # @deprecated Use {#execute} and call `to_s` on the result. + def to_s + deprecate_and_run + @commits&.map(&:to_s)&.join("\n") end - def [](index) - deprecate_method(__method__) - check_log - begin - @commits[index] - rescue StandardError - nil + # @deprecated Use {#execute} and call the method on the result. + %i[first last []].each do |method_name| + define_method(method_name) do |*args| + deprecate_and_run + @commits&.public_send(method_name, *args) end end - private + # @!endgroup - def deprecate_method(method_name) - Git::Deprecation.warn( - "Calling Git::Log##{method_name} is deprecated and will be removed in a future version. " \ - "Call #execute and then ##{method_name} on the result object." - ) - end + private - def dirty_log - @dirty_flag = true + def set_option(key, value) + @dirty = true + @options[key] = value + self end - def check_log - return unless @dirty_flag + def run_log_if_dirty + return unless @dirty - run_log - @dirty_flag = false + log_data = @base.lib.full_log_commits(@options) + @commits = log_data.map { |c| Git::Object::Commit.new(@base, c['sha'], c) } + @dirty = false end - # actually run the 'git log' command - def run_log - log = @base.lib.full_log_commits( - count: @max_count, all: @all, object: @object, path_limiter: @path, since: @since, - author: @author, grep: @grep, skip: @skip, until: @until, between: @between, - cherry: @cherry, merges: @merges + def deprecate_and_run(method = caller_locations(1, 1)[0].label) + Git::Deprecation.warn( + "Calling Git::Log##{method} is deprecated. " \ + "Call #execute and then ##{method} on the result object." ) - @commits = log.map { |c| Git::Object::Commit.new(@base, c['sha'], c) } + run_log_if_dirty end end end