Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Issue 1708] Ignore invalid values in setUsageHelpLongOptionsMaxWidth #1712

Merged
merged 1 commit into from
Jun 19, 2022
Merged

[Issue 1708] Ignore invalid values in setUsageHelpLongOptionsMaxWidth #1712

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 5 additions & 7 deletions src/main/java/picocli/CommandLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7820,17 +7820,15 @@ public UsageMessageSpec width(int newValue) {
/**
* Sets the maximum usage help long options column max width to the specified value.
* This value controls the maximum width of the long options column: any positional parameter labels or long options that are longer than the specified value will overflow into the description column, and cause the description to be displayed on the next line.
* @param newValue the new maximum usage help long options column max width. Must be 20 or greater.
* @param newValue the new maximum usage help long options column max width. Must be 20 or greater, otherwise the new value will be ignored.
* @return this {@code UsageMessageSpec} for method chaining
* @throws IllegalArgumentException if the specified long options column max is less than 20
* @since 4.2 */
public UsageMessageSpec longOptionsMaxWidth(int newValue) {
if (newValue < DEFAULT_USAGE_LONG_OPTIONS_WIDTH) {
throw new InitializationException("Invalid usage long options max width " + newValue + ". Minimum value is " + DEFAULT_USAGE_LONG_OPTIONS_WIDTH);
} else if (newValue > width() - DEFAULT_USAGE_LONG_OPTIONS_WIDTH) {
throw new InitializationException("Invalid usage long options max width " + newValue + ". Value must not exceed width(" + width() + ") - " + DEFAULT_USAGE_LONG_OPTIONS_WIDTH);
if (newValue >= DEFAULT_USAGE_LONG_OPTIONS_WIDTH && newValue <= width() - DEFAULT_USAGE_LONG_OPTIONS_WIDTH) {
longOptionsMaxWidth = newValue;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with not throwing an exception, but I would like to log an INFO-level trace message that tells the user that we are ignoring the specified value, and why (similar to the message text of the InitializationException).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, it totally makes sense. I will update the branch and send you an update soon.

}
longOptionsMaxWidth = newValue; return this;

return this;
}

private int getSysPropertyWidthOrDefault(int defaultWidth, boolean detectTerminalSize) {
Expand Down
22 changes: 10 additions & 12 deletions src/test/java/picocli/HelpTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4387,25 +4387,23 @@ public void testSetLongOptionsColumnLengthMinimum_ModifiesSpec() {

@Test
public void testUsageMessageSpec_LongOptionsColumnLengthMinimum_Minimum20() {
try {
CommandSpec.create().usageMessage().longOptionsMaxWidth(19);
fail("Expected exception");
} catch (InitializationException ok) {
assertEquals("Invalid usage long options max width 19. Minimum value is 20", ok.getMessage());
}
CommandLine cmd = new CommandLine(CommandSpec.create());
cmd.setUsageHelpLongOptionsMaxWidth(20);

CommandLine returnValue = cmd.setUsageHelpLongOptionsMaxWidth(19);

assertEquals(20, cmd.getUsageHelpLongOptionsMaxWidth());
assertEquals(20, cmd.getCommandSpec().usageMessage().longOptionsMaxWidth());
}

@Test
public void testUsageMessageSpec_LongOptionsColumnLengthMinimum_MaxWidthMinus20() {
UsageMessageSpec spec = CommandSpec.create().usageMessage();
spec.longOptionsMaxWidth(spec.width() - 20);
assertEquals(60, spec.longOptionsMaxWidth());
try {
spec.longOptionsMaxWidth(61);
fail("Expected exception");
} catch (InitializationException ok) {
assertEquals("Invalid usage long options max width 61. Value must not exceed width(80) - 20", ok.getMessage());
}

spec.longOptionsMaxWidth(61);
assertEquals(60, spec.longOptionsMaxWidth());
}

@Test
Expand Down