commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Rob Oxspring" <roxspr...@imapmail.org>
Subject Re: [CLI] Do we have the correct model? (was Re: [CLI] Support for CVS style command line)
Date Thu, 19 Jun 2003 23:27:05 GMT
----- Original Message ----- 
From: "John Keyes" <jbjk@mac.com>
To: "Jakarta Commons Developers List" <commons-dev@jakarta.apache.org>
Sent: Wednesday, June 18, 2003 10:21 PM
Subject: Re: [CLI] Do we have the correct model? (was Re: [CLI] Support for CVS style command
line)


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

Not sure if I'm helping to clear things up but here goes.  I would regard "-f" as an option,
and therefore when faced with "-f file" I see an option "-f" with an argument "file".  Thinking
more though, this seems like a bit of a bikeshed.  Having an Option that references an Argument
is probably the closest to my description above but the proposal takes the opposite approach
where the Argument wraps a (Parent)Option.  This means that the Argument object concerns itself
with processing the "file" section but in effect could be considered the complete "-f file".

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

Yeah - thats all fair enough.  The "code smell" talk was definitely the OO purest in me, in
reality UME is a useful compromising tool.  

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

So the "print the usage" and "validate the command line" methods should use the same code
path?  Hmm, I'll have to ponder that.

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

The proposed Option is a slightly higher level concept than the current Option.   In the first
version of my diagram it was "CommandLineElement" but this suggested that Options should be
called "CommandLineElements" and the names of all the other classes just seemed uglier and
uglier.  So in the proposal an Option is considered a generic term for anything that could
appear in the command line structure.  The useful part is that, just like awt's Component/Container
relationship, an Options instance can be treated just like any other Option but has the facility
of holding other Option instances.

> 
> > Class Diagram (Composition):
> >
> > Argument --------1 ParentOption --------1 Options --------* Option
> 
> Options == childOptions here?

Yes.  An argument "-f file" delegates the processing of "-f" to a ParentOption.  A ParentOption
delegates the processing of any child options to an Options instance. And the Options class
delegates further processing to one of its Option instances.

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

+1  I'm not in the running for any class naming awards.

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

Thats just my personal habit - I tend to have some sort of append(StringBuffer) method and
then have toString() make use of it, getUsage() would probably be better than the toString()
though.  The important point is that the Option instance itself becomes responsible for producing
the usage text.

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

Currently the parser has a loop with an if statement:
for each argument
    if argument=="--" process --
    if argumetn "-D" processProperty
    if argument begins "-" processOption


But because all options become the same, the equivelent section (now within Options) becomes:

for each argument
    for each option
        if option canProcess argument
            option.process

> 
> > getPrefixes() - returns the argument prefixes that could indicate this 
> > Option can process a given argument.
> 
> Can you explain this in more detail?

After several attempts to start I'm tempted to say no!

Right. Firstly, it may well be unimportant for now as it is really an implementation detail
of my experimental code - there may be other stratergies for processing.

The idea is that each option will return the set of string prefixes that this option can handle.
 This unifies long and short option names {-?, -h, --help} along with command names and aliases
{update, upd, up} into a common structure that should make lookups easier.  In the case of
Opions, the set of prefixes is the union of the child options' prefixes.

The canProcess method becomes a trivial case of "return getPrefixes().contains(arg);" and
the Options implementation can use a prefix->option map to lookup the option to hand off
processing to.  Of course both of these become more complicated as allowances are made for
option bursting and arguments of the style "--arg=val1,val2".

I've attached my experimental code in the hope that it'll help you understand what I mean.
 I have options, commands, bursting, option groups and arguments parsing in isolation but
haven't had the chance to check that combinations work.

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

Yeah that'd work too!
> 
> > 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).

I'd suggest Collections.EMPTY_SET if getPrefixes() doesn't apply.

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

Hope all that helps,

Rob

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