diff-match-patch
diff-match-patch copied to clipboard
Add Line Mode diff methods to the public API
We have a bug in the Java implementation of diff-match-patch: users are unable to follow the following online recipe for Line Mode diffs:
https://github.com/google/diff-match-patch/wiki/Line-or-Word-Diffs
The problem is that the crucial methods needed by this recipe are marked as "protected" in the diff_match_patch class, meaning they cannot be invoked by typical user application code since that code is neither a subclass of nor should it be in the same package as diff_match_patch. The most reasonable workaround for users is to artificially relocate their calling classes into the "name.fraser.neil.plaintext" package, but this is a bad solution since it suggests that all line-mode-diff-capable user code also belongs to the diff-match-patch project.
The solution in this PR is to:
(1) Mark needed methods and one class as "public" rather than "protected"
(2) Split the test class in two:
diff_match_patch_apitest
/ ^^^^^^^
diff_match_patch_test <
\
diff_match_patch_impltest
^^^^^^^^
The apitest class contains the majority of the original test, since
the majority of tested methods are already marked as "public" and
(presumably) must be on the public API of our library. This test
would not compile or run cleanly until we made the "protected" -->
"public" changes in (1).
The impltest class contains the smaller number of tests from the
original class which were targeting protected methods that
(presumably) are not on the public API of our library.
Other solutions are obviously possible: the simplest solution is just to make the protected-->public changes in (1) and not bother to restructure the test. I'm willing to accept this solution if the maintainers don't want to make such a big change to the test, but I do think we're in a better situation if we don't allow our public API tests to live in the same package as our implementation tests.
NOTE: I saw that we have the same problem in some other languages which support method privacy-- for example the C++ implementation has this problem and currently uses "friend" in the diff_match_patch class to allow diff_match_patch_test to access protected members. This PR does not attempt to fix those other languages, since I am not an expert.
My company has created a Contributor License Agreement as per your contributing guidelines: https://drive.google.com/open?id=1UZ6Ur64oUkitUzUVSL5tH3vMyTh1p2bM
@NeilFraser - Did I overreach in this PR? If it makes you uncomfortable that I've rearranged the Java version of the test code, I could change the PR so it only fixes the production code and leaves the test unchanged. But I do think that in the long run it's unhealthy to let the tests for Java and C++ act as "friends" to the production code...
I would love to see this merged.