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

[bayard]

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());
+    }
 }