← 返回首页
support merging inline deltas split by whitespace or a minor equality (#168) by epictecch · Pull Request #191 · java-diff-utils/java-diff-utils · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

support merging inline deltas split by whitespace or a minor equality (#168)#191

Merged
wumpz merged 2 commits into
java-diff-utils:masterfrom
epictecch:master
Feb 5, 2025
Merged

support merging inline deltas split by whitespace or a minor equality (#168)#191
wumpz merged 2 commits into
java-diff-utils:masterfrom
epictecch:master

Conversation

epictecch commented Jul 30, 2024

Copy link
Copy Markdown
Contributor

No description provided.

epictecch commented Sep 18, 2024

Copy link
Copy Markdown
Contributor Author

@wumpz: Could you please provide any feedback regarding this pull request that would solve #168?
Is this PR in line with other concepts of the library?

wumpz requested changes Oct 5, 2024
View reviewed changes

wumpz left a comment

Copy link
Copy Markdown
Collaborator

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 Hide comment

Your PR looks good. Please change the default behaviour. And thx for using java-diff-utils.

Comment thread java-diff-utils/src/main/java/com/github/difflib/text/DiffRowGenerator.java
List<AbstractDelta<String>> inlineDeltas = DiffUtils.diff(origList, revList, equalizer).getDeltas();
List<AbstractDelta<String>> originalInlineDeltas = DiffUtils.diff(origList, revList, equalizer)
.getDeltas();
List<AbstractDelta<String>> inlineDeltas = inlineDeltaMerger

wumpz Oct 5, 2024

Copy link
Copy Markdown
Collaborator

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 Hide comment

Please change it so that the default configuration without a merger leaves the functionality as it was, so no default merger.

epictecch Oct 11, 2024

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 Hide comment

Thanks for your review and feedback.
The default behaviour is not changed by my pull request. By default, the inlineDeltaMerger is set to DEFAULT_INLINE_DELTA_MERGER which does not change the deltas. It just returns the original deltas.
By using the DEFAULT_INLINE_DELTA_MERGER, we can omit having null-checks on the inlineDeltaMerger and the code part that you've quoted stays cleaner.
Mainly, I used this approach compared to a null-check as it seems to be more in line with the other mechanisms. E.g. having a DEFAULT_EQUALIZER instead of having if-else cascades where Object::equals is used in case the equalizer property is null.

Please advise how to continue. Shall I:

  • leave my original code unchanged
  • add a comment above your quoted code part that explains that the default-delta-merger does not merge any deltas
  • Remove the DEFAULT_INLINE_DELTA_MERGER and checking the inlineDeltaMerger for null

epictecch Dec 2, 2024

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 Hide comment

@wumpz: Please advise how to continue.

github-actions Bot commented Feb 1, 2025

Copy link
Copy Markdown

Stale pull request message

epictecch commented Feb 3, 2025

Copy link
Copy Markdown
Contributor Author

@wumpz: Is there anything I can do to help you decide on how to continue?

wumpz merged commit b5b7dac into java-diff-utils:master Feb 5, 2025
wumpz added a commit that referenced this pull request Feb 5, 2025
fixes #191 - downgraded patch to Java 8 compliance
803fc4a
wumpz added a commit that referenced this pull request Feb 5, 2025
fixes #191 - downgraded patch to Java 8 compliance
110a0da

wumpz commented Feb 5, 2025

Copy link
Copy Markdown
Collaborator

I merged your changes. How did you test it? I assume you did not use the maven build. I had to change multiple methods that were not in Java 8.

epictecch commented Feb 6, 2025

Copy link
Copy Markdown
Contributor Author

Thank you very much!
To be honest, I don't remember anymore how I built the project. But it seems very likely that I didn't realize that the library should be Java 8 compatible.
I tested it manually and by executing the existing and newly added unit tests.

wumpz commented Feb 6, 2025

Copy link
Copy Markdown
Collaborator

However, thx for your contribution.
Sorry for beeing so slow. At the moment there is a time issue. So I have not so much time for my libs.

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 join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

wumpz wumpz requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Footer

© 2026 GitHub, Inc.