Reintroduce Diff::LCS::Change#to_ary

- This required some level of code remediation in the main library, but all tests now pass: - Patchsets are now internally flattened one level explicitly, rather than using Array#flatten. This ensures that only the outer patchset array is flattened. Fixes #48.
halostatue · Feb 2, 2019 · a798f6f · a798f6f
1 parent 45ea1b3
commit a798f6f
Show file tree

Hide file tree

Showing 8 changed files with 116 additions and 62 deletions.
diff --git a/Contributing.md b/Contributing.md
@@ -77,6 +77,7 @@ Thanks to everyone else who has contributed to Diff::LCS:
 *   Josef Strzibny
 *   Josh Bronson
 *   Mark Friedgan
+*   Akinori MUSHA
 
 [Rspec]: http://rspec.info/documentation/
 [quality commit messages]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html

diff --git a/History.md b/History.md
@@ -1,23 +1,27 @@
 ## 1.NEXT / 2019-MM-DD
 
-*   Ruby versions lower than 2.3 are soft-deprecated and will not be run as part
-    of the CI process any longer.
+*   Ruby versions lower than 2.3 are soft-deprecated and will not be run as
+    part of the CI process any longer.
+*   Akinora MUSHA (knu) added the ability for Diff::LCS::Change objects to be
+    implicitly treated arrays. Originally provided as pull request [#47][],
+    but it introduced a number of test failures as documented in [#48][], and
+    remediation of Diff::LCS itself was introduced in [#49][].
 
 ## 1.3 / 2017-01-18
 
 *   Bugs fixed:
 
-    *   Fixed an error for bin/ldiff --version. Fixes [issue #21][].
+    *   Fixed an error for bin/ldiff --version. Fixes issue [#21][].
     *   Force Diff::LCS::Change and Diff::LCS::ContextChange to only perform
         equality comparisons against themselves. Provided by Kevin Mook in
-        [pull request #29][].
+        pull request [#29][].
     *   Fix tab expansion in htmldiff, provided by Mark Friedgan in
-        [pull request #25][].
-    *   Silence Ruby 2.4 Fixnum deprecation warnings. Fixxues [issue #38][] and
+        pull request [#25][].
+    *   Silence Ruby 2.4 Fixnum deprecation warnings. Fixxues issue [#38][] and
         [pull request#36][].
-    *   Ensure that test dependencies are loaded properly. Fixes [issue #33][]
-        and [pull request #34][].
-    *   Fix [issue #1][] with incorrect intuition of patch direction. Tentative
+    *   Ensure that test dependencies are loaded properly. Fixes issue [#33][]
+        and pull request [#34][].
+    *   Fix issue [#1][] with incorrect intuition of patch direction. Tentative
         fix, but the previous failure cases pass now.
 
 *   Tooling changes:
@@ -36,15 +40,15 @@
 *   Bugs fixed:
 
     *   Comparing arrays flattened them too far, especially with
-        Diff::LCS.sdiff. Fixed by Josh Bronson in [pull request #23][].
+        Diff::LCS.sdiff. Fixed by Josh Bronson in pull request [#23][].
 
 ## 1.2.4 / 2013-04-20
 
 *   Bugs fixed:
 
     *   A bug was introduced after 1.1.3 when pruning common sequences at the
         start of comparison. Paul Kunysch (@pck) fixed this in
-        [pull request #18][]. Thanks!
+        pull request [#18][]. Thanks!
 
     *   The Rubinius (1.9 mode) bug in [rubinius/rubinius#2268][] has been
         fixed by the Rubinius team two days after it was filed. Thanks for
@@ -76,7 +80,7 @@
         sets that are not US-ASCII-compatible because of the use of literal
         regular expressions and strings. Jon Rowe found this in
         [rspec/rspec-expectations#219][] and provided a first pass
-        implementation in [pull request #15][]. I've reworked it because of
+        implementation in pull request [#15][]. I've reworked it because of
         test failures in Rubinius when running in Ruby 1.9 mode. This coerces
         the added values to the encoding of the old dataset (as determined by
         the first piece of the old dataset).
@@ -106,30 +110,30 @@
 
 *   Bugs Fixed:
 
-    *   Fixed [issue #1][] patch direction detection.
-    *   Resolved [issue #2][] by handling `string[string.size, 1]` properly (it
+    *   Fixed issue [#1][] patch direction detection.
+    *   Resolved issue [#2][] by handling `string[string.size, 1]` properly (it
         returns `""` not `nil`).
     *   Michael Granger (ged) fixed an implementation error in
-        Diff::LCS::Change and added specs in [pull request #8][]. Thanks!
+        Diff::LCS::Change and added specs in pull request [#8][]. Thanks!
     *   Made the code auto-testable.
     *   Vít Ondruch (voxik) provided the latest version of the GPL2 license
-        file in [pull request #10][]. Thanks!
+        file in pull request [#10][]. Thanks!
     *   Fixed a documentation issue with the includable versions of #patch! and
         #unpatch! where they implied that they would replace the original
         value. Given that Diff::LCS.patch always returns a copy, the
         documentation was incorrect and has been corrected. To provide the
         behaviour that was originally documented, two new methods were added to
-        provide this behaviour. Found by scooter-dangle in [issue #12][].
+        provide this behaviour. Found by scooter-dangle in issue [#12][].
         Thanks!
 
 *   Code Style Changes:
 
     *   Removed trailing spaces.
     *   Calling class methods using `.` instead of `::`.
-    *   Vít Ondruch (voxik) removed unnecessary shebangs in [pull request #9][].
+    *   Vít Ondruch (voxik) removed unnecessary shebangs in pull request [#9][].
         Thanks!
     *   Kenichi Kamiya (kachick) removed some warnings of an unused variable in
-        lucky [pull request #13][]. Thanks!
+        lucky pull request [#13][]. Thanks!
     *   Embarked on a major refactoring to make the files a little more
         manageable and understand the code on a deeper level.
     *   Adding to http://travis-ci.org.
@@ -144,8 +148,8 @@
 *   Bugs fixed:
 
     *   Eliminated the explicit use of RubyGems in both bin/htmldiff and
-        bin/ldiff. Resolves [issue #4][].
-    *   Eliminated Ruby warnings. Resolves [issue #3][].
+        bin/ldiff. Resolves issue [#4][].
+    *   Eliminated Ruby warnings. Resolves issue [#3][].
 
 ## 1.1.2 / 2004-10-20
 
@@ -204,22 +208,25 @@
 [rspec/rspec-expectations#219]: https://github.com/rspec/rspec-expectations/issues/219
 [rspec/rspec-expectations@3d6fc82c]: https://github.com/rspec/rspec-expectations/commit/3d6fc82c
 [rspec/rspec-expectations#200]: https://github.com/rspec/rspec-expectations/pull/200
-[pull request #36]: https://github.com/halostatue/diff-lcs/pull/36
-[pull request #34]: https://github.com/halostatue/diff-lcs/pull/34
-[pull request #29]: https://github.com/halostatue/diff-lcs/pull/29
-[pull request #25]: https://github.com/halostatue/diff-lcs/pull/25
-[pull request #23]: https://github.com/halostatue/diff-lcs/pull/23
-[pull request #18]: https://github.com/halostatue/diff-lcs/pull/18
-[pull request #15]: https://github.com/halostatue/diff-lcs/pull/15
-[pull request #13]: https://github.com/halostatue/diff-lcs/pull/13
-[pull request #10]: https://github.com/halostatue/diff-lcs/pull/10
-[pull request #9]: https://github.com/halostatue/diff-lcs/pull/9
-[pull request #8]: https://github.com/halostatue/diff-lcs/pull/8
-[issue #38]: https://github.com/halostatue/diff-lcs/issues/38
-[issue #33]: https://github.com/halostatue/diff-lcs/issues/33
-[issue #21]: https://github.com/halostatue/diff-lcs/issues/21
-[issue #12]: https://github.com/halostatue/diff-lcs/issues/12
-[issue #4]: https://github.com/halostatue/diff-lcs/issues/4
-[issue #3]: https://github.com/halostatue/diff-lcs/issues/3
-[issue #2]: https://github.com/halostatue/diff-lcs/issues/2
-[issue #1]: https://github.com/halostatue/diff-lcs/issues/1
+[#1]: https://github.com/halostatue/diff-lcs/issues/1
+[#2]: https://github.com/halostatue/diff-lcs/issues/2
+[#3]: https://github.com/halostatue/diff-lcs/issues/3
+[#4]: https://github.com/halostatue/diff-lcs/issues/4
+[#8]: https://github.com/halostatue/diff-lcs/pull/8
+[#9]: https://github.com/halostatue/diff-lcs/pull/9
+[#10]: https://github.com/halostatue/diff-lcs/pull/10
+[#12]: https://github.com/halostatue/diff-lcs/issues/12
+[#13]: https://github.com/halostatue/diff-lcs/pull/13
+[#15]: https://github.com/halostatue/diff-lcs/pull/15
+[#18]: https://github.com/halostatue/diff-lcs/pull/18
+[#21]: https://github.com/halostatue/diff-lcs/issues/21
+[#23]: https://github.com/halostatue/diff-lcs/pull/23
+[#25]: https://github.com/halostatue/diff-lcs/pull/25
+[#29]: https://github.com/halostatue/diff-lcs/pull/29
+[#33]: https://github.com/halostatue/diff-lcs/issues/33
+[#34]: https://github.com/halostatue/diff-lcs/pull/34
+[#36]: https://github.com/halostatue/diff-lcs/pull/36
+[#38]: https://github.com/halostatue/diff-lcs/issues/38
+[#47]: https://github.com/halostatue/diff-lcs/pull/47
+[#48]: https://github.com/halostatue/diff-lcs/issues/48
+[#49]: https://github.com/halostatue/diff-lcs/pull/49
diff --git a/README.rdoc b/README.rdoc
@@ -13,10 +13,14 @@ Diff::LCS computes the difference between two Enumerable sequences using the
 McIlroy-Hunt longest common subsequence (LCS) algorithm. It includes utilities
 to create a simple HTML diff output format and a standard diff-like tool.
 
-This is release 1.3, providing a tentative fix to a long-standing issue related
-to incorrect detection of a patch direction. Also modernizes the gem
-infrastructure, testing infrastructure, and provides a warning-free experience
-to Ruby 2.4 users.
+This is release 1.4, providing a simple extension that allows for
+Diff::LCS::Change objects to be treated implicitly as arrays. Ruby versions
+below 2.3 are soft-deprecated.
+
+This means that older versions are no longer part of the CI test suite. If any
+changes have been introduced that break those versions, bug reports and patches
+will be accepted, but it will be up to the reporter to verify any fixes prior
+to release. A future release will completely break compatibility.
 
 == Synopsis
 

diff --git a/diff-lcs.gemspec b/diff-lcs.gemspec
@@ -1,19 +1,19 @@
 # -*- encoding: utf-8 -*-
-# stub: diff-lcs 1.3 ruby lib
+# stub: diff-lcs 1.4 ruby lib
 
 Gem::Specification.new do |s|
   s.name = "diff-lcs".freeze
-  s.version = "1.3"
+  s.version = "1.4"
 
   s.required_rubygems_version = Gem::Requirement.new(">= 0".freeze) if s.respond_to? :required_rubygems_version=
   s.require_paths = ["lib".freeze]
   s.authors = ["Austin Ziegler".freeze]
   s.date = "2019-01-28"
-  s.description = "Diff::LCS computes the difference between two Enumerable sequences using the\nMcIlroy-Hunt longest common subsequence (LCS) algorithm. It includes utilities\nto create a simple HTML diff output format and a standard diff-like tool.\n\nThis is release 1.3, providing a tentative fix to a long-standing issue related\nto incorrect detection of a patch direction. Also modernizes the gem\ninfrastructure, testing infrastructure, and provides a warning-free experience\nto Ruby 2.4 users.".freeze
+  s.description = "Diff::LCS computes the difference between two Enumerable sequences using the\nMcIlroy-Hunt longest common subsequence (LCS) algorithm. It includes utilities\nto create a simple HTML diff output format and a standard diff-like tool.\n\nThis is release 1.4, providing a simple extension that allows for\nDiff::LCS::Change objects to be treated implicitly as arrays. Ruby versions\nbelow 2.3 are soft-deprecated.\n\nThis means that older versions are no longer part of the CI test suite. If any\nchanges have been introduced that break those versions, bug reports and patches\nwill be accepted, but it will be up to the reporter to verify any fixes prior\nto release. A future release will completely break compatibility.".freeze
   s.email = ["halostatue@gmail.com".freeze]
   s.executables = ["htmldiff".freeze, "ldiff".freeze]
   s.extra_rdoc_files = ["Code-of-Conduct.md".freeze, "Contributing.md".freeze, "History.md".freeze, "License.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "docs/COPYING.txt".freeze, "docs/artistic.txt".freeze]
-  s.files = [".rspec".freeze, "Code-of-Conduct.md".freeze, "Contributing.md".freeze, "History.md".freeze, "License.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "Rakefile".freeze, "autotest/discover.rb".freeze, "bin/htmldiff".freeze, "bin/ldiff".freeze, "docs/COPYING.txt".freeze, "docs/artistic.txt".freeze, "lib/diff-lcs.rb".freeze, "lib/diff/lcs.rb".freeze, "lib/diff/lcs/array.rb".freeze, "lib/diff/lcs/block.rb".freeze, "lib/diff/lcs/callbacks.rb".freeze, "lib/diff/lcs/change.rb".freeze, "lib/diff/lcs/htmldiff.rb".freeze, "lib/diff/lcs/hunk.rb".freeze, "lib/diff/lcs/internals.rb".freeze, "lib/diff/lcs/ldiff.rb".freeze, "lib/diff/lcs/string.rb".freeze, "spec/change_spec.rb".freeze, "spec/diff_spec.rb".freeze, "spec/fixtures/ds1.csv".freeze, "spec/fixtures/ds2.csv".freeze, "spec/hunk_spec.rb".freeze, "spec/issues_spec.rb".freeze, "spec/lcs_spec.rb".freeze, "spec/ldiff_spec.rb".freeze, "spec/patch_spec.rb".freeze, "spec/sdiff_spec.rb".freeze, "spec/spec_helper.rb".freeze, "spec/traverse_balanced_spec.rb".freeze, "spec/traverse_sequences_spec.rb".freeze]
+  s.files = [".rspec".freeze, "Code-of-Conduct.md".freeze, "Contributing.md".freeze, "History.md".freeze, "License.md".freeze, "Manifest.txt".freeze, "README.rdoc".freeze, "Rakefile".freeze, "autotest/discover.rb".freeze, "bin/htmldiff".freeze, "bin/ldiff".freeze, "docs/COPYING.txt".freeze, "docs/artistic.txt".freeze, "lib/diff-lcs.rb".freeze, "lib/diff/lcs.rb".freeze, "lib/diff/lcs/array.rb".freeze, "lib/diff/lcs/backports.rb".freeze, "lib/diff/lcs/block.rb".freeze, "lib/diff/lcs/callbacks.rb".freeze, "lib/diff/lcs/change.rb".freeze, "lib/diff/lcs/htmldiff.rb".freeze, "lib/diff/lcs/hunk.rb".freeze, "lib/diff/lcs/internals.rb".freeze, "lib/diff/lcs/ldiff.rb".freeze, "lib/diff/lcs/string.rb".freeze, "spec/change_spec.rb".freeze, "spec/diff_spec.rb".freeze, "spec/fixtures/ds1.csv".freeze, "spec/fixtures/ds2.csv".freeze, "spec/hunk_spec.rb".freeze, "spec/issues_spec.rb".freeze, "spec/lcs_spec.rb".freeze, "spec/ldiff_spec.rb".freeze, "spec/patch_spec.rb".freeze, "spec/sdiff_spec.rb".freeze, "spec/spec_helper.rb".freeze, "spec/traverse_balanced_spec.rb".freeze, "spec/traverse_sequences_spec.rb".freeze]
   s.homepage = "https://github.com/halostatue/diff-lcs".freeze
   s.licenses = ["MIT".freeze, "Artistic-2.0".freeze, "GPL-2.0+".freeze]
   s.rdoc_options = ["--main".freeze, "README.rdoc".freeze]

diff --git a/lib/diff/lcs.rb b/lib/diff/lcs.rb
@@ -50,7 +50,7 @@ module Diff; end unless defined? Diff # rubocop:disable Style/Documentation
 #          a x b y c z p d q
 #    a b c a x b y c z
 module Diff::LCS
-  VERSION = '1.3'
+  VERSION = '1.4'
 end
 
 require 'diff/lcs/callbacks'
@@ -181,6 +181,20 @@ def diff(seq1, seq2, callbacks = nil, &block) # :yields diff changes:
   # Class argument is provided for +callbacks+, #diff will attempt to
   # initialise it. If the +callbacks+ object (possibly initialised) responds
   # to #finish, it will be called.
+  #
+  # Each element of a returned array is a Diff::LCS::ContextChange object,
+  # which can be implicitly converted to an array.
+  #
+  #   Diff::LCS.sdiff(a, b).each do |action, (old_pos, old_element), (new_pos, new_element)|
+  #     case action
+  #     when '!'
+  #       # replace
+  #     when '-'
+  #       # delete
+  #     when '+'
+  #       # insert
+  #     end
+  #   end
   def sdiff(seq1, seq2, callbacks = nil, &block) #:yields diff changes:
     diff_traversal(:sdiff, seq1, seq2, callbacks || Diff::LCS::SDiffCallbacks, &block)
   end
@@ -623,7 +637,7 @@ def patch(src, patchset, direction = nil)
 
     patch_map = PATCH_MAP[direction]
 
-    patchset.flatten.each do |change|
+    patchset.each do |change|
       # Both Change and ContextChange support #action
       action = patch_map[change.action]
 

diff --git a/lib/diff/lcs/change.rb b/lib/diff/lcs/change.rb
@@ -39,6 +39,8 @@ def to_a
     [@action, @position, @element]
   end
 
+  alias to_ary to_a
+
   def self.from_a(arr)
     arr = arr.flatten(1)
     case arr.size
@@ -125,6 +127,8 @@ def to_a
     ]
   end
 
+  alias to_ary to_a
+
   def self.from_a(arr)
     Diff::LCS::Change.from_a(arr)
   end

diff --git a/lib/diff/lcs/internals.rb b/lib/diff/lcs/internals.rb
@@ -91,14 +91,15 @@ def lcs(a, b)
     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.
+  # 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)
     fail 'Patchset too complex' if depth > 1
 
     has_changes = false
+    new_patchset = []
 
     # Format:
     # [ # patchset
@@ -108,29 +109,28 @@ def analyze_patchset(patchset, depth = 0)
     #   ]
     # ]
 
-    patchset = patchset.map { |hunk|
+    patchset.each do |hunk|
       case hunk
       when Diff::LCS::Change
         has_changes ||= !hunk.unchanged?
-        hunk
+        new_patchset << hunk
       when Array
-        # Detect if the 'hunk' is actually an array-format
-        # Change object.
+        # 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
+          new_patchset << hunk
         else
           with_changes, hunk = analyze_patchset(hunk, depth + 1)
           has_changes ||= with_changes
-          hunk.flatten
+          new_patchset.concat(hunk)
         end
       else
         fail ArgumentError, "Cannot normalise a hunk of class #{hunk.class}."
       end
-    }
+    end
 
-    [has_changes, patchset.flatten(1)]
+    [has_changes, new_patchset]
   end
 
   # Examine the patchset and the source to see in which direction the

diff --git a/spec/change_spec.rb b/spec/change_spec.rb
@@ -62,4 +62,28 @@
     it { should_not be_finished_a }
     it { should     be_finished_b }
   end
+
+  describe 'as array' do
+    it 'should be converted' do
+      action, position, element = described_class.new('!', 0, 'element')
+      expect(action).to eq '!'
+      expect(position).to eq 0
+      expect(element).to eq 'element'
+    end
+  end
+end
+
+describe Diff::LCS::ContextChange do
+  describe 'as array' do
+    it 'should be converted' do
+      action, (old_position, old_element), (new_position, new_element) =
+        described_class.new('!', 1, 'old_element', 2, 'new_element')
+
+      expect(action).to eq '!'
+      expect(old_position).to eq 1
+      expect(old_element).to eq 'old_element'
+      expect(new_position).to eq 2
+      expect(new_element).to eq 'new_element'
+    end
+  end
 end