Wraps text in preamble as well #110
Conversation
yfarjoun
commented
Oct 28, 2017
yfarjoun
commented
- fixes WordWrap the preamble, not only the argument description. #103
- also adds some finals and other aesthetics
- fixes #103 - also adds some finals and other easthetics
Codecov Report
@@ Coverage Diff @@
## master #110 +/- ##
============================================
+ Coverage 76.01% 76.24% +0.22%
- Complexity 583 599 +16
============================================
Files 22 22
Lines 2172 2277 +105
Branches 450 492 +42
============================================
+ Hits 1651 1736 +85
- Misses 343 356 +13
- Partials 178 185 +7
Continue to review full report at Codecov.
|
| } | ||
| } catch (IOException e) { | ||
| throw new RuntimeException("Unreachable Statement.\nHow did the Buffered reader throw when it was " + | ||
| "wrapping a StringReader?", e); | ||
| } | ||
|
|
||
| //unless the original string ends in a newline, we need to remove the last newline that was added. | ||
| if (input.charAt(input.length() - 1) != '\n') { | ||
| out.deleteCharAt(out.length() - 1); | ||
| if (input.charAt(input.length() - 1) == '\n') { |
There was a problem hiding this comment.
The comment above is out of sync with this code now.
| out.append("\n"); | ||
| } else { | ||
| break; | ||
| } |
There was a problem hiding this comment.
Most of the code in these files isn't written in functional style, but this whole statement block could be reduced to:
out.append(bufReader.lines()
.map(line -> WordUtils.wrap(line, width))
.collect(Collectors.joining("\n")));
| "characters..."; | ||
|
|
||
| final String result = byteArrayOutputStream.toString(); | ||
| Assert.assertEquals(result.substring(0, expected.length()), expected); | ||
| } | ||
|
|
There was a problem hiding this comment.
If both parsers were going to live forever, we'd probably invest in refactoring to share test cases, but for now, please duplicate this test for CommandLineParserTest, since since both were modified.
There was a problem hiding this comment.
hmmm. I tried doing that but the inverse html doesn't seem to work in nonLegacy...am I doing something wrong?
There was a problem hiding this comment.
It looks like non legacy parser doesn't do the html->text rendering that the legacy parser does. I added a created #112 for this. Hopefully that won't block adding the test.
|
no, it's just that it means that the test has to be different...
…On Tue, Nov 14, 2017 at 4:53 PM, Chris Norman ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/test/java/org/broadinstitute/barclay/argparser/
LegacyCommandLineArgumentParserTest.java
<#110 (comment)>
:
> + public void testLongPreamble() {
+ final LegacyCommandLineArgumentParser clp = new LegacyCommandLineArgumentParser(new TestParserLongPreamble());
+
+ final ByteArrayOutputStream byteArrayOutputStream = new ByteArrayOutputStream();
+ final PrintStream stream = new PrintStream(byteArrayOutputStream);
+ clp.parseArguments(stream, new String[]{});
+ stream.append(clp.usage(true, true));
+
+ final String expected = "USAGE: TestParserLongPreamble [options]\n" +
+ "\n" +
+ "X < Y This is the first row but it's really long and it has lots of words...really big words. Because it knows the best\n" +
+ "people and really has lots of interesting things it needs to get across. Definitely more than can fit in 140\n" +
+ "characters...";
+
+ final String result = byteArrayOutputStream.toString();
+ Assert.assertEquals(result.substring(0, expected.length()), expected);
}
It looks like non legacy parser doesn't do the html->text rendering that
the legacy parser does. I added a created #112
<#112> for this.
Hopefully that won't block adding the test.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#110 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACnk0r8JZODSRqPYm_ORkl0Cg__3rIQrks5s2gu9gaJpZM4QJ4of>
.
|
|
back to you @cmnbroad |
|
bump. |
