Starting on a substantial refactoring to fix bugs.

1. All Diff::LCS::__* methods are now moved to Diff::LCS::Internals and
   named without the underscores.
2. Fixed the documentation for Diff::LCS::patch. (Some formatting may
   have been broken.)
3. Eliminated normalize_patchset in favour of the (substantially
   similar) analyze_patchset. This allows for a single-pass over a
   patchset to determine whether the patchset contains changes and
   transform it to the object-based format.
4. Made Diff::LCS::ContextChange descend from Diff::LCS::Change. This
   will probably change so that both descend from something like a
   Diff::LCS::AbstractChange because of some issues that may arise with
   case statement order.

2 files changed, 380 insertions, 100 deletions

diff --git a/lib/diff/lcs/change.rb b/lib/diff/lcs/change.rb
index 5139821..50c1de9 100644
--- a/lib/diff/lcs/change.rb
+++ b/lib/diff/lcs/change.rb
@@ -1,59 +1,59 @@
-#! /usr/env/bin ruby
-#--
-# Copyright 2004 Austin Ziegler <diff-lcs@halostatue.ca>
-#   adapted from:
-#     Algorithm::Diff (Perl) by Ned Konz <perl@bike-nomad.com>
-#     Smalltalk by Mario I. Wolczko <mario@wolczko.com>
-#   implements McIlroy-Hunt diff algorithm
-#
-# This program is free software. It may be redistributed and/or modified under
-# the terms of the GPL version 2 (or later), the Perl Artistic licence, or the
-# Ruby licence.
-# 
-# $Id$
-#++
+# -*- ruby encoding: utf-8 -*-
 # Provides Diff::LCS::Change and Diff::LCS::ContextChange.
 
-  # Centralises the change test code in Diff::LCS::Change and
-  # Diff::LCS::ContextChange, since it's the same for both classes.
-module Diff::LCS::ChangeTypeTests
-  def deleting?
-    @action == '-'
-  end
+# Represents a simplistic (non-contextual) change. Represents the removal or
+# addition of an element from either the old or the new sequenced
+# enumerable.
+class Diff::LCS::Change
+  # The only actions valid for changes are '+' (add), '-' (delete), '='
+  # (no change), '!' (changed), '<' (tail changes from first sequence), or
+  # '>' (tail changes from second sequence). The last two ('<>') are only
+  # found with Diff::LCS::diff and Diff::LCS::sdiff.
+  VALID_ACTIONS = %W(+ - = ! > <)
 
-  def adding?
-    @action == '+'
+  def self.valid_action?(action)
+    VALID_ACTIONS.include? action
   end
 
-  def unchanged?
-    @action == '='
-  end
+  # Returns the action this Change represents.
+  attr_reader :action
 
-  def changed?
-    @action == '!'
+  # Returns the position of the Change.
+  attr_reader :position
+  # Returns the sequence element of the Change.
+  attr_reader :element
+
+  def initialize(*args)
+    @action, @position, @element = *args
+
+    unless Diff::LCS::Change.valid_action?(@action)
+      raise "Invalid Change Action '#{@action}'"
+    end
+    raise "Invalid Position Type" unless @position.kind_of? Fixnum
   end
 
-  def finished_a?
-    @action == '>'
+  def inspect
+    %Q(#<#{self.class.name}:#{__id__.to_s(16)} @action=#{action} position=#{position} element=#{element.inspect})
   end
 
-  def finished_b?
-    @action == '<'
+  def to_a
+    [ @action, @position, @element ]
   end
-end
 
-  # Represents a simplistic (non-contextual) change. Represents the removal or
-  # addition of an element from either the old or the new sequenced enumerable.
-class Diff::LCS::Change
-    # Returns the action this Change represents. Can be '+' (#adding?), '-'
-    # (#deleting?), '=' (#unchanged?), # or '!' (#changed?). When created by
-    # Diff::LCS#diff or Diff::LCS#sdiff, it may also be '>' (#finished_a?) or
-    # '<' (#finished_b?).
-  attr_reader :action
-  attr_reader :position
-  attr_reader :element
+  def self.from_a(arr)
+    arr = arr.flatten
+    case arr.size
+    when 5
+      Diff::LCS::ContextChange.new(*(arr[0...5]))
+    when 3
+      Diff::LCS::Change.new(*(arr[0...3]))
+    else
+      raise "Invalid change array format provided."
+    end
+  end
 
   include Comparable
+
   def ==(other)
     (self.action == other.action) and
     (self.position == other.position) and
@@ -67,85 +67,79 @@ class Diff::LCS::Change
     r
   end
 
-  def initialize(action, position, element)
-    @action = action
-    @position = position
-    @element = element
+  def adding?
+    @action == '+'
   end
 
-    # Creates a Change from an array produced by Change#to_a.
-  def to_a
-    [@action, @position, @element]
+  def deleting?
+    @action == '-'
   end
 
-  def self.from_a(arr)
-    Diff::LCS::Change.new(arr[0], arr[1], arr[2])
+  def unchanged?
+    @action == '='
+  end
+
+  def changed?
+    @action == '!'
   end
 
-  include Diff::LCS::ChangeTypeTests
+  def finished_a?
+    @action == '>'
+  end
+
+  def finished_b?
+    @action == '<'
+  end
 end
 
-  # Represents a contextual change. Contains the position and values of the
-  # elements in the old and the new sequenced enumerables as well as the action
-  # taken.
-class Diff::LCS::ContextChange
-    # Returns the action this Change represents. Can be '+' (#adding?), '-'
-    # (#deleting?), '=' (#unchanged?), # or '!' (#changed?). When
-    # created by Diff::LCS#diff or Diff::LCS#sdiff, it may also be '>'
-    # (#finished_a?) or '<' (#finished_b?).
-  attr_reader :action
+# Represents a contextual change. Contains the position and values of the
+# elements in the old and the new sequenced enumerables as well as the action
+# taken.
+class Diff::LCS::ContextChange < Diff::LCS::Change
+  # We don't need these two values.
+  undef :position
+  undef :element
+
+  # Returns the old position being changed.
   attr_reader :old_position
-  attr_reader :old_element
+  # Returns the new position being changed.
   attr_reader :new_position
+  # Returns the old element being changed.
+  attr_reader :old_element
+  # Returns the new element being changed.
   attr_reader :new_element
 
-  include Comparable
+  def initialize(*args)
+    @action, @old_position, @old_element, @new_position, @new_element = *args
 
-  def ==(other)
-    (@action == other.action) and
-    (@old_position == other.old_position) and
-    (@new_position == other.new_position) and
-    (@old_element == other.old_element) and
-    (@new_element == other.new_element)
-  end
-
-  def inspect(*args)
-    %Q(#<#{self.class.name}:#{__id__} @action=#{action} positions=#{old_position},#{new_position} elements=#{old_element.inspect},#{new_element.inspect}>)
-  end
-
-  def <=>(other)
-    r = @action <=> other.action
-    r = @old_position <=> other.old_position if r.zero?
-    r = @new_position <=> other.new_position if r.zero?
-    r = @old_element <=> other.old_element if r.zero?
-    r = @new_element <=> other.new_element if r.zero?
-    r
+    unless Diff::LCS::Change.valid_action?(@action)
+      raise "Invalid Change Action '#{@action}'"
+    end
+    unless @old_position.nil? or @old_position.kind_of? Fixnum
+      raise "Invalid (Old) Position Type"
+    end
+    unless @new_position.nil? or @new_position.kind_of? Fixnum
+      raise "Invalid (New) Position Type"
+    end
   end
 
-  def initialize(action, old_position, old_element, new_position, new_element)
-    @action = action
-    @old_position = old_position
-    @old_element = old_element
-    @new_position = new_position
-    @new_element = new_element
+  def to_a
+    [ @action,
+      [ @old_position, @old_element ],
+      [ @new_position, @new_element ]
+    ]
   end
 
-  def to_a
-    [@action, [@old_position, @old_element], [@new_position, @new_element]]
+  def inspect(*args)
+    %Q(#<#{self.class.name}:#{__id__} @action=#{action} positions=#{old_position},#{new_position} elements=#{old_element.inspect},#{new_element.inspect}>)
   end
 
-    # Creates a ContextChange from an array produced by ContextChange#to_a.
   def self.from_a(arr)
-    if arr.size == 5
-      Diff::LCS::ContextChange.new(arr[0], arr[1], arr[2], arr[3], arr[4])
-    else
-      Diff::LCS::ContextChange.new(arr[0], arr[1][0], arr[1][1], arr[2][0],
-                                   arr[2][1])
-    end
+    Diff::LCS::Change.from_a(arr)
   end
 
-    # Simplifies a context change for use in some diff callbacks. '<' actions
-    # are converted to '-' and '>' actions are converted to '+'. 
+  # Simplifies a context change for use in some diff callbacks. '<' actions
+  # are converted to '-' and '>' actions are converted to '+'.
   def self.simplify(event)
     ea = event.to_a
 
@@ -165,5 +159,20 @@ class Diff::LCS::ContextChange
     Diff::LCS::ContextChange.from_a(ea)
   end
 
-  include Diff::LCS::ChangeTypeTests
+  def ==(other)
+    (@action == other.action) and
+    (@old_position == other.old_position) and
+    (@new_position == other.new_position) and
+    (@old_element == other.old_element) and
+    (@new_element == other.new_element)
+  end
+
+  def <=>(other)
+    r = @action <=> other.action
+    r = @old_position <=> other.old_position if r.zero?
+    r = @new_position <=> other.new_position if r.zero?
+    r = @old_element <=> other.old_element if r.zero?
+    r = @new_element <=> other.new_element if r.zero?
+    r
+  end
 end
diff --git a/lib/diff/lcs/internals.rb b/lib/diff/lcs/internals.rb
new file mode 100644
index 0000000..373a18a
--- /dev/null
+++ b/lib/diff/lcs/internals.rb
@@ -0,0 +1,271 @@
+# -*- ruby encoding: utf-8 -*-
+
+module Diff::LCS::Internals # :nodoc:
+  class << self
+    # Compute the longest common subsequence between the sequenced
+    # Enumerables +a+ and +b+. The result is an array whose contents is such
+    # that
+    #
+    #     result = Diff::LCS::Internals.lcs(a, b)
+    #     result.each_with_index do |e, ii|
+    #       assert_equal(a[ii], b[e]) unless e.nil?
+    #     end
+    def lcs(a, b)
+      a_start = b_start = 0
+      a_finish = a.size - 1
+      b_finish = b.size - 1
+      vector = []
+
+      # Prune off any common elements at the beginning...
+      while (a_start <= a_finish) and
+        (b_start <= b_finish) and
+        (a[a_start] == b[b_start])
+        vector[a_start] = b_start
+        a_start += 1
+        b_start += 1
+      end
+
+      # Now the end...
+      while (a_start <= a_finish) and
+        (b_start <= b_finish) and
+        (a[a_finish] == b[b_finish])
+        vector[a_finish] = b_finish
+        a_finish -= 1
+        b_finish -= 1
+      end
+
+      # Now, compute the equivalence classes of positions of elements.
+      b_matches = position_hash(b, b_start .. b_finish)
+
+      thresh = []
+      links = []
+
+      (a_start .. a_finish).each do |ii|
+        ai = a.kind_of?(String) ? a[ii, 1] : a[ii]
+        bm = b_matches[ai]
+        kk = nil
+        bm.reverse_each do |jj|
+          if kk and (thresh[kk] > jj) and (thresh[kk - 1] < jj)
+            thresh[kk] = jj
+          else
+            kk = replace_next_larger(thresh, jj, kk)
+          end
+          links[kk] = [ (kk > 0) ? links[kk - 1] : nil, ii, jj ] unless kk.nil?
+        end
+      end
+
+      unless thresh.empty?
+        link = links[thresh.size - 1]
+        while not link.nil?
+          vector[link[1]] = link[2]
+          link = link[0]
+        end
+      end
+
+      vector
+    end
+
+    # This method will analyze the provided patchset to provide a
+    # single-pass normalization (conversion of the array form of
+    # Diff::LCS::Change objects to the object form of same) and detection of
+    # whether the patchset represents changes to be made.
+    def analyze_patchset(patchset, depth = 0)
+      raise "Patchset too complex" if depth > 1
+
+      has_changes = false
+
+      # Format:
+      # [ # patchset
+      #   # hunk (change)
+      #   [ # hunk
+      #     # change
+      #   ]
+      # ]
+
+      patchset = patchset.map do |hunk|
+        case hunk
+        when Diff::LCS::Change
+          has_changes ||= !hunk.unchanged?
+          hunk
+        when Array
+          # Detect if the 'hunk' is actually an array-format
+          # Change object.
+          if Diff::LCS::Change.valid_action? hunk[0]
+            hunk = Diff::LCS::Change.from_a(hunk)
+            has_changes ||= !hunk.unchanged?
+            hunk
+          else
+            with_changes, hunk = analyze_patchset(hunk, depth + 1)
+            has_changes ||= with_changes
+            hunk
+          end
+        else
+          raise ArgumentError, "Cannot normalise a hunk of class #{hunk.class}."
+        end
+      end
+
+      [ has_changes, patchset ]
+    end
+
+    # Find the place at which +value+ would normally be inserted into the
+    # Enumerable. If that place is already occupied by +value+, do nothing
+    # and return +nil+. If the place does not exist (i.e., it is off the end
+    # of the Enumerable), add it to the end. Otherwise, replace the element
+    # at that point with +value+. It is assumed that the Enumerable's values
+    # are numeric.
+    #
+    # This operation preserves the sort order.
+    def replace_next_larger(enum, value, last_index = nil)
+        # Off the end?
+      if enum.empty? or (value > enum[-1])
+        enum << value
+        return enum.size - 1
+      end
+
+        # Binary search for the insertion point
+      last_index ||= enum.size
+      first_index = 0
+      while (first_index <= last_index)
+        ii = (first_index + last_index) >> 1
+
+        found = enum[ii]
+
+        if value == found
+          return nil
+        elsif value > found
+          first_index = ii + 1
+        else
+          last_index = ii - 1
+        end
+      end
+
+        # The insertion point is in first_index; overwrite the next larger
+        # value.
+      enum[first_index] = value
+      return first_index
+    end
+
+    # If +vector+ maps the matching elements of another collection onto this
+    # Enumerable, compute the inverse +vector+ that maps this Enumerable
+    # onto the collection. (Currently unused.)
+    def inverse_vector(a, vector)
+      inverse = a.dup
+      (0 ... vector.size).each do |ii|
+        inverse[vector[ii]] = ii unless vector[ii].nil?
+      end
+      inverse
+    end
+
+    # Returns a hash mapping each element of an Enumerable to the set of
+    # positions it occupies in the Enumerable, optionally restricted to the
+    # elements specified in the range of indexes specified by +interval+.
+    def position_hash(enum, interval = 0 .. -1)
+      hash = Hash.new { |hh, kk| hh[kk] = [] }
+      interval.each do |ii|
+        kk = enum.kind_of?(String) ? enum[ii, 1] : enum[ii]
+        hash[kk] << ii
+      end
+      hash
+    end
+
+    # Examine the patchset and the source to see in which direction the
+    # patch should be applied.
+    #
+    # WARNING: By default, this examines the whole patch, so this could take
+    # some time. This also works better with Diff::LCS::ContextChange or
+    # Diff::LCS::Change as its source, as an array will cause the creation
+    # of one of the above.
+    #
+    # Note: This will be deprecated as a public function in a future release.
+    def diff_direction(src, patchset, limit = nil)
+      count = left_match = left_miss = right_match = right_miss = 0
+      string = src.kind_of?(String)
+
+      patchset.each do |change|
+        count += 1
+
+        case change
+        when Diff::LCS::Change
+          # With a simplistic change, we can't tell the difference between
+          # the left and right on '!' actions, so we ignore those. On '='
+          # actions, if there's a miss, we miss both left and right.
+          element = string ? src[change.position, 1] : src[change.position]
+
+          case change.action
+          when '-'
+            if element == change.element
+              left_match += 1
+            else
+              left_miss += 1
+            end
+          when '+'
+            if element == change.element
+              right_match += 1
+            else
+              right_miss += 1
+            end
+          when '='
+            if element != change.element
+              left_miss += 1
+              right_miss += 1
+            end
+          end
+        when Diff::LCS::ContextChange
+          case change.action
+          when '-' # Remove details from the old string
+            element = string ? src[change.old_position, 1] : src[change.old_position]
+
+            if element == change.old_element
+              left_match += 1
+            else
+              left_miss += 1
+            end
+          when '+'
+            element = string ? src[change.new_position, 1] : src[change.new_position]
+            if element == change.new_element
+              right_match += 1
+            else
+              right_miss += 1
+            end
+          when '='
+            le = string ? src[change.old_position, 1] : src[change.old_position]
+            re = string ? src[change.new_position, 1] : src[change.new_position]
+
+            left_miss += 1 if le != change.old_element
+            right_miss += 1 if re != change.new_element
+          when '!'
+            element = string ? src[change.old_position, 1] : src[change.old_position]
+            if element == change.old_element
+              left_match += 1
+            else
+              element = string ? src[change.new_position, 1] : src[change.new_position]
+              if element == change.new_element
+                right_match += 1
+              else
+                left_miss += 1
+                right_miss += 1
+              end
+            end
+          end
+        end
+
+        break if (not limit.nil?) && (count > limit)
+      end
+
+      if left_match.zero?
+      end
+
+      no_left = (left_match == 0) && (left_miss >= 0)
+      no_right = (right_match == 0) && (right_miss >= 0)
+
+      direction = case [no_left, 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."
+                  end
+    end
+  end
+end