support merging inline deltas split by whitespace or a minor equality (#168) #191
Conversation
| 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 |
There was a problem hiding this comment.
Please change it so that the default configuration without a merger leaves the functionality as it was, so no default merger.
There was a problem hiding this 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_MERGERand checking theinlineDeltaMergerfor null
There was a problem hiding this comment.
@wumpz: Please advise how to continue.
|
Stale pull request message |
|
@wumpz: Is there anything I can do to help you decide on how to continue? |
|
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. |
|
Thank you very much! |
|
However, thx for your contribution. |
No description provided.