Skip to content
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

Use rugged for diffing #124

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions diffy.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ Gem::Specification.new do |spec|
spec.test_files = spec.files.grep(%r{^(test|spec|features)/})
spec.require_paths = ["lib"]

spec.required_ruby_version = ">= 2.0.0"

spec.add_dependency "rugged", "~> 1.7"

spec.add_development_dependency "rake"
spec.add_development_dependency "rspec"
end
6 changes: 1 addition & 5 deletions lib/diffy.rb
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
require 'tempfile'
require 'erb'
require 'rbconfig'
require 'rugged'

module Diffy
WINDOWS = (RbConfig::CONFIG['host_os'] =~ /mswin|mingw|cygwin/)
end
require 'open3' unless Diffy::WINDOWS
require File.join(File.dirname(__FILE__), 'diffy', 'format')
require File.join(File.dirname(__FILE__), 'diffy', 'html_formatter')
require File.join(File.dirname(__FILE__), 'diffy', 'diff')
Expand Down
80 changes: 14 additions & 66 deletions lib/diffy/diff.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
module Diffy
class Diff
ORIGINAL_DEFAULT_OPTIONS = {
:diff => '-U10000',
:source => 'strings',
:include_diff_info => false,
:include_plus_and_minus_in_html => false,
:context => nil,
:context => 10_000,
:allow_empty_diff => true,
:rugged => {},
}

class << self
Expand Down Expand Up @@ -37,43 +37,30 @@ def initialize(string1, string2, options = {})
if ! ['strings', 'files'].include?(@options[:source])
raise ArgumentError, "Invalid :source option #{@options[:source].inspect}. Supported options are 'strings' and 'files'."
end
@options[:rugged][:context_lines] = @options[:context] if @options[:rugged][:context_lines].nil?
@string1, @string2 = string1, string2
@diff_empty = false
end

def diff
@diff ||= begin
@paths = case options[:source]
case options[:source]
when 'strings'
[tempfile(string1), tempfile(string2)]
diff = Rugged::Patch.from_strings(string1, string2, **options[:rugged]).to_s
when 'files'
[string1, string2]
diff = Rugged::Patch.from_strings(File.read(string1), File.read(string2), **options[:rugged]).to_s
end

diff, _stderr, _process_status = Open3.capture3(diff_bin, *(diff_options + @paths))
diff.force_encoding('ASCII-8BIT') if diff.respond_to?(:valid_encoding?) && !diff.valid_encoding?
if diff =~ /\A\s*\Z/ && !options[:allow_empty_diff]
@diff_empty = true
diff = case options[:source]
when 'strings' then string1
when 'files' then File.read(string1)
end.gsub(/^/, " ")
end
diff
end
ensure
# unlink the tempfiles explicitly now that the diff is generated
if defined? @tempfiles # to avoid Ruby warnings about undefined ins var.
Array(@tempfiles).each do |t|
begin
# check that the path is not nil and file still exists.
# REE seems to be very agressive with when it magically removes
# tempfiles
t.unlink if t.path && File.exist?(t.path)
rescue => e
warn "#{e.class}: #{e}"
warn e.backtrace.join("\n")
end
end
end
end

def each
Expand All @@ -82,11 +69,12 @@ def each
# this "primes" the diff and sets up the paths we'll reference below.
diff

# caching this regexp improves the performance of the loop by a
# considerable amount.
regexp = /^(--- "?#{@paths[0]}"?|\+\+\+ "?#{@paths[1]}"?|@@|\\\\)/

diff.split("\n").reject{|x| x =~ regexp }.map {|line| line + "\n" }
# diff --git a/file b/file
# index 71779d2..d5f7fc3 100644
# --- test/file
# +++ b/file
# @@ -1 +1 @@
diff.split("\n").drop(@diff_empty ? 0 : 5).map {|line| line + "\n" }

when true
diff.split("\n").map {|line| line + "\n" }
Expand Down Expand Up @@ -119,18 +107,6 @@ def each_chunk
end
end

def tempfile(string)
t = Tempfile.new('diffy')
# ensure tempfiles aren't unlinked when GC runs by maintaining a
# reference to them.
@tempfiles ||=[]
@tempfiles.push(t)
t.print(string)
t.flush
t.close
t.path
end

def to_s(format = nil)
format ||= self.class.default_format
formats = Format.instance_methods(false).map{|x| x.to_s}
Expand All @@ -143,33 +119,5 @@ def to_s(format = nil)
"Format #{format.inspect} not found in #{formats.inspect}"
end
end
private

@@bin = nil
def diff_bin
return @@bin if @@bin

if @@bin = ENV['DIFFY_DIFF']
# system() trick from Minitest
raise "Can't execute diff program '#@@bin'" unless system(@@bin, __FILE__, __FILE__)
return @@bin
end

diffs = ['diff', 'ldiff']
diffs.first << '.exe' if WINDOWS # ldiff does not have exe extension
@@bin = diffs.find { |name| system(name, __FILE__, __FILE__) }

if @@bin.nil?
raise "Can't find a diff executable in PATH #{ENV['PATH']}"
end

@@bin
end

# options pass to diff program
def diff_options
Array(options[:context] ? "-U#{options[:context]}" : options[:diff])
end

end
end
47 changes: 6 additions & 41 deletions spec/diffy_spec.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
require 'rspec'
require 'tempfile'
require File.expand_path(File.join(File.dirname(__FILE__), '..', 'lib', 'diffy'))

describe Diffy::Diff do
Expand Down Expand Up @@ -38,11 +39,8 @@ def tempfile(string, fn = 'diffy-spec')
DIFF
end

it "should accept file paths with spaces as arguments on windows" do
it "should accept file paths with spaces as arguments" do
begin

orig_verbose, $VERBOSE = $VERBOSE, nil #silence redefine constant warnings
orig_windows, Diffy::WINDOWS = Diffy::WINDOWS, true
string1 = "foo\nbar\nbang\n"
string2 = "foo\nbang\n"
path1, path2 = tempfile(string1, 'path with spaces'), tempfile(string2, 'path with spaces')
Expand All @@ -51,10 +49,7 @@ def tempfile(string, fn = 'diffy-spec')
-bar
bang
DIFF
ensure
Diffy::WINDOWS, $VERBOSE = orig_windows, orig_verbose
end

end

describe "with no line different" do
Expand Down Expand Up @@ -113,13 +108,6 @@ def tempfile(string, fn = 'diffy-spec')
end

describe "handling temp files" do
it "should unlink tempfiles after generating the diff" do
before_tmpfiles = Dir.entries(Dir.tmpdir)
::Diffy::Diff.new("a", "b").to_s
after_tmpfiles = Dir.entries(Dir.tmpdir)
expect(before_tmpfiles).to match_array(after_tmpfiles)
end

it "should still be able to generate multiple diffs" do
d = ::Diffy::Diff.new("a", "b")
expect(d.to_s).to be_a String
Expand Down Expand Up @@ -220,22 +208,13 @@ def tempfile(string, fn = 'diffy-spec')
end

describe "options[:diff]" do
it "should accept an option to diff" do
it "should accept options for rugged" do
@diff = Diffy::Diff.new(" foo\nbar\n", "foo\nbar\n", :diff => "-w", :allow_empty_diff => false)
expect(@diff.to_s).to eq <<-DIFF
foo
bar
DIFF
end

it "should accept multiple arguments to diff" do
@diff = Diffy::Diff.new(" foo\nbar\n", "foo\nbaz\n", :diff => ["-w", "-U 3"])
expect(@diff.to_s).to eq <<-DIFF
foo
-bar
+baz
DIFF
end
end

describe "#to_s" do
Expand Down Expand Up @@ -285,6 +264,7 @@ def tempfile(string, fn = 'diffy-spec')
Diffy::Diff.default_options = old_options.merge(:include_diff_info => true)
expect(Diffy::Diff.new(@string1, @string2).to_s).
to include('@@ -1,3 +1,2 @@')
ensure
Diffy::Diff.default_options = old_options
end

Expand Down Expand Up @@ -365,20 +345,6 @@ def tempfile(string, fn = 'diffy-spec')
</div>
HTML
end

it "should accept overrides to diff's options" do
@diff = Diffy::Diff.new(@string1, @string2, :diff => "--rcs")
expect(@diff.to_s).to eq <<-DIFF
d1 1
a1 3
one
two
three
d4 1
a4 1
baz
DIFF
end
end

describe "html" do
Expand Down Expand Up @@ -548,7 +514,7 @@ def tempfile(string, fn = 'diffy-spec')
it "should correctly do inline hightlighting when default diff options are changed" do
original_options = ::Diffy::Diff.default_options
begin
::Diffy::Diff.default_options.merge!(:diff => '-U0')
::Diffy::Diff.default_options.merge!(:context => 0)

s1 = "foo\nbar\nbang"
s2 = "foo\nbar\nbangleize"
Expand Down Expand Up @@ -621,7 +587,7 @@ def tempfile(string, fn = 'diffy-spec')

describe Diffy::SplitDiff do
before do
::Diffy::Diff.default_options.merge!(:diff => '-U10000')
::Diffy::Diff.default_options.merge!(:context => 10_000)
end

it "should fail with invalid format" do
Expand Down Expand Up @@ -691,4 +657,3 @@ def tempfile(string, fn = 'diffy-spec')
expect(Diffy::CSS).to include 'diff{overflow:auto;}'
end
end