Refactor git status handling, add some tests (#299)

Refactor git status handling, add some tests

lib/colorls/core.rb

@@ -247,23 +247,19 @@ module ColorLS
     def git_file_info(path)
       return '  ✓ '.colorize(@colors[:unchanged]) unless @git_status[path]
 
-      Git.colored_status_symbols(@git_status[path].uniq, @colors)
+      Git.colored_status_symbols(@git_status[path], @colors)
     end
 
     def git_dir_info(path)
-      return '    ' if path == '..'
+      modes = if path == '.'
+                Set.new(@git_status.values).flatten
+              else
+                @git_status.fetch(path, nil)
+              end
 
-      direct_status = @git_status.fetch("#{path}/", nil)
+      return Git.colored_status_symbols(modes, @colors) unless modes.nil?
 
-      return Git.colored_status_symbols(direct_status.uniq, @colors) unless direct_status.nil?
-
-      prefix = path == '.' ? '' : path + '/'
-
-      modes = @git_status.select { |file, mode| file.start_with?(prefix) && mode != '!!' }.values
-
-      return '  ✓ '.colorize(@colors[:unchanged]) if modes.empty?
-
-      Git.colored_status_symbols(modes.join.uniq, @colors)
+      '    '
     end
 
     def long_info(content)

lib/colorls/git.rb

@@ -1,20 +1,25 @@
 # frozen_string_literal: true
 
+require 'pathname'
+
 module ColorLS
   module Git
     def self.status(repo_path)
-      prefix = IO.popen(['git', '-C', repo_path, 'rev-parse', '--show-prefix'], err: :close, &:gets)
+      prefix = git_prefix(repo_path)
 
       return unless $CHILD_STATUS.success?
 
-      prefix.chomp!
-      git_status = {}
+      prefix = Pathname.new(prefix.chomp)
 
-      IO.popen(['git', '-C', repo_path, 'status', '--porcelain', '-z', '-unormal', '--ignored', '.']) do |output|
+      git_status = Hash.new { |hash, key| hash[key] = Set.new }
+
+      git_subdir_status(repo_path) do |output|
         while (status_line = output.gets "\x0")
           mode, file = status_line.chomp("\x0").split(' ', 2)
 
-          git_status[file.delete_prefix(prefix)] = mode
+          path = Pathname.new(file).relative_path_from(prefix)
+
+          git_status[path.descend.first.cleanpath.to_s].add(mode)
 
           # skip the next \x0 separated original path for renames, issue #185
           output.gets("\x0") if mode.start_with? 'R'
@@ -26,7 +31,9 @@ module ColorLS
     end
 
     def self.colored_status_symbols(modes, colors)
-      modes = modes.rjust(3).ljust(4)
+      return '  ✓ '.colorize(colors[:unchanged]) if modes.empty?
+
+      modes = modes.to_a.join.uniq.rjust(3).ljust(4)
 
       modes
         .gsub('?', '?'.colorize(colors[:untracked]))
@@ -35,5 +42,17 @@ module ColorLS
         .gsub('D', 'D'.colorize(colors[:deletion]))
         .tr('!', ' ')
     end
+
+    class << self
+      private
+
+      def git_prefix(repo_path)
+        IO.popen(['git', '-C', repo_path, 'rev-parse', '--show-prefix'], err: :close, &:gets)
+      end
+
+      def git_subdir_status(repo_path)
+        IO.popen(['git', '-C', repo_path, 'status', '--porcelain', '-z', '-unormal', '--ignored', '.'])
+      end
+    end
   end
 end

spec/color_ls/git_spec.rb

@@ -0,0 +1,39 @@
+require 'spec_helper'
+
+RSpec.describe ColorLS::Git do
+  def git_status(*entries)
+    StringIO.new entries.map { |line| line + "\x0" }.join
+  end
+
+  context 'file in repository root' do
+    it 'should return `M`' do
+      allow(subject).to receive(:git_prefix).with('/repo/').and_return('')
+      allow(subject).to receive(:git_subdir_status).and_yield(git_status(" M foo.txt"))
+
+      expect(subject.status('/repo/')).to include('foo.txt' => Set['M'])
+    end
+
+    it 'should return `M`' do
+      allow(subject).to receive(:git_prefix).with('/repo/').and_return('')
+      allow(subject).to receive(:git_subdir_status).and_yield(git_status("?? foo.txt"))
+
+      expect(subject.status('/repo/')).to include('foo.txt' => Set['??'])
+    end
+  end
+
+  context 'file in subdir' do
+    it 'should return `M` for subdir' do
+      allow(subject).to receive(:git_prefix).with('/repo/').and_return('')
+      allow(subject).to receive(:git_subdir_status).and_yield(git_status(" M subdir/foo.txt"))
+
+      expect(subject.status('/repo/')).to include('subdir' => Set['M'])
+    end
+
+    it 'should return `M` and `D` for subdir' do
+      allow(subject).to receive(:git_prefix).with('/repo/').and_return('')
+      allow(subject).to receive(:git_subdir_status).and_yield(git_status(" M subdir/foo.txt", "D  subdir/other.c"))
+
+      expect(subject.status('/repo/')).to include('subdir' => Set['M', 'D'])
+    end
+  end
+end