TEXT-231: WordUtils.wrap react to pre-existing "newline string"

[michaelkarnerfors]

WordUtils.wrap made to react to pre-existing "newline string" as a wrap

• The main loop is completely rewritten, aiming to increase legibility of the code
• The wrap method now respects pre-existing newline strings
• An inconsistency in how zero-length "wrapOn" patterns are treated has been addressed
• Test cases for the wrap method are made parametrised and named
• Minor Javadoc fixes

[garydgregory]

-1 as is:

• This PR will break the build: Run mvn to run the default Maven goal which in turn runs all build checks
• Don't rewrite existing tests: It makes it harder to review the PR and adds the possibility of changing test/feature coverage. It's better to add new test methods or add to existing tests methods.
• Don't clutter the tests with @DisplayName, use Javadoc if a test needs explanations, this provides better formatting.
• In general, keep changes to a minimum to easy the reviews.

[michaelkarnerfors]

-1 as is:

• This PR will break the build: Run mvn to run the default Maven goal which in turn runs all build checks
• Don't rewrite existing tests: It makes it harder to review the PR and adds the possibility of changing test/feature coverage. It's better to add new test methods or add to existing tests methods.
• Don't clutter the tests with @DisplayName, use Javadoc if a test needs explanations, this provides better formatting.
• In general, keep changes to a minimum to easy the reviews.

• After numerous attempts at building both master and this PR with 'mvn', I find no difference between the results. One test (DoubleFormatTest.testPlain_localeFormatComparison) that is unrelated to the change is broken in both master and this PR. Also another test breaks unless I set my machine to en_US, but — again — this happens in both master and this PR, and is unrelated. If you can point to any tests that fail at your end, I will happily take a look at them.

• "It makes it harder to review the PR" Yes, improvements do increase the task of reviewing a PR. If it is the case — that keeping the workload of reviewing a PR to an abject minimum is a vetoing priority in Apache Commons Text that discourages things such as improving existing code — then let me know and I will revert back to the old format. If not however, I consider the (in my not very humble opinion) messy tests, with uninformative names, that have not been touched up in up to 7 years, to be debt that should be dealt with and modernised.

• "the possibility of changing test/feature coverage" Since new functionality was added, the test coverage increased, yes. All the old tests have remained organised as they were before, and the contents are essentially untouched, with the exception for test cases that accepted broken behaviour.

• "add new test methods or add to existing tests methods" I did just that, too.

• Javadoc does not provide the functionality of @DisplayName and is wholly unsuitable for the intended purpose for which the annotations were added. To explain the purpose of using @DisplayName and parametrised tests, here is a comparative screenshot from Eclipse.

Unannotated test cases to the left, annotated on the right

IntelliJ IDEA also supports @DisplayName and parametrised tests (and Jetbrains even recommends it), as does Visual Studio Code.

Hence the comment is counterproductive: Javadoc adds more "clutter" than @DisplayName, without achieving the purpose for which the annotation was added.

• "In general, keep changes to a minimum to easy the reviews" I agree with this sentiment. With this in mind, lessening debt, increasing maintainability and aiding future development, is well worth one more PR in 7 years, I think you will agree. The very purpose of Open Source / Free Software is — after all — that they shall(!) be able to be improved over time.