ruby-git · jcouball · Jul 5, 2025 · Jul 5, 2025
diff --git a/.rubocop.yml b/.rubocop.yml
@@ -3,6 +3,12 @@ inherit_from: .rubocop_todo.yml
 inherit_gem:
   main_branch_shared_rubocop_config: config/rubocop.yml
 
+# Don't care so much about length of methods in tests
+Metrics/MethodLength:
+  Exclude:
+    - "tests/test_helper.rb"
+    - "tests/units/**/*"
+
 # Allow test data to have long lines
 Layout/LineLength:
   Exclude:

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,32 +1,27 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-07-03 00:33:40 UTC using RuboCop version 1.77.0.
+# on 2025-07-05 15:13:23 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: 68
+# Offense count: 56
 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
   Max: 109
 
 # Offense count: 21
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
-  Max: 925
+  Max: 976
 
-# Offense count: 14
+# Offense count: 9
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/CyclomaticComplexity:
-  Max: 21
+  Max: 14
 
-# Offense count: 111
-# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
-Metrics/MethodLength:
-  Max: 51
-
-# Offense count: 12
+# Offense count: 7
 # Configuration parameters: AllowedMethods, AllowedPatterns.
 Metrics/PerceivedComplexity:
-  Max: 22
+  Max: 14
diff --git a/bin/command_line_test b/bin/command_line_test
@@ -83,13 +83,21 @@ class CommandLineParser
   attr_reader :option_parser
 
   def define_options
+    define_banner_and_separators
+    define_all_cli_options
+  end
+
+  def define_banner_and_separators
     option_parser.banner = "Usage:\n#{command_template}"
     option_parser.separator ''
     option_parser.separator 'Both --stdout and --stderr can be given.'
     option_parser.separator 'If --signal is given, --exitstatus is ignored.'
     option_parser.separator 'If nothing is given, the script will exit with exitstatus 0.'
     option_parser.separator ''
     option_parser.separator 'Options:'
+  end
+
+  def define_all_cli_options
     %i[
       define_help_option define_stdout_option define_stdout_file_option
       define_stderr_option define_stderr_file_option

diff --git a/lib/git/base.rb b/lib/git/base.rb
@@ -39,20 +39,29 @@ def self.config
     end
 
     def self.binary_version(binary_path)
-      result = nil
-      status = nil
-
-      begin
-        result, status = Open3.capture2e(binary_path, '-c', 'core.quotePath=true', '-c', 'color.ui=false', 'version')
-        result = result.chomp
-      rescue Errno::ENOENT
-        raise "Failed to get git version: #{binary_path} not found"
-      end
+      result, status = execute_git_version(binary_path)
 
       raise "Failed to get git version: #{status}\n#{result}" unless status.success?
 
-      version = result[/\d+(\.\d+)+/]
-      version_parts = version.split('.').collect(&:to_i)
+      parse_version_string(result)
+    end
+
+    private_class_method def self.execute_git_version(binary_path)
+      Open3.capture2e(
+        binary_path,
+        '-c', 'core.quotePath=true',
+        '-c', 'color.ui=false',
+        'version'
+      )
+    rescue Errno::ENOENT
+      raise "Failed to get git version: #{binary_path} not found"
+    end
+
+    private_class_method def self.parse_version_string(raw_string)
+      version_match = raw_string.match(/\d+(\.\d+)+/)
+      return [0, 0, 0] unless version_match
+
+      version_parts = version_match[0].split('.').map(&:to_i)
       version_parts.fill(0, version_parts.length...3)
     end
 
@@ -85,24 +94,28 @@ def self.init(directory = '.', options = {})
     end
 
     def self.root_of_worktree(working_dir)
-      result = working_dir
-      status = nil
-
       raise ArgumentError, "'#{working_dir}' does not exist" unless Dir.exist?(working_dir)
 
-      begin
-        result, status = Open3.capture2e(
-          Git::Base.config.binary_path, '-c', 'core.quotePath=true', '-c',
-          'color.ui=false', 'rev-parse', '--show-toplevel', chdir: File.expand_path(working_dir)
-        )
-        result = result.chomp
-      rescue Errno::ENOENT
-        raise ArgumentError, 'Failed to find the root of the worktree: git binary not found'
-      end
+      result, status = execute_rev_parse_toplevel(working_dir)
+      process_rev_parse_result(result, status, working_dir)
+    end
+
+    private_class_method def self.execute_rev_parse_toplevel(working_dir)
+      Open3.capture2e(
+        Git::Base.config.binary_path,
+        '-c', 'core.quotePath=true',
+        '-c', 'color.ui=false',
+        'rev-parse', '--show-toplevel',
+        chdir: File.expand_path(working_dir)
+      )
+    rescue Errno::ENOENT
+      raise ArgumentError, 'Failed to find the root of the worktree: git binary not found'
+    end
 
+    private_class_method def self.process_rev_parse_result(result, status, working_dir)
       raise ArgumentError, "'#{working_dir}' is not in a git working tree" unless status.success?
 
-      result
+      result.chomp
     end
 
     # (see Git.open)
@@ -879,19 +892,58 @@ def diff_path_status(objectish = 'HEAD', obj2 = nil)
     #   2. the working directory if NOT working with a bare repository
     #
     private_class_method def self.normalize_repository(options, default:, bare: false)
-      repository =
-        if bare
-          File.expand_path(options[:repository] || default || Dir.pwd)
-        else
-          File.expand_path(options[:repository] || '.git', options[:working_directory])
-        end
+      initial_path = initial_repository_path(options, default: default, bare: bare)
+      final_path = resolve_gitdir_if_present(initial_path, options[:working_directory])
+      options[:repository] = final_path
+    end
 
-      if File.file?(repository)
-        repository = File.expand_path(File.read(repository)[8..].strip,
-                                      options[:working_directory])
+    # Determines the initial, potential path to the repository directory
+    #
+    # This path is considered 'initial' because it is not guaranteed to be the
+    # final repository location. For features like submodules or worktrees,
+    # this path may point to a text file containing a `gitdir:` pointer to the
+    # actual repository directory elsewhere. This initial path must be
+    # subsequently resolved.
+    #
+    # @api private
+    #
+    # @param options [Hash] The options hash, checked for `[:repository]`.
+    #
+    # @param default [String] A fallback path if `options[:repository]` is not set.
+    #
+    # @param bare [Boolean] Whether the repository is bare, which changes path resolution.
+    #
+    # @return [String] The initial, absolute path to the `.git` directory or file.
+    #
+    private_class_method def self.initial_repository_path(options, default:, bare:)
+      if bare
+        File.expand_path(options[:repository] || default || Dir.pwd)
+      else
+        File.expand_path(options[:repository] || '.git', options[:working_directory])
       end
+    end
+
+    # Resolves the path to the actual repository if it's a `gitdir:` pointer file.
+    #
+    # If `path` points to a file (common in submodules and worktrees), this
+    # method reads the `gitdir:` path from it and returns the real repository
+    # path. Otherwise, it returns the original path.
+    #
+    # @api private
+    #
+    # @param path [String] The initial path to the repository, which may be a pointer file.
+    #
+    # @param working_dir [String] The working directory, used as a base to resolve the path.
+    #
+    # @return [String] The final, resolved absolute path to the repository directory.
+    #
+    private_class_method def self.resolve_gitdir_if_present(path, working_dir)
+      return path unless File.file?(path)
 
-      options[:repository] = repository
+      # The file contains `gitdir: <path>`, so we read the file,
+      # extract the path part, and expand it.
+      gitdir_pointer = File.read(path).sub(/\Agitdir: /, '').strip
+      File.expand_path(gitdir_pointer, working_dir)
     end
 
     # Normalize options[:index]

diff --git a/lib/git/diff.rb b/lib/git/diff.rb
@@ -115,41 +115,78 @@ def process_full
       @full_diff_files = process_full_diff
     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
 
-    # CORRECTED: Pass the @path variable to the new objects
     def stats_provider
       @stats_provider ||= Git::DiffStats.new(@base, @from, @to, @path)
     end
 
     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 })
+      FullDiffParser.new(@base, patch).parse
+    end
+
+    # A private parser class to process the output of `git diff`
+    # @api private
+    class FullDiffParser
+      def initialize(base, patch_text)
+        @base = base
+        @patch_text = patch_text
+        @final_files = {}
+        @current_file_data = nil
+        @defaults = { mode: '', src: '', dst: '', type: 'modified', binary: false }
+      end
+
+      def parse
+        @patch_text.split("\n").each { |line| process_line(line) }
+        @final_files.map { |filename, data| [filename, DiffFile.new(@base, data)] }
+      end
+
+      private
+
+      def process_line(line)
+        if (new_file_match = line.match(%r{\Adiff --git ("?)a/(.+?)\1 ("?)b/(.+?)\3\z}))
+          start_new_file(new_file_match, line)
         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
-          final[current_file][:binary] = true if /^Binary files /.match(line)
-          final[current_file][:patch] << "\n#{line}"
+          append_to_current_file(line)
         end
       end
-      final.map { |e| [e[0], DiffFile.new(@base, e[1])] }
+
+      def start_new_file(match, line)
+        filename = Git::EscapedPath.new(match[2]).unescape
+        @current_file_data = @defaults.merge({ patch: line, path: filename })
+        @final_files[filename] = @current_file_data
+      end
+
+      def append_to_current_file(line)
+        return unless @current_file_data
+
+        parse_index_line(line)
+        parse_file_mode_line(line)
+        check_for_binary(line)
+
+        @current_file_data[:patch] << "\n#{line}"
+      end
+
+      def parse_index_line(line)
+        return unless (match = line.match(/^index ([0-9a-f]{4,40})\.\.([0-9a-f]{4,40})( ......)*/))
+
+        @current_file_data[:src] = match[1]
+        @current_file_data[:dst] = match[2]
+        @current_file_data[:mode] = match[3].strip if match[3]
+      end
+
+      def parse_file_mode_line(line)
+        return unless (match = line.match(/^([[:alpha:]]*?) file mode (......)/))
+
+        @current_file_data[:type] = match[1]
+        @current_file_data[:mode] = match[2]
+      end
+
+      def check_for_binary(line)
+        @current_file_data[:binary] = true if line.match?(/^Binary files /)
+      end
     end
   end
 end