commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Brian Egge (JIRA)" <j...@apache.org>
Subject [jira] Commented: (CLI-71) [cli] A weakness of parser
Date Thu, 24 May 2007 10:56:17 GMT

    [ 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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message