Fixed Issue #1 patch direction detection.

This bug has been filed for five and a half years and I have finally
fixed it. By ghu it feels good.

3 files changed, 62 insertions, 54 deletions

diff --git a/lib/diff/lcs/internals.rb b/lib/diff/lcs/internals.rb
index 94d6c83..59c8582 100644
--- a/lib/diff/lcs/internals.rb
+++ b/lib/diff/lcs/internals.rb
@@ -133,7 +133,9 @@ class << Diff::LCS::Internals
     end
 
     def inspect
-        "#{count}: L#{left_match}:#{left_miss} R#{right_match}:#{right_miss}"
+      l = "L#{left_match}:#{left_miss} (#{(left_match == 0) && left_miss > 0})"
+      r = "R#{right_match}:#{right_miss} (#{(right_match == 0) && right_miss > 0})"
+      "#{l} #{r}"
     end
   end
 
@@ -147,48 +149,48 @@ class << Diff::LCS::Internals
   #
   # Note: This will be deprecated as a public function in a future release.
   def diff_direction(src, patchset, limit = nil)
-    ddc = DiffDirectionCounter.new
-    right_match = right_miss = 0
+    fwd_ddc = DiffDirectionCounter.new
+
     string = src.kind_of?(String)
+    count = 0
 
     patchset.each do |change|
-      ddc.count!
+      count += 1
 
       case change
       when Diff::LCS::ContextChange
+        le = string ? src[change.old_position, 1] : src[change.old_position]
+        re = string ? src[change.new_position, 1] : src[change.new_position]
+
         case change.action
         when '-' # Remove details from the old string
-          element = string ? src[change.old_position, 1] : src[change.old_position]
+          if le == change.old_element
+            fwd_ddc.left_match!
+          else
+            fwd_ddc.left_miss!
+          end
 
-          if element == change.old_element
-            ddc.left_match!
+          if re == change.old_element
           else
-            ddc.left_miss!
           end
         when '+'
-          element = string ? src[change.new_position, 1] : src[change.new_position]
-          if element == change.new_element
-            ddc.right_match!
+          if re == change.new_element
+            fwd_ddc.right_match!
           else
-            ddc.right_miss!
+            fwd_ddc.right_miss!
           end
         when '='
-          le = string ? src[change.old_position, 1] : src[change.old_position]
-          re = string ? src[change.new_position, 1] : src[change.new_position]
-
-          ddc.left_miss! if le != change.old_element
-          ddc.right_miss! if re != change.new_element
+          fwd_ddc.left_miss! if le != change.old_element
+          fwd_ddc.right_miss! if re != change.new_element
         when '!'
-          element = string ? src[change.old_position, 1] : src[change.old_position]
-          if element == change.old_element
-            ddc.left_match!
+          if le == change.old_element
+            fwd_ddc.left_match!
           else
-            element = string ? src[change.new_position, 1] : src[change.new_position]
-            if element == change.new_element
-              ddc.right_match!
+            if re == change.new_element
+              fwd_ddc.right_match!
             else
-              ddc.left_miss!
-              ddc.right_miss!
+              fwd_ddc.left_miss!
+              fwd_ddc.right_miss!
             end
           end
         end
@@ -201,42 +203,44 @@ class << Diff::LCS::Internals
         case change.action
         when '-'
           if element == change.element
-            ddc.left_match!
+            fwd_ddc.left_match!
           else
-            ddc.left_miss!
+            fwd_ddc.left_miss!
           end
         when '+'
           if element == change.element
-            ddc.right_match!
+            fwd_ddc.right_match!
           else
-            ddc.right_miss!
+            fwd_ddc.right_miss!
           end
         when '='
           if element != change.element
-            ddc.left_miss!
-            ddc.right_miss!
+            fwd_ddc.left_miss!
+            fwd_ddc.right_miss!
           end
         end
       end
 
-      break if (not limit.nil?) && (ddc.count > limit)
+      break if (not limit.nil?) && (count > limit)
     end
 
-    if ddc.left_match.zero?
-    end
-
-    no_left = (ddc.left_match.zero?) && (ddc.left_miss >= 0)
-    no_right = (ddc.right_match.zero?) && (ddc.right_miss >= 0)
-
-    p ddc, no_left, no_right
+    fwd_no_left = (fwd_ddc.left_match == 0) && (fwd_ddc.left_miss > 0)
+    fwd_no_right = (fwd_ddc.right_match == 0) && (fwd_ddc.right_miss > 0)
 
-    case [no_left, no_right]
+    case [fwd_no_left, fwd_no_right]
     when [false, true]
       :patch
     when [true, false]
       :unpatch
     else
-      raise "The provided patchset does not appear to apply to the provided value as either source or destination value."
+      case fwd_ddc.left_match <=> fwd_ddc.right_match
+      when 1
+        :patch
+      when -1
+        :unpatch
+      else
+        raise "The provided patchset does not appear to apply to the provided value as either source or destination value."
+      end
     end
   end
 
diff --git a/spec/issues_spec.rb b/spec/issues_spec.rb
index 1b12825..c3d8f87 100644
--- a/spec/issues_spec.rb
+++ b/spec/issues_spec.rb
@@ -5,7 +5,7 @@ require 'spec_helper'
 describe "Diff::LCS Issues" do
   include Diff::LCS::SpecHelper::Matchers
 
-  it "should not fail to provide a simple patchset (issue 1)", :broken => true do
+  it "should not fail to provide a simple patchset (issue 1)" do
     s1, s2 = *%W(aX bXaX)
     correct_forward_diff = [
       [ [ '+', 0, 'b' ],
@@ -17,5 +17,8 @@ describe "Diff::LCS Issues" do
     expect do
       Diff::LCS.patch(s1, diff_s1_s2).should == s2
     end.to_not raise_error(RuntimeError, /provided patchset/)
+    expect do
+      Diff::LCS.patch(s2, diff_s1_s2).should == s1
+    end.to_not raise_error(RuntimeError, /provided patchset/)
   end
 end
diff --git a/spec/patch_spec.rb b/spec/patch_spec.rb
index 7172aea..83d2c31 100644
--- a/spec/patch_spec.rb
+++ b/spec/patch_spec.rb
@@ -189,13 +189,13 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.diff(@s2, @s1)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
@@ -227,13 +227,13 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.diff(@s2, @s1, Diff::LCS::ContextDiffCallbacks)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
@@ -265,13 +265,13 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.diff(@s2, @s1, Diff::LCS::SDiffCallbacks)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
@@ -303,13 +303,13 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.sdiff(@s2, @s1)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
@@ -341,13 +341,13 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.sdiff(@s2, @s1, Diff::LCS::ContextDiffCallbacks)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
@@ -379,13 +379,14 @@ describe "Diff::LCS.patch" do
         @patch_set_s2_s1 = Diff::LCS.sdiff(@s2, @s1, Diff::LCS::DiffCallbacks)
       end
 
-      it "should autodiscover s1 to s2 patches", :broken => true do
+      it "should autodiscover s1 to s2 patches" do
         expect do
           Diff::LCS.patch(@s1, @patch_set_s1_s2).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)
       end
 
-      it "should autodiscover s2 to s1 patches", :broken => true do
+      it "should autodiscover s2 to s1 patches" do
+p @s2, @s1, @patch_set_s2_s1
         expect do
           Diff::LCS.patch(@s1, @patch_set_s2_s1).should == @s2
         end.to_not raise_error(RuntimeError, /provided patchset/)