Diff::LCS.patch! does not change Array instance

[scooter-dangle]

If I understand the documentation correctly, when an Array instance a is extended with Diff::LCS, a.patch! a_diff should alter a when a_diff is non-trivial.

Example of Actual behavior:

irb(main):025:0> a, b = [:a, :b, :c], [:a, :b]
[[:a, :b, :c], [:a, :b]]

irb(main):026:0> a.extend Diff::LCS
[:a, :b, :c]

irb(main):027:0> b.extend Diff::LCS
[:a, :b]

irb(main):028:0> a_diff = Diff::LCS.diff a, b
[[#<Diff::LCS::Change:0xd1f0ad @element=:c, @position=2, @action="-">]]

irb(main):029:0> a.patch a_diff
[:a, :b]

irb(main):030:0> a
[:a, :b, :c]

irb(main):031:0> a.patch! a_diff
[:a, :b]

irb(main):032:0> a
[:a, :b, :c]

irb(main):033:0> a_diff2 = a.diff b
[[#<Diff::LCS::Change:0x2870b7 @element=:c, @position=2, @action="-">]]

irb(main):034:0> a
[:a, :b, :c]

irb(main):035:0> b
[:a, :b]

irb(main):036:0> a.patch a_diff2
[:a, :b]

irb(main):037:0> a
[:a, :b, :c]

irb(main):038:0> a.patch! a_diff2
[:a, :b]

irb(main):039:0> a
[:a, :b, :c]

irb(main):040:0> b
[:a, :b]

Shouldn't a == [:a, :b]?

Thanks,
Scott

[halostatue]

The documentation says that, but the code disagrees. :|

  def patch(src, patchset, direction = nil)
    # Normalize the patchset.
    has_changes, patchset = Diff::LCS::Internals.analyze_patchset(patchset)

    if not has_changes
      return src.dup if src.respond_to? :dup
      return src
    end

    string = src.kind_of?(String)
    # Start with a new empty type of the source's class
    res = src.class.new
    …
  end

I can make a change to the included method where if the class responds to #replace, it will replace the current value with the other, but I am more inclined to change the documentation because the underlying method Diff::LCS.patch! means "patch forward only". Changing this behaviour is a breaking change. Diff::LCS.(un)patch! should be renamed to Diff::LCS.force_(un)patch, and the includable module should have the same in parallel, that will allow #patch! and #unpatch! to be replace-in-place.

Since we always try to build a new version of the sequence we're comparing, we can't do a meaningful #patch! that reduces memory usage—I'm not sure that making this breaking change is what we should do or not.

What do you think?

[halostatue]

I thought about it and decided that I'm not going to change the behaviour of #patch! and #unpatch!, but will instead add #patch_me and #unpatch_me to do this. Committed in 2513a70.