Merge pull request #167 from mail6543210/master

Fix "diff" action on binary files

lib/homesick/cli.rb

@@ -27,13 +27,13 @@ module Homesick
       # Hack in support for diffing symlinks
       # Also adds support for checking if destination or content is a directory
       shell_metaclass = class << shell; self; end
-      shell_metaclass.send(:define_method, :show_diff) do |destination, content|
+      shell_metaclass.send(:define_method, :show_diff) do |destination, source|
         destination = Pathname.new(destination)
-        content = Pathname.new(content)
+        source = Pathname.new(source)
-        return 'Unable to create diff: destination or content is a directory' if destination.directory? || content.directory?
+        return 'Unable to create diff: destination or content is a directory' if destination.directory? || source.directory?
-        return super(destination, content) unless destination.symlink?
+        return super(destination, File.binread(source)) unless destination.symlink?
         say "- #{destination.readlink}", :red, true
-        say "+ #{content.expand_path}", :green, true
+        say "+ #{source.expand_path}", :green, true
       end
     end
 

lib/homesick/utils.rb

@@ -150,7 +150,7 @@ module Homesick
 
     def collision_accepted?(destination, source)
       fail "Arguments must be instances of Pathname, #{destination.class.name} and #{source.class.name} given" unless destination.instance_of?(Pathname) && source.instance_of?(Pathname)
-      options[:force] || shell.file_collision(destination) { File.binread(source) }
+      options[:force] || shell.file_collision(destination) { source }
     end
 
     def each_file(castle, basedir, subdirs)

spec/homesick_cli_spec.rb

@@ -331,6 +331,40 @@ describe Homesick::CLI do
         expect(home.join('.some_dotfile').readlink).to eq(dotfile)
       end
     end
+
+    context 'when call and some files conflict' do
+      it 'shows differences for conflicting text files' do
+        contents = {:castle => 'castle has new content', :home => 'home already has content'}
+
+        dotfile = castle.file('text')
+        File.open(dotfile.to_s, 'w') do |f|
+          f.write contents[:castle]
+        end
+        File.open(home.join('text').to_s, 'w') do |f|
+          f.write contents[:home]
+        end
+        message = Capture.stdout { homesick.shell.show_diff(home.join('text'), dotfile) }
+        expect(message.b).to match(/- ?#{contents[:home]}\n.*\+ ?#{contents[:castle]}$/m)
+      end
+      it 'shows message or differences for conflicting binary files' do
+        # content which contains NULL character, without any parentheses, braces, ...
+        contents = {:castle => (0..255).step(30).map{|e| e.chr}.join(), :home => (0..255).step(30).reverse_each.map{|e| e.chr}.join()}
+
+        dotfile = castle.file('binary')
+        File.open(dotfile.to_s, 'w') do |f|
+          f.write contents[:castle]
+        end
+        File.open(home.join('binary').to_s, 'w') do |f|
+          f.write contents[:home]
+        end
+        message = Capture.stdout { homesick.shell.show_diff(home.join('binary'), dotfile) }
+        if homesick.shell.is_a?(Thor::Shell::Color)
+          expect(message.b).to match(/- ?#{contents[:home]}\n.*\+ ?#{contents[:castle]}$/m)
+        elsif homesick.shell.is_a?(Thor::Shell::Basic)
+          expect(message.b).to match(/^Binary files .+ differ$/)
+        end
+      end
+    end
   end
 
   describe 'unlink' do