[ 
https://issues.apache.org/jira/browse/CLI-71?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#action_12498581
 ] 

Brian Egge commented on CLI-71:
-------------------------------

I went down the clone route for quite a while, as this seemed to be the 
reasonable approach.  After implementing clone and equals in several classes, I 
got everything working except for this test case:

    public void test15046() throws Exception {
        CommandLineParser parser = new PosixParser();
        final String[] CLI_ARGS = new String[] {"-z", "c"};
        Option option = new Option("z", "timezone", true, 
                                   "affected option");
        Options cliOptions = new Options();
        cliOptions.addOption(option);
        parser.parse(cliOptions, CLI_ARGS);
                
        //now add conflicting option
        cliOptions.addOption("c", "conflict", true, "conflict option");
        CommandLine line = parser.parse(cliOptions, CLI_ARGS);
        assertEquals( option.getValue(), "c" );
        assertTrue( !line.hasOption("c") );
    }

The gist of it is, the class has a reference to the option is passes into 
parse.  It then checks this exact same reference for it's value.  If parse 
clones the option, the code doesn't see the value show up.

I know I've written code like this in the past, so changing the behavior would 
break some existing code.  In 2.0, we can have immutable objects and not have 
to worry about these messy mutable type, but for this case, I think the best 
option is for parse to reset all the values.  Here's the proposed fix.  I'll 
also attach this, and the test case as an ASF safe patch.

Index: src/java/org/apache/commons/cli/Parser.java
===================================================================
--- src/java/org/apache/commons/cli/Parser.java (revision 541143)
+++ src/java/org/apache/commons/cli/Parser.java (working copy)
@@ -131,6 +131,10 @@
         throws ParseException
     {
         // initialise members
+        for (Iterator it = options.helpOptions().iterator(); it.hasNext();) {
+            Option opt = (Option) it.next();
+            opt.clear();
+        }
         this.options = options;
         requiredOptions = options.getRequiredOptions();
         cmd = new CommandLine();



> [cli] A weakness of parser
> --------------------------
>
>                 Key: CLI-71
>                 URL: https://issues.apache.org/jira/browse/CLI-71
>             Project: Commons CLI
>          Issue Type: Bug
>          Components: CLI-1.x
>         Environment: Operating System: other
> Platform: All
>            Reporter: Amro Al-Akkad
>             Fix For: 1.1
>
>         Attachments: BugCLI71Test.java, BugCLI71Test.java, 
> CLI-71-badfix.patch, CLI-71-fix.patch, TestCommonsCLI.java
>
>
> I found a weakness of Jakarta Commons CLI and want to explain it with a simple
> example: 
> Our program provides 2 options: 
> 1.    -a or --algo <name>: The -a option requires an argument.
> 2.    -k or --key <value>: The -k option requires an argument too.
> a)
> If you pass the following command line arguments everything will be ok:
> -a Caesar -k A
> After evaluation:
> •     "Caesar" is the parameter of the -a option and
> •     "A" is the parameter of the -k option.
> b)
> However an org.apache.commons.cli.MissingArgumentException: no argument for:k 
> is
> thrown if you pass the following input:
> -a Caesar -k a
> The Parser assumes that the argument "a" after the -k option, is the -a option
> missing the hyphen. At the end of this description there is Java code for
> executing this problem.
> Information:
> The handling of this command line 
> -a Caesar -k a 
> works in Getopt without any problem:
> •     "Caesar" is the parameter of the -a option and
> •     "a" of the -k option.
> After parsing a valid option Getopt always takes the next (available) command
> line argument as the option's parameter if the option requires an argument -
> means if you pass to the command line 
> -k -a Caesar
> After evaluation:
> •     "a" is the parameter of the -k option
> •     the "Caesar" argument is just ignored
> If the option's parameter (<value>) represents an optional argument the next
> argument is not required, if it represents a valid option - means if you pass 
> to
> the command line 
> -k -a Caesar
> After evaluation:
> •     "Caesar" is the parameter of the -a option
> •     k option is set without a parameter - in this case a default value 
> makes sense.
> Last but not least here is the code snippet for the CLI Test:
> import org.apache.commons.cli.CommandLine;
> import org.apache.commons.cli.CommandLineParser;
> import org.apache.commons.cli.Option;
> import org.apache.commons.cli.Options;
> import org.apache.commons.cli.ParseException;
> import org.apache.commons.cli.PosixParser;
> public class TestCommonsCLI {
>       /**
>        * @param args
>        */
>       public static void main(String[] args) {
>               
>               Options options = new Options();
>               
>               Option algorithm = new Option("a" , "algo", true, "the 
> algorithm which it to
> perform executing");
>               algorithm.setArgName("algorithm name");
>               options.addOption(algorithm);
>               
>               Option key = new Option("k" , "key", true, "the key the setted 
> algorithm uses
> to process");
>               algorithm.setArgName("value");
>               options.addOption(key);
>               
>               CommandLineParser parser = new PosixParser();
>               
>                try {
>                       CommandLine line = parser.parse( options, args);
>                       
>                       if(line.hasOption('a')){
>                       System.out.println("algo: "+ line.getOptionValue( "a" 
> ));
>                   }
>                       
>                       if(line.hasOption('k')){
>                       System.out.println("key: " + line.getOptionValue('k'));
>                   }
>                       
>                       
>               } catch (ParseException e) {
>                       // TODO Auto-generated catch block
>                       e.printStackTrace();
>               }
>       }
> }

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to