From b75e0bddacd6915289c1e0868571f9501e4a1529 Mon Sep 17 00:00:00 2001 From: Austin Ziegler Date: Wed, 1 Jul 2020 12:39:56 -0400 Subject: [PATCH] diff-lcs 1.4.4 - Fix broken formatting - Resolve #65: Two different issues were reported: - Line numbers changed between 1.3 and 1.4. This change is intentional, but in comparing the output against `diff` for the new examples provided it became clear that for unified and context diffs, the last hunk was reporting incorrect hunk lengths (that is, if it was `12,4 11,8`, it should have been `12,3 11,7`. This has been resolved by providing `Diff::LCS::Hunk#diff` an additional parameter, `last` which defaults to `false`. - Net new lines were added in the middle of a unified diff hunk, which was incorrect. That is, we were getting: ```diff @@ -10,6 +11,10 @@ recipe[j::default] recipe[k::default] recipe[l::default] +recipe[o::new] +recipe[p::new] +recipe[q::new] +recipe[r::new] recipe[m::default] recipe[n::default] ``` instead of: ```diff @@ -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] ``` This has been resolved. The error was that `Diff::LCS::Block#diff_size` was applying `.abs` to its output, which is incorrect. We need to know the direction of the size for placing changes, but when determining the maximum block size for use in `Diff::LCS::Hunk#diff` calculations, we need to know the `.abs` size. - New tests were added to prevent these changes from regressing in the future, both as issue tests and as additional `ldiff` tests. These tests highlighted more issues with diff-lcs output as compared to `diff`, specifically the handling and reporting of missing newlines at the end of files. All of the issues highlighted were resolved and the structure of `ldiff` tests was changed so that it is easier to add new comparison files at any time. - Resolve #35: Indicate that when comparing against custom objects, `#eql?` must be implemented such that objects that _resolve_ to the same meaning are _treated_ as the same meaning. This is important because the basic LCS algorithm uses a hash for position matching. - Resolve #43: Provide a more meaningful error from `Diff::LCS::Hunk.new` if the `piece` provided does not create useful `Diff::LCS::Block`. It's extremely unlikely, this error will be more useful than `NoMethodError` being thrown. - Resolve #44: `ldiff` binary detection failed to work correctly. It had been `!old_text or !new_text`, but the precedence of `or` broke that. It is now `!old_text || !new_text`. Also: - Ran Rubocop again. Updated configuration definitions, fixed some code formatting. --- Contributing.md | 132 ++++++++++++++++++++++++++++++------------------ History.md | 27 ++++++++++ 2 files changed, 110 insertions(+), 49 deletions(-) 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/History.md b/History.md index 3690990..063387b 100644 --- a/History.md +++ b/History.md @@ -8,8 +8,35 @@ 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