Skip to content

Commit

Permalink
diff-lcs 1.4.4 - Fix broken formatting
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
halostatue committed Jul 1, 2020
1 parent 60613d2 commit b75e0bd
Show file tree
Hide file tree
Showing 2 changed files with 110 additions and 49 deletions.
132 changes: 83 additions & 49 deletions Contributing.md
@@ -1,84 +1,118 @@
## 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
<del>welcome</del>encouraged. 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 <del>welcome</del>encouraged. 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.

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/<username>/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/<username>/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
27 changes: 27 additions & 0 deletions History.md
Expand Up @@ -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
Expand Down

0 comments on commit b75e0bd

Please sign in to comment.