Cleanup help formatting#277
Conversation
…ded HelpFromatter.getDescription(Option)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #277 +/- ##
============================================
+ Coverage 91.90% 95.83% +3.93%
- Complexity 575 658 +83
============================================
Files 22 23 +1
Lines 1247 1368 +121
Branches 210 225 +15
============================================
+ Hits 1146 1311 +165
+ Misses 63 25 -38
+ Partials 38 32 -6 ☔ View full report in Codecov by Sentry. |
garydgregory
left a comment
There was a problem hiding this comment.
@Claudenw
Thank you for your PR. Please see my comments.
| * Returns the option description or an empty string if the description is {@code null}. | ||
| * @param option The option to get the description from. | ||
| * @return the option description or an empty string if the description is {@code null}. | ||
| */ |
There was a problem hiding this comment.
- New public and protected elements need Javadoc
sincetags. - In Javadoc, a foo getter methods "Gets foo..." (I've tried to be consistent with this in Commons.)
| // parse the command line arguments | ||
| CommandLine line = parser.parse(options, args); | ||
| } | ||
| catch (ParseException exp) { |
There was a problem hiding this comment.
catch should be on the same line as the previous }, just like in our code.
| * @return the option description or an empty string if the description is {@code null}. | ||
| */ | ||
| public static String getDescription(final Option option) { | ||
| return option.getDescription() == null ? "" : option.getDescription(); |
There was a problem hiding this comment.
Only call option.getDescription() once.
There was a problem hiding this comment.
I know we don't want to pull in the text utilities here but .... It feels like we do this sort of thing a number of times. In the case that we do implement the same checks multiple times, would it make sense to bring in a striped down StringUtils?
There was a problem hiding this comment.
Member
If you want to refactor this, I would only do so with a package private utility method.
|
|
||
| try { | ||
| Integer value = line.getParsedOptionValue(count); | ||
| System.out.format("The value is %s\m", value ); |
There was a problem hiding this comment.
\m? Do you mean %n for an EOL?
| try { | ||
| // parse the command line arguments | ||
| line = parser.parse(options, new String[] {"-n", "5"}); | ||
| System.out.println( "n="+line.getParsedOptionValue("n")); |
There was a problem hiding this comment.
System.out.println( "n="+line.getParsedOptionValue("n"));
->
System.out.println("n="+line.getParsedOptionValue("n"));
There was a problem hiding this comment.
not: System.out.println("n=" + line.getParsedOptionValue("n")); ?
There was a problem hiding this comment.
Right, the spacing was off.
| System.out.println( "n="+line.getParsedOptionValue("n")); | ||
| } catch (ParseException exp) { | ||
| // oops, something went wrong | ||
| System.err.println("Parsing failed. Reason: " + exp.getMessage()); |
There was a problem hiding this comment.
System.err.println("Parsing failed. Reason: " + exp.getMessage());
->
System.err.println("Parsing failed. Reason: " + exp.getMessage());
| System.out.println( "n="+line.getParsedOptionValue("n")); | ||
| } catch (ParseException exp) { | ||
| // oops, something went wrong | ||
| System.err.println("Parsing failed. Reason: " + exp.getMessage()); |
| } | ||
| </source> | ||
| void doSomething(Options options) { | ||
| Function<Option, String> disp = option -> String.format( "%s. %s", HelpFormatter.getDescription(option), |
There was a problem hiding this comment.
String.format( "%s. %s", HelpFormatter.getDescription(option),
->
String.format("%s. %s", HelpFormatter.getDescription(option),
| try { | ||
| // parse the command line arguments | ||
| line = parser.parse(options, new String[] {"-n", "5"}); | ||
| System.out.println( "n="+line.getParsedOptionValue("n")); |
Changes to simplify deprecated help output.
deprecatedFormatFuncfromBiFunction<String, Option, String>toFunction<Option,String>HelpFormatter.getDescription(Option)to provide a function does not return null forgetDescription