You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ba...@apache.org on 2009/03/16 08:26:44 UTC
svn commit: r754830 - in /commons/proper/cli/branches/cli-1.x/src:
java/org/apache/commons/cli/OptionBuilder.java
test/org/apache/commons/cli/OptionBuilderTest.java
Author: bayard
Date: Mon Mar 16 07:26:44 2009
New Revision: 754830
URL: http://svn.apache.org/viewvc?rev=754830&view=rev
Log:
Applying Jorg Schaible's patch from CLI-177 to fix the OptionBuilder's not resetting when an Exception is thrown
Modified:
commons/proper/cli/branches/cli-1.x/src/java/org/apache/commons/cli/OptionBuilder.java
commons/proper/cli/branches/cli-1.x/src/test/org/apache/commons/cli/OptionBuilderTest.java
Modified: commons/proper/cli/branches/cli-1.x/src/java/org/apache/commons/cli/OptionBuilder.java
URL: http://svn.apache.org/viewvc/commons/proper/cli/branches/cli-1.x/src/java/org/apache/commons/cli/OptionBuilder.java?rev=754830&r1=754829&r2=754830&view=diff
==============================================================================
--- commons/proper/cli/branches/cli-1.x/src/java/org/apache/commons/cli/OptionBuilder.java (original)
+++ commons/proper/cli/branches/cli-1.x/src/java/org/apache/commons/cli/OptionBuilder.java Mon Mar 16 07:26:44 2009
@@ -326,6 +326,7 @@
{
if (longopt == null)
{
+ OptionBuilder.reset();
throw new IllegalArgumentException("must specify longopt");
}
@@ -344,21 +345,23 @@
*/
public static Option create(String opt) throws IllegalArgumentException
{
- // create the option
- Option option = new Option(opt, description);
-
- // set the option properties
- option.setLongOpt(longopt);
- option.setRequired(required);
- option.setOptionalArg(optionalArg);
- option.setArgs(numberOfArgs);
- option.setType(type);
- option.setValueSeparator(valuesep);
- option.setArgName(argName);
-
-
- // reset the OptionBuilder properties
- OptionBuilder.reset();
+ Option option = null;
+ try {
+ // create the option
+ option = new Option(opt, description);
+
+ // set the option properties
+ option.setLongOpt(longopt);
+ option.setRequired(required);
+ option.setOptionalArg(optionalArg);
+ option.setArgs(numberOfArgs);
+ option.setType(type);
+ option.setValueSeparator(valuesep);
+ option.setArgName(argName);
+ } finally {
+ // reset the OptionBuilder properties
+ OptionBuilder.reset();
+ }
// return the Option instance
return option;
Modified: commons/proper/cli/branches/cli-1.x/src/test/org/apache/commons/cli/OptionBuilderTest.java
URL: http://svn.apache.org/viewvc/commons/proper/cli/branches/cli-1.x/src/test/org/apache/commons/cli/OptionBuilderTest.java?rev=754830&r1=754829&r2=754830&view=diff
==============================================================================
--- commons/proper/cli/branches/cli-1.x/src/test/org/apache/commons/cli/OptionBuilderTest.java (original)
+++ commons/proper/cli/branches/cli-1.x/src/test/org/apache/commons/cli/OptionBuilderTest.java Mon Mar 16 07:26:44 2009
@@ -145,6 +145,33 @@
catch (IllegalArgumentException e)
{
// expected
+
+ // implicitly reset the builder
+ OptionBuilder.create( "opt" );
}
}
+
+ public void testBuilderIsResettedAlways() {
+ try
+ {
+ OptionBuilder.withDescription("JUnit").create('"');
+ fail("IllegalArgumentException expected");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // expected
+ }
+ assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
+
+ try
+ {
+ OptionBuilder.withDescription("JUnit").create();
+ fail("IllegalArgumentException expected");
+ }
+ catch (IllegalArgumentException e)
+ {
+ // expected
+ }
+ assertNull("we inherited a description", OptionBuilder.create('x').getDescription());
+ }
}