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 Sun, 29 Jun 2003 22:10:04 GMT
Rob,

I've been looking at your code and I like the structure.  I'm willing  
to add
this to a new branch (so we can synchronize changes) and we can work on  
it
for the next couple of weeks and see how we think things are  
progressing.

I think the first thing we should do is add the tests from the cli_1_x.
This will allow us to get the API in place.  By this I mean adding the
XXXBuilder classes and the CommandLineParser class.

Let me know what you think and I'll get the ball rolling,
-John K

On Friday, Jun 20, 2003, at 00:27 Europe/Dublin, Rob Oxspring wrote:

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