commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From John Keyes <j...@mac.com>
Subject Re: [CLI] Do we have the correct model? (was Re: [CLI] Support for CVS style command line)
Date Wed, 18 Jun 2003 21:21:40 GMT
Hi Rob,

<snip/>

> (I hope you don't regret the "bouncing the ideas around" comment too 
> much
> John :o) )

Is it too late to take it back ;)

> OptionImpl vs. CommandImpl.  Although these two types of object share 
> the common feature of appearing on a command line and having a group 
> of child options, I'm not convinced that one is an extension of the 
> other.  As a result I've struggled to fit Command into the hierarchy.

I've been struggling to find a clean entry point as well.  It might be
the case that it cannot be done.

> Option vs. Argument.  It might be just me but in the command "ant -f 
> file" I've always regarded it as an option -f with an argument file, 
> whereas the current model would have an argument "-f file".  This has 
> caused me occasional confusion and it seems that it
> would sit easier if Argument were able to deal purely with the "file" 
> section and not have to deal with the option's properties.  If the 
> Argument were able to drop the option properties then it might also be 
> possible to mix Arguments and Commands together.

I've always treated it as '-f file', and never thought of it in any 
other manner
as the value is so tightly coupled to the option.  Correct me if my 
understanding
is off but is this was you are suggesting, '-f file' is represented by 
an Option
that holds a reference to an Argument.

> AnonymousArgument and UnsupportedMethodException.  Again it might be a 
> personal thing but I've always thought of UME as being a bit of a code 
> smell - to me it says "the object isn't really a subtype of the 
> specifying class but we've put it here anyway, so there".  I think 
> this strengthens the case against Argument implementing Option.

The AnonymousArgument feature has not been designed, it has been 
*fitted* to the
current implementation.  I think there are places where UME's are 
appropriate
(partial implementations of specifications for example), but I too 
dislike the
UMEs in AnonymousArgument.  I suspect I was trying to limit the number 
of public
types used to 2 (Option and Argument), which I think is obvious from 
the impl.

> CommandLineParser knows an awful lot.  I've commented before that the 
> internal handling of -D and -- should be optional but the knowledge of 
> how all the other options behave seems to be encoded in this class 
> too.  There's nothing inherently wrong with that but
> it left me thinking it odd that the Option implementations didn't 
> control the behaviour themselves.  It also struck me that the same 
> would be true of any HelpFormatter built for the current model.

I was working on simplifying CommandLineParser last night and I was 
thinking
about the validation step (now that it is a separate call) and it 
struck me
that the validation and a HelpFormatter would follow the same path 
through
the Options instance and that there should be a means to plug-in the
functionality you require, rather than having a separate help() method
being defined on the Option as well.

I do think the parsing is slighty different, but I need to think about 
it in
a bit more detail.  It is possible that with your new proposal, it 
works out
fine.

> Without boring you with the route my thoughts took, here's the model I 
> came to with a bit of explanation afterwards.
>
> Class Diagram (Inheritance):
>
> Option <-----------+-- Options <---------------+-- ExclusiveOptions
>   getDescription() |     parse(...)            |
>   isRequired()     |                           +-- InclusiveOptions
>   appendUsage(...) |
>   validate(...)    +-- ParentOption <----------+-- DefaultOption
>   process(...)     |     processParent(...)    |
>   canProcess(...)  |     processChildren(...)  +-- CommandOption
>   getPrefixes()    |
>                    +-- Argument
>                    |
>                    +-- PropertyOption
>                    |     getInstance()
>                    |
>                    +-- IgnoreOption
>                          getInstance()

Can you explain how Options inherits from Option?  I don't understand
that piece.

> Class Diagram (Composition):
>
> Argument --------1 ParentOption --------1 Options --------* Option

Options == childOptions here?

> The first thing to note is that I've separated out the Option 
> interface from the DefaultOption class.  The Option interface has lost 
> most of its structure enforcing role and gained behaviour specifying 
> instead.  Some new OptionBuilder would build DefaultOption instances 
> with all the longName / shortName and other properties of the current 
> OptionImpl class.  Similarly a CommandBuilder would allow 
> CommandOption instances to be built up with preferredName / aliases 
> properties.
>
> Another important point is that Argument is no longer directly related 
> to the DefaultOption.  Instead it is composed of either a 
> DefaultOption or CommandOption and concentrates purely on the details 
> of the argument.
>
> Options just becomes a generic object for storing a set of options, 
> the Exclusive / Inclusive just specify more restrictive behaviour as 
> they do presently.
>
> The idea of PropertyOption (-D) and IgnoreOption (--) just dropped out 
> of the structure and means that this standard behaviour can be treated 
> as any other option and if people have their own plan for a -D option 
> then they simply don't put a PropertyOption
> instance in their Options.

That makes sense.  I would rename IgnoreOption to ConsumeOption to 
signify that
it actually does do something rather than simply ignores the values on 
the
command line.

> After a little experimentation the sensible methods for Option to 
> specify seem to be the following:
> getDescription() - used to allow a HelpFormatter to describe any given 
> Option.
> appendUsage(...) - allow the option itself to append a usage string to 
> a StringBuffer

I don't fully understand the reason for appendUsage, why would there 
not simply
be a getUsage?

> validate(...) - validates a CommandLine's use of this Option
> isRequired() - presumably used by validate() and appendUsage()
> process(...) - takes a ListIterator of arguments and deals with 
> some/all of them.
> canProcess(...) - checks that a call to process(...) will succeed.

Is this necessary?  The current impl has two phases, one is to build the
CommandLine and the other is to validate the CommandLine against the 
Options.

> getPrefixes() - returns the argument prefixes that could indicate this 
> Option can process a given argument.

Can you explain this in more detail?

> One of the nice things about this structure is that when somebody 
> wants to extend the system to cope with windows style "/?" options, or 
> case insensitive commands, they should only need to add a single 
> Option implementation and any Parser and Formatter will automatically 
> cope.

Weird!  I had just began looking into this last night.  I removed the 
references
to "-" and replaced it with a reference to a static (PREFIX).

> I've not yet figured out whether a separate AnonymousArgument is 
> needed or if an Argument could exist without a ParentOption but I'm 
> sure that the UnsupportedMethodException methods would be gone 
> completely whichever route was taken.

Based on the approach that Option no longer enforces the structure, it 
will
follow that there will be no NMEs (depending on what getPrefixes does).

> So, thoughts? I'm happy to answer questions and help with 
> implementation as time permits but I'll be on holiday from Friday for 
> 10 days so will be offline for a while.

Well I think I've enough thoughts in there for you ;)

Cheers for the great input,
-John K

>
> Rob
>
>
> ----- Original Message -----
> From: "John Keyes" <jbjk@mac.com>
> To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
> Sent: Thursday, June 12, 2003 10:18 AM
> Subject: Re: [CLI] Support for CVS style command line
>
>
>>> The "setDisplayCommandAlias(boolean)" is purely hypothetical at the 
>>> moment, it illustrates
>>> the level of control I was planning on offering but is beyond the 
>>> implementation so far.
>>> I haven't tackled Commands and aliases at all but I guess should put 
>>> my code
>>> where my mouth is.  I'll keep you posted on progress.
>> Keep bouncing the ideas around anyway.  If you don't get around to 
>> implementing it
>> at least the knowledge will be archived.
>>
>> Thanks,
>> -John K
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>>
>>
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org
>
>
- - - - - - - - - - - - - - - - - - - - - - -
Jakarta Commons CLI
http://jakarta.apache.org/commons/cli


---------------------------------------------------------------------
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