simplify traverse_sequence method traversal logic

[tiendo1011]

• Use element directly instead of accessing it from the index
• The symmetrically makes it easier to understand

---

This change is 

[halostatue]

I had seen a different set of errors that I pointed out in #77, but this is what is failing locally with this branch:

❯ bundle exec rake
DEPRECATED: I want to drop this entirely. Let me know if you use this! from /Users/austin/.gem/ruby/2.7.4/gems/hoe-3.23.0/lib/hoe/test.rb:139:in `define_test_tasks'
/Users/austin/.rubies/ruby-2.7.4/bin/ruby -I/Users/austin/.gem/ruby/2.7.4/gems/rspec-core-3.10.1/lib:/Users/austin/.gem/ruby/2.7.4/gems/rspec-support-3.10.3/lib /Users/austin/.gem/ruby/2.7.4/gems/rspec-core-3.10.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb -Ispec:lib
Run options: exclude {:broken=>true}
......................................FF.................................................................................................................................................................................................................................................."called #finished_b"
."called #finished_b"
."called #finished_b"
."called #finished_b"
."called #finished_b"
.

Failures:

  1) Diff::LCS.diff correctly diffs seq1 to seq2
     Failure/Error: expect(change_diff(correct_forward_diff)).to eq(diff_s1_s2)

       expected: [[#<Diff::LCS::Change: ["-", 0, "a"]>], [#<Diff::LCS::Change: ["+", 2, "d"]>], [#<Diff::LCS::Change: ...Change: ["-", 9, "p"]>, #<Diff::LCS::Change: ["+", 10, "s"]>, #<Diff::LCS::Change: ["+", 11, "t"]>]]
            got: [[#<Diff::LCS::Change: ["-", 0, "a"]>], [#<Diff::LCS::Change: ["+", 2, "d"]>], [#<Diff::LCS::Change: ...Change: ["+", 9, "r"]>, #<Diff::LCS::Change: ["+", 10, "s"]>, #<Diff::LCS::Change: ["+", 11, "t"]>]]

       (compared using ==)
     # ./spec/diff_spec.rb:10:in `block (2 levels) in <top (required)>'

  2) Diff::LCS.diff correctly diffs seq2 to seq1
     Failure/Error: expect(change_diff(correct_backward_diff)).to eq(diff_s2_s1)

       expected: [[#<Diff::LCS::Change: ["+", 0, "a"]>], [#<Diff::LCS::Change: ["-", 2, "d"]>], [#<Diff::LCS::Change: ...Change: ["-", 10, "s"]>, #<Diff::LCS::Change: ["+", 9, "p"]>, #<Diff::LCS::Change: ["-", 11, "t"]>]]
            got: [[#<Diff::LCS::Change: ["+", 0, "a"]>], [#<Diff::LCS::Change: ["-", 2, "d"]>], [#<Diff::LCS::Change: ...:Change: ["+", 8, "n"]>, #<Diff::LCS::Change: ["-", 11, "t"]>, #<Diff::LCS::Change: ["+", 9, "p"]>]]

       (compared using ==)
     # ./spec/diff_spec.rb:15:in `block (2 levels) in <top (required)>'

Finished in 1.86 seconds (files took 0.10589 seconds to load)
287 examples, 2 failures

Failed examples:

rspec ./spec/diff_spec.rb:8 # Diff::LCS.diff correctly diffs seq1 to seq2
rspec ./spec/diff_spec.rb:13 # Diff::LCS.diff correctly diffs seq2 to seq1

/Users/austin/.rubies/ruby-2.7.4/bin/ruby -I/Users/austin/.gem/ruby/2.7.4/gems/rspec-core-3.10.1/lib:/Users/austin/.gem/ruby/2.7.4/gems/rspec-support-3.10.3/lib /Users/austin/.gem/ruby/2.7.4/gems/rspec-core-3.10.1/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb -Ispec:lib failed

I’m not entirely sure what the issue is with these two and why they’re failing on my local box and not in CI.

[halostatue]

The two tests above are fixed if we do the following:

diff --git i/lib/diff/lcs.rb w/lib/diff/lcs.rb
index 64f46c957de3..bd43fa76492b 100644
--- i/lib/diff/lcs.rb
+++ w/lib/diff/lcs.rb
@@ -323,6 +323,18 @@ def traverse_sequences(seq1, seq2, callbacks = Diff::LCS::SequenceCallbacks) #:y

         bj += 1
       end
+
+      ai += 1
+    end
+
+    unless seq1[ai].nil?
+      ax = string ? seq1[ai, 1] : seq1[ai]
+      bx = string ? seq2[bj, 1] : seq2[bj]
+
+      event = Diff::LCS::ContextChange.new('-', ai, ax, bj, bx)
+      event = yield event if block_given?
+      callbacks.discard_a(event)
+
       ai += 1
     end

But that makes the changes present in traverse_sequence_spec.rb fail, reintroducing the transitiveness bug.

[halostatue]

It may be that the two failing unit tests are wrong, but I would expect to see those fail in CI.

[halostatue]

I can see why the tests are succeeding in CI, and it’s because we’re not running the correct command.

Please apply this patch:

diff --git i/Rakefile w/Rakefile
index d65f2eac8bbf..2ef1da0cc4ec 100644
--- i/Rakefile
+++ w/Rakefile
@@ -90,6 +90,7 @@ RSpec::Core::RakeTask.new(:spec) do |t|
   t.rspec_opts << "-I#{rspec_dirs.join(":")}" unless rspec_dirs.empty?
 end
 task :default => :spec
+task :test => :spec

 if RUBY_VERSION >= '2.0' && RUBY_ENGINE == 'ruby'
   namespace :spec do

We could change the workflow file to use rake spec instead of rake test, but rake test is so ingrained into my typing that I frequently have this issue.

[halostatue]

The above patch to the Rakefile has been applied to master, so you can rebase and get it for free.

[tiendo1011]

Since the cause of some failed tests is from the previous PR, I'll try to address it first, to bring master to its expected state, before proceeding with this PR.

Here is my take on why it's failed, I take the courtesy and continue our discussion in #77, which seems more appropriate for the purpose :).

[tiendo1011]

Closing due to this