diff --git a/.rubocop.yml b/.rubocop.yml index 9768185..2ee932a 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -7,7 +7,7 @@ AllCops: - diff-lcs.gemspec - research/**/* -Layout/AlignParameters: +Layout/ParameterAlignment: EnforcedStyle: with_fixed_indentation Layout/DotPosition: @@ -20,7 +20,7 @@ Layout/ExtraSpacing: Layout/MultilineMethodCallIndentation: EnforcedStyle: indented -Metrics/LineLength: +Layout/LineLength: Max: 110 Naming/FileName: @@ -30,17 +30,15 @@ Naming/FileName: Naming/MemoizedInstanceVariableName: Exclude: [] -Naming/UncommunicativeMethodParamName: +Naming/MethodParameterName: Exclude: - lib/diff/lcs/internals.rb + - lib/diff/lcs/hunk.rb - spec/spec_helper.rb Naming/VariableNumber: Exclude: [] -Performance/Caller: - Exclude: [] - Security/MarshalLoad: Exclude: [] @@ -91,7 +89,7 @@ Style/RescueStandardError: Style/SignalException: EnforcedStyle: semantic -Layout/IndentHeredoc: { Enabled: false } +Layout/HeredocIndentation: { Enabled: false } Metrics/AbcSize: { Enabled: false } Metrics/BlockLength: { Enabled: false } Metrics/BlockNesting: { Enabled: false } @@ -119,3 +117,17 @@ Style/SpecialGlobalVars: { Enabled: false } Style/SymbolArray: { Enabled: false } Style/SymbolProc: { Enabled: false } Style/WordArray: { Enabled: false } + +Layout/EmptyLinesAroundAttributeAccessor: { Enabled: true } +Layout/SpaceAroundMethodCallOperator: { Enabled: true } +Lint/DeprecatedOpenSSLConstant: { Enabled: true } +Lint/MixedRegexpCaptureTypes: { Enabled: true } +Lint/RaiseException: { Enabled: true } +Lint/StructNewOverride: { Enabled: true } +Style/ExponentialNotation: { Enabled: true } +Style/HashEachMethods: { Enabled: true } +Style/HashTransformKeys: { Enabled: true } +Style/HashTransformValues: { Enabled: true } +Style/RedundantRegexpCharacterClass: { Enabled: true } +Style/RedundantRegexpEscape: { Enabled: true } +Style/SlicingWithRange: { Enabled: true } diff --git a/Contributing.md b/Contributing.md index 08f424b..7ec1f52 100644 --- a/Contributing.md +++ b/Contributing.md @@ -1,32 +1,35 @@ ## Contributing -I value any contribution to Diff::LCS you can provide: a bug report, a feature -request, or code contributions. Code contributions to Diff::LCS are especially -welcomeencouraged. Because Diff::LCS is a complex codebase, there -are a few guidelines: - -* Code changes *will not* be accepted without tests. The test suite is - written with [RSpec][]. -* Match my coding style. -* Use a thoughtfully-named topic branch that contains your change. Rebase - your commits into logical chunks as necessary. -* Use [quality commit messages][]. -* Do not change the version number; when your patch is accepted and a release - is made, the version will be updated at that point. -* Submit a GitHub pull request with your changes. -* New or changed behaviours require appropriate documentation. +I value any contribution to Diff::LCS you can provide: a bug report, a +feature request, or code contributions. Code contributions to Diff::LCS are +especially welcomeencouraged. Because Diff::LCS is a complex +codebase, there are a few guidelines: + +- Code changes _will not_ be accepted without tests. The test suite is + written with [RSpec][]. +- Match my coding style. +- Use a thoughtfully-named topic branch that contains your change. Rebase + your commits into logical chunks as necessary. +- Use [quality commit messages][]. +- Do not change the version number; when your patch is accepted and a release + is made, the version will be updated at that point. +- Submit a GitHub pull request with your changes. +- New or changed behaviours require appropriate documentation. ### Test Dependencies -Diff::LCS uses Ryan Davis’s [Hoe][] to manage the release process, and it adds -a number of rake tasks. You will mostly be interested in: +Diff::LCS uses Ryan Davis’s [Hoe][] to manage the release process, and it +adds a number of rake tasks. You will mostly be interested in: - $ rake +```sh +$ rake +``` which runs the tests the same way that: - $ rake spec - $ rake travis +```sh +$ rake spec +``` will do. @@ -34,51 +37,82 @@ To assist with the installation of the development dependencies, I have provided a Gemfile pointing to the (generated) `diff-lcs.gemspec` file. This will permit you to do: - $ bundle install +```sh +$ bundle install +``` to get the development dependencies. If you aleady have `hoe` installed, you can accomplish the same thing with: - $ rake newb +```sh +$ rake newb +``` This task will install any missing dependencies, run the tests/specs, and generate the RDoc. You can run tests with code coverage analysis by running: - $ rake spec:coverage +```sh +$ rake spec:coverage +``` ### Workflow Here's the most direct way to get your work merged into the project: -* Fork the project. -* Clone down your fork (`git clone git://github.com//diff-lcs.git`). -* Create a topic branch to contain your change (`git checkout -b - my_awesome_feature`). -* Hack away, add tests. Not necessarily in that order. -* Make sure everything still passes by running `rake`. -* If necessary, rebase your commits into logical chunks, without errors. -* Push the branch up (`git push origin my_awesome_feature`). -* Create a pull request against halostatue/diff-lcs and describe what your - change does and the why you think it should be merged. +- Fork the project. +- Clone down your fork (`git clone git://github.com//diff-lcs.git`). +- Create a topic branch to contain your change (`git checkout -b my_awesome_feature`). +- Hack away, add tests. Not necessarily in that order. +- Make sure everything still passes by running `rake`. +- If necessary, rebase your commits into logical chunks, without errors. +- Push the branch up (`git push origin my_awesome_feature`). +- Create a pull request against halostatue/diff-lcs and describe what your + change does and the why you think it should be merged. ### Contributors -* Austin Ziegler created Diff::LCS. - -Thanks to everyone else who has contributed to Diff::LCS: - -* Kenichi Kamiya -* Michael Granger -* Vít Ondruch -* Jon Rowe -* Koichi Ito -* Josef Strzibny -* Josh Bronson -* Mark Friedgan -* Akinori MUSHA - -[Rspec]: http://rspec.info/documentation/ +- Austin Ziegler created Diff::LCS. + +Thanks to everyone else who has contributed code or bug reports to Diff::LCS: + +- @ginriki +- @joshbronson +- @kevinmook +- @mckaz +- Akinori Musha +- Artem Ignatyev +- Brandon Fish +- Camille Drapier +- Cédric Boutillier +- Gregg Kellogg +- Jagdeep Singh +- Jason Gladish +- Jon Rowe +- Josef Strzibny +- Josep (@apuratepp) +- Josh Bronson +- Jun Aruga +- Kenichi Kamiya +- Kensuke Nagae +- Kevin Ansfield +- Koichi Ito +- Mark Friedgan +- Michael Granger +- Myron Marston +- Nicolas Leger +- Oleg Orlov +- Paul Kunysch +- Pete Higgins +- Peter Wagenet +- Philippe Lafoucrière +- Ryan Lovelett +- Scott Steele +- Simon Courtois +- Tomas Jura +- Vít Ondruch + +[rspec]: http://rspec.info/documentation/ [quality commit messages]: http://tbaggery.com/2008/04/19/a-note-about-git-commit-messages.html -[Hoe]: https://github.com/seattlerb/hoe +[hoe]: https://github.com/seattlerb/hoe diff --git a/Gemfile b/Gemfile index db504a7..7b9f817 100644 --- a/Gemfile +++ b/Gemfile @@ -6,14 +6,15 @@ source 'https://rubygems.org/' if RUBY_VERSION < '1.9' + gem 'hoe', '~> 3.20' gem 'rake', '< 11' gem 'rdoc', '< 4' - gem 'hoe', '~> 3.20' gem 'ruby-debug' elsif RUBY_VERSION >= '2.0' if RUBY_ENGINE == 'ruby' gem 'simplecov', '~> 0.18' + gem 'byebug' end end diff --git a/History.md b/History.md index ff24b50..063387b 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,42 @@ # History +## 1.4.4 / 2020-07-01 + +- Fixed an issue reported by Jun Aruga in the Diff::LCS::Ldiff binary text + detection. [#44][] +- Fixed a theoretical issue reported by Jun Aruga in Diff::LCS::Hunk to raise + a more useful exception. [#43][] +- Added documentation that should address custom object issues as reported in + [#35][]. + +- Fixed more diff errors, in part reported in [#65][]. + + - The use of `Numeric#abs` is incorrect in `Diff::LCS::Block#diff_size`. + The diff size _must_ be accurate for correct change placement. + - When selecting @max_diff_size in Diff::LCS::Hunk, choose it based on + `block.diff_size.abs`. + - Made a number of changes that will, unfortunately, increase allocations + at the cost of being safe with frozen strings. + - Add some knowledge that when `Diff::LCS::Hunk#diff` is called, that we + are processing the _last_ hunk, so some changes will be made to how the + output is generated. + + - `old`, `ed`, and `reverse_ed` formats have no differences. + - `unified` format will report `\ No newline at end of file` given the + correct conditions, at most once. Unified range reporting also + differs for the last hunk such that the `length` of the range is + reduced by one. + - `context` format will report `\No newline at end of file` given the + correct conditions, up to once per "file". Context range reporting also + differs for the last hunk such that the `end` part of the range is + reduced by one to a minimum of one. + +- Added a bunch more tests for the cases above, and fixed `hunk_spec.rb` so + that the phrase being compared isn't nonsense French. + +- Updated formatting. +- Added a Rake task to assist with manual testing on Ruby 1.8. + ## 1.4.3 / 2020-06-29 - Fixed several issues with the 1.4 on Rubies older than 2.0. Some of this was @@ -263,8 +300,11 @@ [#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 +[#35]: https://github.com/halostatue/diff-lcs/issues/35 [#36]: https://github.com/halostatue/diff-lcs/pull/36 [#38]: https://github.com/halostatue/diff-lcs/issues/38 +[#43]: https://github.com/halostatue/diff-lcs/issues/43 +[#44]: https://github.com/halostatue/diff-lcs/issues/44 [#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 @@ -276,3 +316,4 @@ [#60]: https://github.com/halostatue/diff-lcs/issues/60 [#61]: https://github.com/halostatue/diff-lcs/pull/61 [#63]: https://github.com/halostatue/diff-lcs/issues/63 +[#65]: https://github.com/halostatue/diff-lcs/issues/65 diff --git a/Rakefile b/Rakefile index 7beb3b6..afd9307 100644 --- a/Rakefile +++ b/Rakefile @@ -10,18 +10,19 @@ Hoe.plugin :gemspec2 Hoe.plugin :git if RUBY_VERSION < '1.9' - class Array + class Array #:nodoc: def to_h - Hash[*self.flatten(1)] + Hash[*flatten(1)] end end - class Gem::Specification - def metadata=(*) - end + class Gem::Specification #:nodoc: + def metadata=(*); end + + def default_value(*); end end - class Object + class Object #:nodoc: def caller_locations(*) [] end @@ -55,3 +56,19 @@ if RUBY_VERSION >= '2.0' && RUBY_ENGINE == 'ruby' end end end + +task :ruby18 do + puts <<-MESSAGE +You are starting a barebones Ruby 1.8 docker environment. You will need to +do the following: + +- mv Gemfile.lock{,.v2} +- gem install bundler --version 1.17.2 --no-ri --no-rdoc +- ruby -S bundle +- rake + +Don't forget to restore your Gemfile.lock after testing. + + MESSAGE + sh "docker run -it --rm -v #{Dir.pwd}:/root/diff-lcs bellbind/docker-ruby18-rails2 bash -l" +end diff --git a/diff-lcs.gemspec b/diff-lcs.gemspec index 03f9738..565ba13 100644 --- a/diff-lcs.gemspec +++ b/diff-lcs.gemspec @@ -1,16 +1,16 @@ # -*- encoding: utf-8 -*- -# stub: diff-lcs 1.4.3 ruby lib +# stub: diff-lcs 1.4.4 ruby lib Gem::Specification.new do |s| s.name = "diff-lcs".freeze - s.version = "1.4.3" + s.version = "1.4.4" s.required_rubygems_version = Gem::Requirement.new(">= 0".freeze) if s.respond_to? :required_rubygems_version= s.metadata = { "bug_tracker_uri" => "https://github.com/halostatue/diff-lcs/issues", "homepage_uri" => "https://github.com/halostatue/diff-lcs", "source_code_uri" => "https://github.com/halostatue/diff-lcs" } if s.respond_to? :metadata= s.require_paths = ["lib".freeze] s.authors = ["Austin Ziegler".freeze] - s.date = "2020-06-29" - 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.5 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.date = "2020-07-01" + 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.3, providing a simple extension that allows for\nDiff::LCS::Change objects to be treated implicitly as arrays and fixes a\nnumber of formatting issues.\n\nRuby versions below 2.5 are soft-deprecated, which means that older versions\nare no longer part of the CI test suite. If any changes have been introduced\nthat break those versions, bug reports and patches will be accepted, but it\nwill be up to the reporter to verify any fixes prior to release. The next\nmajor 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] diff --git a/lib/diff/lcs.rb b/lib/diff/lcs.rb index 9d47064..63888a1 100644 --- a/lib/diff/lcs.rb +++ b/lib/diff/lcs.rb @@ -49,7 +49,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.4.3' + VERSION = '1.4.4' end require 'diff/lcs/callbacks' @@ -60,6 +60,13 @@ module Diff::LCS # rubocop:disable Style/Documentation # +self+ and +other+. See Diff::LCS#lcs. # # lcs = seq1.lcs(seq2) + # + # A note when using objects: Diff::LCS only works properly when each object + # can be used as a key in a Hash, which typically means that the objects must + # implement Object#eql? in a way that two identical values compare + # identically for key purposes. That is: + # + # O.new('a').eql?(O.new('a')) == true def lcs(other, &block) #:yields self[i] if there are matched subsequences: Diff::LCS.lcs(self, other, &block) end diff --git a/lib/diff/lcs/block.rb b/lib/diff/lcs/block.rb index fe86793..430702d 100644 --- a/lib/diff/lcs/block.rb +++ b/lib/diff/lcs/block.rb @@ -19,7 +19,7 @@ def initialize(chunk) end def diff_size - (@insert.size - @remove.size).abs + @insert.size - @remove.size end def op diff --git a/lib/diff/lcs/hunk.rb b/lib/diff/lcs/hunk.rb index d884a1b..49b520e 100644 --- a/lib/diff/lcs/hunk.rb +++ b/lib/diff/lcs/hunk.rb @@ -6,26 +6,38 @@ # each block. (So if we're not using context, every hunk will contain one # block.) Used in the diff program (bin/ldiff). class Diff::LCS::Hunk + OLD_DIFF_OP_ACTION = { '+' => 'a', '-' => 'd', '!' => 'c' }.freeze #:nodoc: + ED_DIFF_OP_ACTION = { '+' => 'a', '-' => 'd', '!' => 'c' }.freeze #:nodoc: + + private_constant :OLD_DIFF_OP_ACTION, :ED_DIFF_OP_ACTION if respond_to?(:private_constant) + # Create a hunk using references to both the old and new data, as well as the # piece of data. def initialize(data_old, data_new, piece, flag_context, file_length_difference) # At first, a hunk will have just one Block in it @blocks = [Diff::LCS::Block.new(piece)] + + if @blocks[0].remove.empty? && @blocks[0].insert.empty? + fail "Cannot build a hunk from #{piece.inspect}; has no add or remove actions" + end + if String.method_defined?(:encoding) @preferred_data_encoding = data_old.fetch(0, data_new.fetch(0, '')).encoding end + @data_old = data_old @data_new = data_new before = after = file_length_difference after += @blocks[0].diff_size @file_length_difference = after # The caller must get this manually - @max_diff_size = @blocks.map { |e| e.diff_size }.max + @max_diff_size = @blocks.map { |e| e.diff_size.abs }.max + # Save the start & end of each array. If the array doesn't exist (e.g., - # we're only adding items in this block), then figure out the line - # number based on the line number of the other file and the current - # difference in file lengths. + # we're only adding items in this block), then figure out the line number + # based on the line number of the other file and the current difference in + # file lengths. if @blocks[0].remove.empty? a1 = a2 = nil else @@ -55,7 +67,7 @@ def initialize(data_old, data_new, piece, flag_context, file_length_difference) # Change the "start" and "end" fields to note that context should be added # to this hunk. - attr_accessor :flag_context + attr_accessor :flag_context # rubocop:disable Layout/EmptyLinesAroundAttributeAccessor undef :flag_context= def flag_context=(context) #:nodoc: # rubocop:disable Lint/DuplicateMethods return if context.nil? or context.zero? @@ -101,18 +113,18 @@ def overlaps?(hunk) end # Returns a diff string based on a format. - def diff(format) + def diff(format, last = false) case format when :old - old_diff + old_diff(last) when :unified - unified_diff + unified_diff(last) when :context - context_diff + context_diff(last) when :ed self when :reverse_ed, :ed_finish - ed_diff(format) + ed_diff(format, last) else fail "Unknown diff format #{format}." end @@ -120,35 +132,34 @@ def diff(format) # Note that an old diff can't have any context. Therefore, we know that # there's only one block in the hunk. - def old_diff + def old_diff(_last = false) warn 'Expecting only one block in an old diff hunk!' if @blocks.size > 1 - op_act = { '+' => 'a', '-' => 'd', '!' => 'c' } block = @blocks[0] # Calculate item number range. Old diff range is just like a context # diff range, except the ranges are on one line with the action between # them. - s = encode("#{context_range(:old)}#{op_act[block.op]}#{context_range(:new)}\n") + s = encode("#{context_range(:old, ',')}#{OLD_DIFF_OP_ACTION[block.op]}#{context_range(:new, ',')}\n") # If removing anything, just print out all the remove lines in the hunk # which is just all the remove lines in the block. unless block.remove.empty? - @data_old[@start_old..@end_old].each { |e| s << encode('< ') + e + encode("\n") } + @data_old[@start_old..@end_old].each { |e| s << encode('< ') + e.chomp + encode("\n") } end s << encode("---\n") if block.op == '!' unless block.insert.empty? - @data_new[@start_new..@end_new].each { |e| s << encode('> ') + e + encode("\n") } + @data_new[@start_new..@end_new].each { |e| s << encode('> ') + e.chomp + encode("\n") } end s end private :old_diff - def unified_diff + def unified_diff(last = false) # Calculate item number range. - s = encode("@@ -#{unified_range(:old)} +#{unified_range(:new)} @@\n") + s = encode("@@ -#{unified_range(:old, last)} +#{unified_range(:new, last)} @@\n") # Outlist starts containing the hunk of the old file. Removing an item # just means putting a '-' in front of it. Inserting an item requires @@ -161,7 +172,14 @@ def unified_diff # file -- don't take removed items into account. lo, hi, num_added, num_removed = @start_old, @end_old, 0, 0 - outlist = @data_old[lo..hi].map { |e| e.insert(0, encode(' ')) } + outlist = @data_old[lo..hi].map { |e| String.new("#{encode(' ')}#{e.chomp}") } + + last_block = blocks[-1] + + if last + old_missing_newline = missing_last_newline?(@data_old) + new_missing_newline = missing_last_newline?(@data_new) + end @blocks.each do |block| block.remove.each do |item| @@ -170,67 +188,100 @@ def unified_diff outlist[offset][0, 1] = encode(op) num_removed += 1 end + + if last && block == last_block && old_missing_newline && !new_missing_newline + outlist << encode('\\ No newline at end of file') + num_removed += 1 + end + block.insert.each do |item| op = item.action.to_s # + offset = item.position - @start_new + num_removed - outlist[offset, 0] = encode(op) + @data_new[item.position] + outlist[offset, 0] = encode(op) + @data_new[item.position].chomp num_added += 1 end end + outlist << encode('\\ No newline at end of file') if last && new_missing_newline + s << outlist.join(encode("\n")) + + s end private :unified_diff - def context_diff + def context_diff(last = false) s = encode("***************\n") - s << encode("*** #{context_range(:old)} ****\n") - r = context_range(:new) + s << encode("*** #{context_range(:old, ',', last)} ****\n") + r = context_range(:new, ',', last) + + if last + old_missing_newline = missing_last_newline?(@data_old) + new_missing_newline = missing_last_newline?(@data_new) + end # Print out file 1 part for each block in context diff format if there # are any blocks that remove items lo, hi = @start_old, @end_old removes = @blocks.reject { |e| e.remove.empty? } - if removes - outlist = @data_old[lo..hi].map { |e| e.insert(0, encode(' ')) } + + unless removes.empty? + outlist = @data_old[lo..hi].map { |e| String.new("#{encode(' ')}#{e.chomp}") } + + last_block = removes[-1] removes.each do |block| block.remove.each do |item| - outlist[item.position - lo].insert(0, encode(block.op)) # - or ! + outlist[item.position - lo][0, 1] = encode(block.op) # - or ! + end + + if last && block == last_block && old_missing_newline + outlist << encode('\\ No newline at end of file') end end - s << outlist.join("\n") + + s << outlist.join(encode("\n")) << encode("\n") end - s << encode("\n--- #{r} ----\n") + s << encode("--- #{r} ----\n") lo, hi = @start_new, @end_new inserts = @blocks.reject { |e| e.insert.empty? } - if inserts - outlist = @data_new[lo..hi].collect { |e| e.insert(0, encode(' ')) } + + unless inserts.empty? + outlist = @data_new[lo..hi].map { |e| String.new("#{encode(' ')}#{e.chomp}") } + + last_block = inserts[-1] + inserts.each do |block| block.insert.each do |item| - outlist[item.position - lo].insert(0, encode(block.op)) # - or ! + outlist[item.position - lo][0, 1] = encode(block.op) # + or ! + end + + if last && block == last_block && new_missing_newline + outlist << encode('\\ No newline at end of file') end end - s << outlist.join("\n") + s << outlist.join(encode("\n")) end + s end private :context_diff - def ed_diff(format) - op_act = { '+' => 'a', '-' => 'd', '!' => 'c' } + def ed_diff(format, _last = false) warn 'Expecting only one block in an old diff hunk!' if @blocks.size > 1 s = if format == :reverse_ed - encode("#{op_act[@blocks[0].op]}#{context_range(:old)}\n") + encode("#{ED_DIFF_OP_ACTION[@blocks[0].op]}#{context_range(:old, ',')}\n") else - encode("#{context_range(:old, ' ')}#{op_act[@blocks[0].op]}\n") + encode("#{context_range(:old, ' ')}#{ED_DIFF_OP_ACTION[@blocks[0].op]}\n") end unless @blocks[0].insert.empty? - @data_new[@start_new..@end_new].each do |e| s << e + encode("\n") end + @data_new[@start_new..@end_new].each do |e| + s << e.chomp + encode("\n") + end s << encode(".\n") end s @@ -239,7 +290,7 @@ def ed_diff(format) # Generate a range of item numbers to print. Only print 1 number if the # range has only one item in it. Otherwise, it's 'start,end' - def context_range(mode, op = ',') # rubocop:disable Naming/UncommunicativeMethodParamName + def context_range(mode, op, last = false) case mode when :old s, e = (@start_old + 1), (@end_old + 1) @@ -247,6 +298,9 @@ def context_range(mode, op = ',') # rubocop:disable Naming/UncommunicativeMethod s, e = (@start_new + 1), (@end_new + 1) end + e -= 1 if last + e = 1 if e.zero? + s < e ? "#{s}#{op}#{e}" : e.to_s end private :context_range @@ -254,7 +308,7 @@ def context_range(mode, op = ',') # rubocop:disable Naming/UncommunicativeMethod # Generate a range of item numbers to print for unified diff. Print number # where block starts, followed by number of lines in the block # (don't print number of lines if it's 1) - def unified_range(mode) + def unified_range(mode, last) case mode when :old s, e = (@start_old + 1), (@end_old + 1) @@ -262,12 +316,25 @@ def unified_range(mode) s, e = (@start_new + 1), (@end_new + 1) end - length = e - s + 1 + length = e - s + (last ? 0 : 1) + first = length < 2 ? e : s # "strange, but correct" - length == 1 ? first.to_s : "#{first},#{length}" + length <= 1 ? first.to_s : "#{first},#{length}" end private :unified_range + def missing_last_newline?(data) + newline = encode("\n") + + if data[-2] + data[-2].end_with?(newline) && !data[-1].end_with?(newline) + elsif data[-1] + !data[-1].end_with?(newline) + else + true + end + end + if String.method_defined?(:encoding) def encode(literal, target_encoding = @preferred_data_encoding) literal.encode target_encoding diff --git a/lib/diff/lcs/ldiff.rb b/lib/diff/lcs/ldiff.rb index 2862267..17b374c 100644 --- a/lib/diff/lcs/ldiff.rb +++ b/lib/diff/lcs/ldiff.rb @@ -98,24 +98,19 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc: # items we've read from each file will differ by FLD (could be 0). file_length_difference = 0 - if @binary.nil? or @binary - data_old = IO.read(file_old) - data_new = IO.read(file_new) - - # Test binary status - if @binary.nil? - old_txt = data_old[0, 4096].scan(/\0/).empty? - new_txt = data_new[0, 4096].scan(/\0/).empty? - @binary = !old_txt or !new_txt - end + data_old = IO.read(file_old) + data_new = IO.read(file_new) + + # Test binary status + if @binary.nil? + old_txt = data_old[0, 4096].scan(/\0/).empty? + new_txt = data_new[0, 4096].scan(/\0/).empty? + @binary = !old_txt || !new_txt + end - unless @binary - data_old = data_old.split($/).map { |e| e.chomp } - data_new = data_new.split($/).map { |e| e.chomp } - end - else - data_old = IO.readlines(file_old).map { |e| e.chomp } - data_new = IO.readlines(file_new).map { |e| e.chomp } + unless @binary + data_old = data_old.lines.to_a + data_new = data_new.lines.to_a end # diff yields lots of pieces, each of which is basically a Block object @@ -150,20 +145,21 @@ def run(args, _input = $stdin, output = $stdout, error = $stderr) #:nodoc: end diffs.each do |piece| - begin + begin # rubocop:disable Style/RedundantBegin hunk = Diff::LCS::Hunk.new(data_old, data_new, piece, @lines, file_length_difference) file_length_difference = hunk.file_length_difference next unless oldhunk next if @lines.positive? and hunk.merge(oldhunk) - output << oldhunk.diff(@format) << "\n" + output << oldhunk.diff(@format) + output << "\n" if @format == :unified ensure oldhunk = hunk end end - last = oldhunk.diff(@format) + last = oldhunk.diff(@format, true) last << "\n" if last.respond_to?(:end_with?) && !last.end_with?("\n") output << last diff --git a/spec/fixtures/ldiff/output.diff-c b/spec/fixtures/ldiff/output.diff-c index 92ea5a2..0e1ad99 100644 --- a/spec/fixtures/ldiff/output.diff-c +++ b/spec/fixtures/ldiff/output.diff-c @@ -1,5 +1,5 @@ -*** spec/fixtures/aX 2019-02-01 22:29:34.000000000 -0500 ---- spec/fixtures/bXaX 2019-02-01 22:29:43.000000000 -0500 +*** spec/fixtures/aX 2020-06-23 11:15:32.000000000 -0400 +--- spec/fixtures/bXaX 2020-06-23 11:15:32.000000000 -0400 *************** *** 1 **** ! aX diff --git a/spec/fixtures/ldiff/output.diff-u b/spec/fixtures/ldiff/output.diff-u index c4ba30f..b84f718 100644 --- a/spec/fixtures/ldiff/output.diff-u +++ b/spec/fixtures/ldiff/output.diff-u @@ -1,5 +1,5 @@ ---- spec/fixtures/aX 2019-02-01 22:29:34.000000000 -0500 -+++ spec/fixtures/bXaX 2019-02-01 22:29:43.000000000 -0500 +--- spec/fixtures/aX 2020-06-23 11:15:32.000000000 -0400 ++++ spec/fixtures/bXaX 2020-06-23 11:15:32.000000000 -0400 @@ -1 +1 @@ -aX +bXaX diff --git a/spec/fixtures/ldiff/output.diff.chef b/spec/fixtures/ldiff/output.diff.chef new file mode 100644 index 0000000..8b98efb --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef @@ -0,0 +1,4 @@ +3c3 +< "description": "hi" +--- +> "description": "lo" diff --git a/spec/fixtures/ldiff/output.diff.chef-c b/spec/fixtures/ldiff/output.diff.chef-c new file mode 100644 index 0000000..efbfa19 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef-c @@ -0,0 +1,15 @@ +*** spec/fixtures/old-chef 2020-06-23 23:18:20.000000000 -0400 +--- spec/fixtures/new-chef 2020-06-23 23:18:20.000000000 -0400 +*************** +*** 1,4 **** + { + "name": "x", +! "description": "hi" + } +\ No newline at end of file +--- 1,4 ---- + { + "name": "x", +! "description": "lo" + } +\ No newline at end of file diff --git a/spec/fixtures/ldiff/output.diff.chef-e b/spec/fixtures/ldiff/output.diff.chef-e new file mode 100644 index 0000000..775d881 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef-e @@ -0,0 +1,3 @@ +3c + "description": "lo" +. diff --git a/spec/fixtures/ldiff/output.diff.chef-f b/spec/fixtures/ldiff/output.diff.chef-f new file mode 100644 index 0000000..9bf1e67 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef-f @@ -0,0 +1,3 @@ +c3 + "description": "lo" +. diff --git a/spec/fixtures/ldiff/output.diff.chef-u b/spec/fixtures/ldiff/output.diff.chef-u index ebf2b12..dbacd88 100644 --- a/spec/fixtures/ldiff/output.diff.chef-u +++ b/spec/fixtures/ldiff/output.diff.chef-u @@ -1,8 +1,9 @@ ---- spec/fixtures/old-chef 2019-02-01 21:57:15.000000000 -0400 -+++ spec/fixtures/new-chef 2019-02-01 21:57:29.000000000 -0400 -@@ -1,5 +1,5 @@ +--- spec/fixtures/old-chef 2020-06-23 23:18:20.000000000 -0400 ++++ spec/fixtures/new-chef 2020-06-23 23:18:20.000000000 -0400 +@@ -1,4 +1,4 @@ { "name": "x", - "description": "hi" + "description": "lo" } +\ No newline at end of file diff --git a/spec/fixtures/ldiff/output.diff.chef2 b/spec/fixtures/ldiff/output.diff.chef2 new file mode 100644 index 0000000..496b3dc --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2 @@ -0,0 +1,7 @@ +2d1 +< recipe[b::default] +14a14,17 +> recipe[o::new] +> recipe[p::new] +> recipe[q::new] +> recipe[r::new] diff --git a/spec/fixtures/ldiff/output.diff.chef2-c b/spec/fixtures/ldiff/output.diff.chef2-c new file mode 100644 index 0000000..8349a7a --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2-c @@ -0,0 +1,20 @@ +*** spec/fixtures/old-chef2 2020-06-30 09:43:35.000000000 -0400 +--- spec/fixtures/new-chef2 2020-06-30 09:44:32.000000000 -0400 +*************** +*** 1,5 **** + recipe[a::default] +- recipe[b::default] + recipe[c::default] + recipe[d::default] + recipe[e::default] +--- 1,4 ---- +*************** +*** 12,14 **** +--- 11,17 ---- + recipe[l::default] + recipe[m::default] + recipe[n::default] ++ recipe[o::new] ++ recipe[p::new] ++ recipe[q::new] ++ recipe[r::new] diff --git a/spec/fixtures/ldiff/output.diff.chef2-d b/spec/fixtures/ldiff/output.diff.chef2-d new file mode 100644 index 0000000..ca32a49 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2-d @@ -0,0 +1,7 @@ +d2 +a14 +recipe[o::new] +recipe[p::new] +recipe[q::new] +recipe[r::new] +. diff --git a/spec/fixtures/ldiff/output.diff.chef2-e b/spec/fixtures/ldiff/output.diff.chef2-e new file mode 100644 index 0000000..89f3fa0 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2-e @@ -0,0 +1,7 @@ +14a +recipe[o::new] +recipe[p::new] +recipe[q::new] +recipe[r::new] +. +2d diff --git a/spec/fixtures/ldiff/output.diff.chef2-f b/spec/fixtures/ldiff/output.diff.chef2-f new file mode 100644 index 0000000..ca32a49 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2-f @@ -0,0 +1,7 @@ +d2 +a14 +recipe[o::new] +recipe[p::new] +recipe[q::new] +recipe[r::new] +. diff --git a/spec/fixtures/ldiff/output.diff.chef2-u b/spec/fixtures/ldiff/output.diff.chef2-u new file mode 100644 index 0000000..ef025c7 --- /dev/null +++ b/spec/fixtures/ldiff/output.diff.chef2-u @@ -0,0 +1,16 @@ +--- spec/fixtures/old-chef2 2020-06-30 09:43:35.000000000 -0400 ++++ spec/fixtures/new-chef2 2020-06-30 09:44:32.000000000 -0400 +@@ -1,5 +1,4 @@ + recipe[a::default] +-recipe[b::default] + recipe[c::default] + recipe[d::default] + recipe[e::default] +@@ -12,3 +11,7 @@ + recipe[l::default] + recipe[m::default] + recipe[n::default] ++recipe[o::new] ++recipe[p::new] ++recipe[q::new] ++recipe[r::new] diff --git a/spec/fixtures/new-chef2 b/spec/fixtures/new-chef2 new file mode 100644 index 0000000..8213c73 --- /dev/null +++ b/spec/fixtures/new-chef2 @@ -0,0 +1,17 @@ +recipe[a::default] +recipe[c::default] +recipe[d::default] +recipe[e::default] +recipe[f::default] +recipe[g::default] +recipe[h::default] +recipe[i::default] +recipe[j::default] +recipe[k::default] +recipe[l::default] +recipe[m::default] +recipe[n::default] +recipe[o::new] +recipe[p::new] +recipe[q::new] +recipe[r::new] diff --git a/spec/fixtures/old-chef2 b/spec/fixtures/old-chef2 new file mode 100644 index 0000000..4a23407 --- /dev/null +++ b/spec/fixtures/old-chef2 @@ -0,0 +1,14 @@ +recipe[a::default] +recipe[b::default] +recipe[c::default] +recipe[d::default] +recipe[e::default] +recipe[f::default] +recipe[g::default] +recipe[h::default] +recipe[i::default] +recipe[j::default] +recipe[k::default] +recipe[l::default] +recipe[m::default] +recipe[n::default] diff --git a/spec/hunk_spec.rb b/spec/hunk_spec.rb index 277f739..b3616bf 100644 --- a/spec/hunk_spec.rb +++ b/spec/hunk_spec.rb @@ -6,28 +6,39 @@ require 'diff/lcs/hunk' describe Diff::LCS::Hunk do - let(:old_data) { ['Tu avec carté {count} itém has'.encode('UTF-16LE')] } - let(:new_data) { ['Tu avec carte {count} item has'.encode('UTF-16LE')] } + let(:old_data) { ['Tu a un carté avec {count} itéms'.encode('UTF-16LE')] } + let(:new_data) { ['Tu a un carte avec {count} items'.encode('UTF-16LE')] } let(:pieces) { Diff::LCS.diff old_data, new_data } let(:hunk) { Diff::LCS::Hunk.new(old_data, new_data, pieces[0], 3, 0) } it 'produces a unified diff from the two pieces' do expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp @@ -1 +1 @@ - -Tu avec carté {count} itém has - +Tu avec carte {count} item has + -Tu a un carté avec {count} itéms + +Tu a un carte avec {count} items EXPECTED expect(hunk.diff(:unified)).to eq(expected) end + it 'produces a unified diff from the two pieces (last entry)' do + expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp + @@ -1 +1 @@ + -Tu a un carté avec {count} itéms + +Tu a un carte avec {count} items + \\ No newline at end of file + EXPECTED + + expect(hunk.diff(:unified, true)).to eq(expected) + end + it 'produces a context diff from the two pieces' do expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp *************** *** 1 **** - ! Tu avec carté {count} itém has + ! Tu a un carté avec {count} itéms --- 1 ---- - ! Tu avec carte {count} item has + ! Tu a un carte avec {count} items EXPECTED expect(hunk.diff(:context)).to eq(expected) @@ -36,9 +47,9 @@ it 'produces an old diff from the two pieces' do expected = <<-EXPECTED.gsub(/^ +/, '').encode('UTF-16LE').chomp 1c1 - < Tu avec carté {count} itém has + < Tu a un carté avec {count} itéms --- - > Tu avec carte {count} item has + > Tu a un carte avec {count} items EXPECTED @@ -48,7 +59,7 @@ it 'produces a reverse ed diff from the two pieces' do expected = <<-EXPECTED.gsub(/^ +/, '').encode('UTF-16LE').chomp c1 - Tu avec carte {count} item has + Tu a un carte avec {count} items . EXPECTED @@ -62,7 +73,7 @@ it 'produces a unified diff' do expected = <<-EXPECTED.gsub(/^\s+/, '').encode('UTF-16LE').chomp @@ -1 +1,2 @@ - +Tu avec carte {count} item has + +Tu a un carte avec {count} items EXPECTED expect(hunk.diff(:unified)).to eq(expected) diff --git a/spec/issues_spec.rb b/spec/issues_spec.rb index c9a1e53..ad73123 100644 --- a/spec/issues_spec.rb +++ b/spec/issues_spec.rb @@ -56,17 +56,17 @@ end end - describe "issue #57" do + describe 'issue #57' do it 'should fail with a correct error' do expect { - actual = {:category=>"app.rack.request"} - expected = {:category=>"rack.middleware", :title=>"Anonymous Middleware"} + actual = { :category => 'app.rack.request' } + expected = { :category => 'rack.middleware', :title => 'Anonymous Middleware' } expect(actual).to eq(expected) }.to raise_error(RSpec::Expectations::ExpectationNotMetError) end end - describe "issue #60" do + describe 'issue #60' do it 'should produce unified output with correct context' do old_data = <<-DATA_OLD.strip.split("\n").map(&:chomp) { @@ -95,4 +95,60 @@ EXPECTED end end + + describe 'issue #65' do + def diff_lines(old_lines, new_lines) + file_length_difference = 0 + previous_hunk = nil + output = [] + + Diff::LCS.diff(old_lines, new_lines).each do |piece| + hunk = Diff::LCS::Hunk.new(old_lines, new_lines, piece, 3, file_length_difference) + file_length_difference = hunk.file_length_difference + maybe_contiguous_hunks = (previous_hunk.nil? || hunk.merge(previous_hunk)) + + output << "#{previous_hunk.diff(:unified)}\n" unless maybe_contiguous_hunks + + previous_hunk = hunk + end + output << "#{previous_hunk.diff(:unified, true)}\n" unless previous_hunk.nil? + output.join + end + + it 'should not misplace the new chunk' do + old_data = [ + 'recipe[a::default]', 'recipe[b::default]', 'recipe[c::default]', + 'recipe[d::default]', 'recipe[e::default]', 'recipe[f::default]', + 'recipe[g::default]', 'recipe[h::default]', 'recipe[i::default]', + 'recipe[j::default]', 'recipe[k::default]', 'recipe[l::default]', + 'recipe[m::default]', 'recipe[n::default]' + ] + + new_data = [ + 'recipe[a::default]', 'recipe[c::default]', 'recipe[d::default]', + 'recipe[e::default]', 'recipe[f::default]', 'recipe[g::default]', + 'recipe[h::default]', 'recipe[i::default]', 'recipe[j::default]', + 'recipe[k::default]', 'recipe[l::default]', 'recipe[m::default]', + 'recipe[n::default]', 'recipe[o::new]', 'recipe[p::new]', + 'recipe[q::new]', 'recipe[r::new]' + ] + + expect(diff_lines(old_data, new_data)).to eq(<<-EODIFF) +@@ -1,5 +1,4 @@ + recipe[a::default] +-recipe[b::default] + recipe[c::default] + recipe[d::default] + recipe[e::default] +@@ -12,3 +11,7 @@ + recipe[l::default] + recipe[m::default] + recipe[n::default] ++recipe[o::new] ++recipe[p::new] ++recipe[q::new] ++recipe[r::new] + EODIFF + end + end end diff --git a/spec/ldiff_spec.rb b/spec/ldiff_spec.rb index 2f4c235..a2468f8 100644 --- a/spec/ldiff_spec.rb +++ b/spec/ldiff_spec.rb @@ -5,40 +5,39 @@ RSpec.describe 'bin/ldiff' do include CaptureSubprocessIO - let(:output_diff) { read_fixture } - let(:output_diff_c) { read_fixture('-c') } - let(:output_diff_e) { read_fixture('-e') } - let(:output_diff_f) { read_fixture('-f') } - let(:output_diff_u) { read_fixture('-u') } - let(:output_diff_chef) { read_fixture('-u', :base => 'output.diff.chef') } + fixtures = [ + { :name => 'output.diff', :left => 'aX', :right => 'bXaX' }, + { :name => 'output.diff.chef', :left => 'old-chef', :right => 'new-chef' }, + { :name => 'output.diff.chef2', :left => 'old-chef2', :right => 'new-chef2' } + ].product([nil, '-e', '-f', '-c', '-u']).map { |(fixture, flag)| + fixture = fixture.dup + fixture[:flag] = flag + fixture + } - specify do - expect(run_ldiff('-u', :left => 'old-chef', :right => 'new-chef')).to eq(output_diff_chef) - end - - specify do - expect(run_ldiff).to eq(output_diff) - end - - specify do - expect(run_ldiff('-c')).to eq(output_diff_c) - end + def self.test_ldiff(fixture) + desc = [ + fixture[:flag], + "spec/fixtures/#{fixture[:left]}", + "spec/fixtures/#{fixture[:right]}", + '#', + '=>', + "spec/fixtures/ldiff/#{fixture[:name]}#{fixture[:flag]}" + ].join(' ') - specify do - expect(run_ldiff('-e')).to eq(output_diff_e) - end - - specify do - expect(run_ldiff('-f')).to eq(output_diff_f) + it desc do + expect(run_ldiff(fixture)).to eq(read_fixture(fixture)) + end end - specify do - expect(run_ldiff('-u')).to eq(output_diff_u) + fixtures.each do |fixture| + test_ldiff(fixture) end - def read_fixture(flag = nil, options = {}) - base = options.fetch(:base, 'output.diff') - name = "spec/fixtures/ldiff/#{base}#{flag}" + def read_fixture(options) + fixture = options.fetch(:name) + flag = options.fetch(:flag) + name = "spec/fixtures/ldiff/#{fixture}#{flag}" data = IO.__send__(IO.respond_to?(:binread) ? :binread : :read, name) clean_data(data, flag) end @@ -68,13 +67,15 @@ def clean_output_timestamp(data) \s* (?:[-+]\d{4}|Z) }x, - '*** spec/fixtures/\1 0000-00-00 00:00:00.000000000 -0000' + '*** spec/fixtures/\1 0000-00-00 :00 =>:00 =>00.000000000 -0000' ) end - def run_ldiff(flag = nil, options = {}) - left = options.fetch(:left, 'aX') - right = options.fetch(:right, 'bXaX') + def run_ldiff(options) + flag = options.fetch(:flag) + left = options.fetch(:left) + right = options.fetch(:right) + stdout, stderr = capture_subprocess_io do system("ruby -Ilib bin/ldiff #{flag} spec/fixtures/#{left} spec/fixtures/#{right}") end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index b1935d0..7212f30 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -5,7 +5,6 @@ require 'psych' if RUBY_VERSION >= '1.9' - if ENV['COVERAGE'] require 'simplecov' @@ -45,30 +44,31 @@ def _synchronize end def capture_subprocess_io - _synchronize do - begin - require 'tempfile' + _synchronize { _capture_subprocess_io { yield } } + end + + def _capture_subprocess_io + require 'tempfile' - captured_stdout, captured_stderr = Tempfile.new('out'), Tempfile.new('err') + captured_stdout, captured_stderr = Tempfile.new('out'), Tempfile.new('err') - orig_stdout, orig_stderr = $stdout.dup, $stderr.dup - $stdout.reopen captured_stdout - $stderr.reopen captured_stderr + orig_stdout, orig_stderr = $stdout.dup, $stderr.dup + $stdout.reopen captured_stdout + $stderr.reopen captured_stderr - yield + yield - $stdout.rewind - $stderr.rewind + $stdout.rewind + $stderr.rewind - return captured_stdout.read, captured_stderr.read - ensure - captured_stdout.unlink - captured_stderr.unlink - $stdout.reopen orig_stdout - $stderr.reopen orig_stderr - end - end + [captured_stdout.read, captured_stderr.read] + ensure + captured_stdout.unlink + captured_stderr.unlink + $stdout.reopen orig_stdout + $stderr.reopen orig_stderr end + private :_capture_subprocess_io end require 'diff-lcs'