commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rob Oxspring <roxspr...@imapmail.org>
Subject Re: [cli] commons cli version 2.0?
Date Fri, 08 Oct 2004 17:22:29 GMT
Probably should switch this thread to commons-dev@, are you subscribed there?

Andrew Ferguson wrote:
>>Thanks for that, I've added a new test (based on yours) and fixed
> 
> existing ones.  It turns out that GroupImpl was only validating options
> that are present and skipping those that were missing.  > I've fixed
> this in cvs but no binary is available yet.
> 
> ok thanks, I've updated my local checkout now and picked it up. I've
> being playing with it some more and have some more minor points..
> 
> 1) On the Builder classes the reset() method has a void return type
> 

This should be unecessary since create() causes an implicit reset().  On the 
other hand, returning 'this' doesn't do any harm and will be liked by the 
paranoid among us.  Done.

> 
> 2) Some error messages could be more specific eg in
> DefaultOption.validate

Good catch - will deal with this ASAP.

> 
> 3) (As noted before) in the "svn/cvs"-style usage there is no easy way
> to determine which command has been invoked - a (hacky?) workaround is
> to do something like
> 
> 	String command = null;
> 	for(Iterator i = line.getOptions().iterator(); i.hasNext(); ) {
> 		Option o = (Option) i.next();
> 		if(!o.getPreferredName().startsWith("-")) {
> 			if(command!=null) {
> 				throw new RuntimeException("You can only
> specify one command"); // this is never thrown as the top-level Group
> has a maximum of 1
> 			} else {
> 				command = o.getPreferredName();
> 			}
> 		}
> 	}
> 
> 	I'm not sure what a nice API for this would be though - maybe
> something to look at only the top level options passed (and not their
> children)
> 
> 		CommandLine line = ...
> 		...
> 		line.getTopLevelOptions(); ??

I suspect that the loop above is the most sensible thing to do, although I'd 
probably use an "o instanceof Command" clause rather than checking for the 
prefix.  Personally I tend to work the other way around and have a construct 
along the lines of:

if(line.contains("update")){
     doUpdate(line);
}
else if(line.contains("commit")){
     doCommit(line);
}
else{
     help.print();
}

This way you never really ask the question which option is present.  I'm 
nervous of adding methods such as getTopLevelOptions() as it seems very 
particular to the SVN style of options and I'm not sure how useful it would be 
  in others (I think that even CVS would need a group of options that contains 
the group of commands to correctly model it, and how getTopLevelOptions would 
know that I'm not sure).

> 
> 4) (As noted before) For non-group option implementations nesting
> exceptions might be a way of allowing detailed information about where
> the parse error occurred
> 	eg in the validate method in Command you could have
> 
>     public void validate(WriteableCommandLine commandLine)
>         throws OptionException {
>         if (isRequired() && !commandLine.hasOption(this)) {
>             throw new OptionException(this);
>         }
>         try {
>                 super.validate(commandLine);
>         } catch(OptionException oe) {
>                 OptionException mine = new OptionException(this); // or
> maybe the message should be altered to "Command "+preferredName+"
> "+oe.getMessage());
>                 mine.initCause(oe);
>                 throw mine;
>         }
>     }
> 
> 	which would mean the HelpFormatter would need to iterate to the
> end of the chain of exceptions to find the true message (or as in the
> comment above parent options could prefix additional
> 	information about what is going on). Its probably better to have
> the HelpFormatter construct this detailed message as an option if this
> idea has any milage in it

I've been pondering this for the last few days, I think chaining (or otherwise 
augmenting) exceptions is probably the right thing to do.  Essentially the 
HelpFormatter needs to be able to identify the correct level of context to 
display.  One method would be to tell HelpFormatter that the context is always 
X options into the chain, another would be to fix at Y options from the end of 
the chain.  Another method for identifying the context would be to add a 
property to ParentImpl (or maybe OptionImpl) to indicate whether that option 
should be used as the context, and then search for the last matching option.

I'm pretty sure that we can find a sensible solution and out do cvs/svn's 
level of helpfulness.  On the other hand I'm still mulling over the options 
here so thoughts are welcome!!

> 
> Also, if you do need/want some extra coding/docs time then I might be
> able to get permission to allocate some time to lending a hand?

Patches are always appreciated, whether in docs, test, code or just ideas! 
Although such discussions should probably migrate to commons-dev@.

Thanks,

Rob


> 
> thanks,
> Andrew

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-user-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-user-help@jakarta.apache.org


Mime
View raw message