FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Using custom string equalizers during diff by andreaskumlehn · Pull Request #1 · KengoTODA/java-diff-utils · GitHub

This repository was archived by the owner on Jun 24, 2018. It is now read-only.
/ java-diff-utils Public archive

Using custom string equalizers during diff - #1

Merged
KengoTODA merged 3 commits into
KengoTODA:masterfrom
andreaskumlehn:master
May 22, 2014
ConversationCommits (3)ChecksFiles changed
Merged

Using custom string equalizers during diff#1
KengoTODA merged 3 commits into
KengoTODA:masterfrom
andreaskumlehn:master

Conversation

andreaskumlehn commented May 16, 2014

Copy link
Copy Markdown
Contributor

Added support to inject a custom Equalizer into the builder of the DiffRowGenerator.

This modification enabled us to use regexes during diff to ignore minor diffs while comparing lines of revisions.

andreaskumlehn reviewed May 16, 2014
View reviewed changes
Comment thread src/main/java/difflib/DiffRowGenerator.java

andreaskumlehn May 16, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Moved the creation of the default Equalizer from constructor of DiffRowGenerator to its builder

KengoTODA self-assigned this May 18, 2014

KengoTODA commented May 19, 2014

Copy link
Copy Markdown
Owner

Thanks for your PR.

IMHO, it is better to:

andreaskumlehn commented May 19, 2014

Copy link
Copy Markdown
Contributor Author

Backwards compatibility is intact as I did not touch the publicly accessible methods.

About the nullness, I pushed a fix. I won't have time to create tests, sorry about that.

KengoTODA pushed a commit that referenced this pull request May 20, 2014
refs #1: reformat .gitignore
05ae3ef
KengoTODA pushed a commit that referenced this pull request May 20, 2014
refs #1: Equalizer does not have to care about ignoreWhiteSpace.
ff5cefc 
Now we have 2 classes which has to care about `ignoreWhiteSpace`
and here is 2 bugs:

1. it converts `null` to empty string
2. `Builder.ignoreWhiteSpace` doesn't work when we set custom
   Equalizer

This change solves these problem.

KengoTODA commented May 20, 2014

Copy link
Copy Markdown
Owner

Well, this change has one problem; there are 2 classes which is responsible to handle ignoreWhiteSpaces flag. Previously only Builder was responsible, but now both of Builder and custom Equalizer are responsible. And if you set custom Equalizer, Builder.ignoreWhiteSpaces doesn't work.

My suggestion is here, how do you think? I just remove this responsibility from Equalizer.

And, current your implementation converts null to empty string, but it breaks backward compatibility.

andreaskumlehn commented May 22, 2014

Copy link
Copy Markdown
Contributor Author

Looks good. Thanks for fixing it.

I did not have a look at the Nullable annotations as they were in place before my fix. Looks much better now.

KengoTODA merged commit cf9a759 into KengoTODA:master May 22, 2014
KengoTODA pushed a commit that referenced this pull request May 22, 2014
close #1: add <contributor> to pom.xml.
e9beb0c

KengoTODA commented May 22, 2014

Copy link
Copy Markdown
Owner

I've merged this branch to master. Thanks @andreaskumlehn.

You can download artifact from Maven central later. Its version is 1.5.0.

KengoTODA commented May 22, 2014

Copy link
Copy Markdown
Owner

Published at http://central.maven.org/maven2/jp/skypencil/java-diff-utils/diffutils/1.5.0/

andreaskumlehn commented May 22, 2014

Copy link
Copy Markdown
Contributor Author

Thanks a lot @eller86 :)

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

KengoTODA

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL