fix bug 14786, some refactorings

git-svn-id: https://svn.apache.org/repos/asf/jakarta/commons/proper/cli/trunk@129852 13f79535-47bb-0310-9956-ffa450edef68

Author: John Keyes

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

@@ -287,140 +287,178 @@ public String getDescription() {
         return this.description;
     }
 
-     /** 
-      * <p>Query to see if this Option requires an argument</p>
-      *
-      * @return boolean flag indicating if an argument is required
-      */
-     public boolean isRequired() {
-         return this.required;
-     }
-
-     /**
-      * <p>Sets whether this Option is mandatory.</p>
-      *
-      * @param required specifies whether this Option is mandatory
-      */
-     public void setRequired( boolean required ) {
-         this.required = required;
-     }
-
-     /**
-      * <p>Sets the display name for the argument value.</p>
-      *
-      * @param argName the display name for the argument value.
-      */
-     public void setArgName( String argName ) {
-         this.argName = argName;
-     }
-
-     /**
-      * <p>Gets the display name for the argument value.</p>
-      *
-      * @return the display name for the argument value.
-      */
-     public String getArgName() {
-         return this.argName;
-     }
-
-     /**
-      * <p>Returns whether the display name for the argument value
-      * has been set.</p>
-      *
-      * @return if the display name for the argument value has been
-      * set.
-      */
-     public boolean hasArgName() {
-         return (this.argName != null && this.argName.length() > 0 );
-     }
-
-     /** 
-      * <p>Query to see if this Option can take many values</p>
-      *
-      * @return boolean flag indicating if multiple values are allowed
-      */
-     public boolean hasArgs() {
-         return ( this.numberOfArgs > 1 || this.numberOfArgs == UNLIMITED_VALUES );
-     }
-
-     /** 
-      * <p>Sets the number of argument values this Option can take.</p>
-      *
-      * @param num the number of argument values
-      */
-     public void setArgs( int num ) {
-         this.numberOfArgs = num;
-     }
-
-     /**
-      * <p>Sets the value separator.  For example if the argument value
-      * was a Java property, the value separator would be '='.</p>
-      *
-      * @param sep The value separator.
-      */
-     public void setValueSeparator( char sep ) {
-         this.valuesep = sep;
-     }
-
-     /**
-      * <p>Returns the value separator character.</p>
-      *
-      * @return the value separator character.
-      */
-     public char getValueSeparator() {
-         return this.valuesep;
-     }
-
-     /** 
-      * <p>Returns the number of argument values this Option can take.</p>
-      *
-      * @return num the number of argument values
-      */
-     public int getArgs( ) {
-         return this.numberOfArgs;
-     }
-
-    public void clearValues() {
-        this.values.clear();
+    /** 
+     * <p>Query to see if this Option requires an argument</p>
+     *
+     * @return boolean flag indicating if an argument is required
+     */
+    public boolean isRequired() {
+        return this.required;
+    }
+
+    /**
+     * <p>Sets whether this Option is mandatory.</p>
+     *
+     * @param required specifies whether this Option is mandatory
+     */
+    public void setRequired( boolean required ) {
+        this.required = required;
+    }
+
+    /**
+     * <p>Sets the display name for the argument value.</p>
+     *
+     * @param argName the display name for the argument value.
+     */
+    public void setArgName( String argName ) {
+        this.argName = argName;
+    }
+
+    /**
+     * <p>Gets the display name for the argument value.</p>
+     *
+     * @return the display name for the argument value.
+     */
+    public String getArgName() {
+        return this.argName;
+    }
+
+    /**
+     * <p>Returns whether the display name for the argument value
+     * has been set.</p>
+     *
+     * @return if the display name for the argument value has been
+     * set.
+     */
+    public boolean hasArgName() {
+        return (this.argName != null && this.argName.length() > 0 );
+    }
+
+    /** 
+     * <p>Query to see if this Option can take many values</p>
+     *
+     * @return boolean flag indicating if multiple values are allowed
+     */
+    public boolean hasArgs() {
+        return this.numberOfArgs > 1 || this.numberOfArgs == UNLIMITED_VALUES;
+    }
+
+    /** 
+     * <p>Sets the number of argument values this Option can take.</p>
+     *
+     * @param num the number of argument values
+     */
+    public void setArgs( int num ) {
+        this.numberOfArgs = num;
+    }
+
+    /**
+     * <p>Sets the value separator.  For example if the argument value
+     * was a Java property, the value separator would be '='.</p>
+     *
+     * @param sep The value separator.
+     */
+    public void setValueSeparator( char sep ) {
+        this.valuesep = sep;
+    }
+
+    /**
+     * <p>Returns the value separator character.</p>
+     *
+     * @return the value separator character.
+     */
+    public char getValueSeparator() {
+        return this.valuesep;
+    }
+
+    /**
+     * ...
+     */
+    public boolean hasValueSeparator() {
+        return ( this.valuesep > 0 );
+    }
+
+    /** 
+     * <p>Returns the number of argument values this Option can take.</p>
+     *
+     * @return num the number of argument values
+     */
+    public int getArgs( ) {
+        return this.numberOfArgs;
     }
 
     /**
      * <p>Adds the specified value to this Option.</p>
      * 
      * @param value is a/the value of this Option
      */
-    public boolean addValue( String value ) {
-
+    void addValue( String value )
+    {
         switch( numberOfArgs ) {
             case UNINITIALIZED:
-                return false;
-            case UNLIMITED_VALUES:
-                if( getValueSeparator() > 0 ) {
-                    int index = 0;
-                    while( (index = value.indexOf( getValueSeparator() ) ) != -1 ) {
-                        this.values.add( value.substring( 0, index ) );
-                        value = value.substring( index+1 );
-                    }
-                }
-                this.values.add( value );
-                return true;
+                break;
             default:
-                if( getValueSeparator() > 0 ) {
-                    int index = 0;
-                    while( (index = value.indexOf( getValueSeparator() ) ) != -1 ) {
-                        if( values.size() > numberOfArgs-1 ) {
-                            return false;
-                        }
-                        this.values.add( value.substring( 0, index ) );
-                        value = value.substring( index+1 );
-                    }
-                }
-                if( values.size() > numberOfArgs-1 ) {
-                    return false;
-                }
-                this.values.add( value );
-                return true;
+                processValue( value );
+        }
+    }
+
+    /**
+     * <p>Processes the value.  If this Option has a value separator
+     * the value will have to be parsed into individual tokens.  When
+     * n-1 tokens have been processed and there are more value separators
+     * in the value, parsing is ceased and the remaining characters are
+     * added as a single token.</p>
+     *
+     * @since 1.0.1
+     */
+    private void processValue( String value ) {
+
+        // this Option has a separator character
+        if( hasValueSeparator() ) {
+
+            // get the separator character
+            char sep = getValueSeparator();
+
+            // store the index for the value separator
+            int index = value.indexOf( sep );
+
+            // while there are more value separators
+            while( index != -1 ) {
+
+                // next value to be added 
+                if( values.size() == numberOfArgs-1 ) {
+                    break;
+                } 
+
+                // store
+                add( value.substring( 0, index ) );
+
+                // parse
+                value = value.substring( index+1 );
+
+                // get new index
+                index = value.indexOf( sep );
+            }
+        }
+
+        // store the actual value or the last value that has been parsed
+        add( value );
+    }
+
+    /**
+     * <p>Add the value to this Option.  If the number of arguments
+     * is greater than zero and there is enough space in the list then
+     * add the value.  Otherwise, throw a runtime exception.
+     * </p>
+     *
+     * @since 1.0.1
+     */
+    private void add( String value ) {
+        if( numberOfArgs > 0 && values.size() > numberOfArgs-1 ) {
+            throw new RuntimeException( "Cannot add value, list full." );
         }
+        // store value
+        this.values.add( value );
     }
 
     /**

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

@@ -165,20 +165,19 @@ public Options addOption(String opt, String longOpt, boolean hasArg, String desc
      * @return the resulting Options instance
      */
     public Options addOption(Option opt)  {
-        String shortOpt = opt.getOpt();
+        String key = opt.getKey();
 
-        // add it to the long option list
-        if ( opt.hasLongOpt() ) {
-            longOpts.put( opt.getLongOpt(), opt );
-        }
+            // add it to the long option list
+            if ( opt.hasLongOpt() ) {
+                longOpts.put( opt.getLongOpt(), opt );
+            }
 
-        // if the option is required add it to the required list
-        if ( opt.isRequired() ) {
-            requiredOpts.add( opt.getKey() );
-        }
+            // if the option is required add it to the required list
+            if ( opt.isRequired() && !requiredOpts.contains( key ) ) {
+                requiredOpts.add( key );
+            }
+            shortOpts.put( key, opt );
 
-        shortOpts.put( shortOpt, opt );
-        
         return this;
     }
 

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

@@ -1,7 +1,7 @@
 /*
- * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//cli/src/java/org/apache/commons/cli/Parser.java,v 1.8 2002/11/18 08:41:26 jkeyes Exp $
- * $Revision: 1.8 $
- * $Date: 2002/11/18 08:41:26 $
+ * $Header: /home/jerenkrantz/tmp/commons/commons-convert/cvs/home/cvs/jakarta-commons//cli/src/java/org/apache/commons/cli/Parser.java,v 1.9 2002/11/25 23:43:40 jkeyes Exp $
+ * $Revision: 1.9 $
+ * $Date: 2002/11/25 23:43:40 $
  *
  * ====================================================================
  *
@@ -74,7 +74,7 @@
  *
  * @author John Keyes (john at integralsource.com)
  * @see Parser
- * @version $Revision: 1.8 $
+ * @version $Revision: 1.9 $
  */
 public abstract class Parser implements CommandLineParser {
 
@@ -299,9 +299,14 @@ public void processArgs( Option opt, ListIterator iter )
                 break;
             }
             // found a value
-            else if( !opt.addValue( str ) ) {
-                iter.previous();
-                break;
+            else {
+                try {
+                    opt.addValue( str ) ;
+                }
+                catch( RuntimeException exp ) {
+                    iter.previous();
+                    break;
+                }
             }
         }