Skip to content

Commit aa2434d

Browse files
committed
Applying Jorg Schaible's patch from CLI-177 to fix the OptionBuilder's not resetting when an Exception is thrown
git-svn-id: https://svn.apache.org/repos/asf/commons/proper/cli/branches/cli-1.x@754830 13f79535-47bb-0310-9956-ffa450edef68
1 parent 879c762 commit aa2434d

2 files changed

Lines changed: 45 additions & 15 deletions

File tree

src/java/org/apache/commons/cli/OptionBuilder.java

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ public static Option create() throws IllegalArgumentException
326326
{
327327
if (longopt == null)
328328
{
329+
OptionBuilder.reset();
329330
throw new IllegalArgumentException("must specify longopt");
330331
}
331332

@@ -344,21 +345,23 @@ public static Option create() throws IllegalArgumentException
344345
*/
345346
public static Option create(String opt) throws IllegalArgumentException
346347
{
347-
// create the option
348-
Option option = new Option(opt, description);
349-
350-
// set the option properties
351-
option.setLongOpt(longopt);
352-
option.setRequired(required);
353-
option.setOptionalArg(optionalArg);
354-
option.setArgs(numberOfArgs);
355-
option.setType(type);
356-
option.setValueSeparator(valuesep);
357-
option.setArgName(argName);
358-
359-
360-
// reset the OptionBuilder properties
361-
OptionBuilder.reset();
348+
Option option = null;
349+
try {
350+
// create the option
351+
option = new Option(opt, description);
352+
353+
// set the option properties
354+
option.setLongOpt(longopt);
355+
option.setRequired(required);
356+
option.setOptionalArg(optionalArg);
357+
option.setArgs(numberOfArgs);
358+
option.setType(type);
359+
option.setValueSeparator(valuesep);
360+
option.setArgName(argName);
361+
} finally {
362+
// reset the OptionBuilder properties
363+
OptionBuilder.reset();
364+
}
362365

363366
// return the Option instance
364367
return option;

src/test/org/apache/commons/cli/OptionBuilderTest.java

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,33 @@ public void testCreateIncompleteOption() {
145145
catch (IllegalArgumentException e)
146146
{
147147
// expected
148+
149+
// implicitly reset the builder
150+
OptionBuilder.create( "opt" );
148151
}
149152
}
153+
154+
public void testBuilderIsResettedAlways() {
155+
try
156+
{
157+
OptionBuilder.withDescription("JUnit").create('"');
158+
fail("IllegalArgumentException expected");
159+
}
160+
catch (IllegalArgumentException e)
161+
{
162+
// expected
163+
}
164+
assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
165+
166+
try
167+
{
168+
OptionBuilder.withDescription("JUnit").create();
169+
fail("IllegalArgumentException expected");
170+
}
171+
catch (IllegalArgumentException e)
172+
{
173+
// expected
174+
}
175+
assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
176+
}
150177
}

0 commit comments

Comments
 (0)