Bug 28774 - [PATCH] Use of Jakarta Commons CLI library for command line processing
Summary: [PATCH] Use of Jakarta Commons CLI library for command line processing
Status: CLOSED WONTFIX
Alias: None
Product: Fop - Now in Jira
Classification: Unclassified
Component: general (show other bugs)
Version: trunk
Hardware: Other other
: P3 normal
Target Milestone: ---
Assignee: fop-dev
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2004-05-04 19:31 UTC by Simon Pepping
Modified: 2012-04-01 13:53 UTC (History)
0 users



Attachments
The patch as described (22.56 KB, patch)
2004-05-04 19:32 UTC, Simon Pepping
Details | Diff
The patch as described (25.60 KB, patch)
2004-05-07 20:01 UTC, Simon Pepping
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Pepping 2004-05-04 19:31:25 UTC
This patch introduces the Jakarta Commons CLI library for command line
processing, in apps/CommandLineOptions.java. It is meant to raise the
question whether this library is useful enough for FOP to use it and
thereby introduce a dependency on another library.

Features: Short and long forms for all options. Any order of the
options is allowed and options and arguments may be mixed freely. The
application may process the options in any order, independently of the
order in which they were specified; the order of multiple
specifications of the same option is retained.

The library does not guard against multiple specifications of the same
option, and the application cannot specify whether that is allowed or
not. Normal option value retrieval retrieves the first specified
value. To obtain all values a different call should be made, see
option 'param'.

In this patch I implemented the same command line conventions as the
current code. There is one exception; I changed '-print help' to '-h
print', because it is more difficult to return an option value to the
command line if it is not a specific value, and for '-h' it seems more
logical to assume always that the following non-option is an option
value and not an argument. Usage of the library would make it easy to
implement a different convention, e.g. Posix (one-letter short
options). The current convention is called GNU by the library,
although GNU applications usually use the GNU getops CLI library,
which implements Posix.

Elsewhere in the code I replaced a few Booleans with calls to the
parsed command line, e.g. "if (cl.hasOption('x'))" instead of 'if
(showConfiguration == Boolean.TRUE)'. Otherwise the code has remained
rather similar, except for options specification and parsing.

The current release of commons-cli is labeled 1.0-beta-2-dev. Peter
West uses commons-cli in his code. There was a discussion about
commons-cli on this list just over a year ago:
http://marc.theaimsgroup.com/?t=105105418200006&r=1&w=2
Comment 1 Simon Pepping 2004-05-04 19:32:23 UTC
Created attachment 11427 [details]
The patch as described
Comment 2 Glen Mazza 2004-05-05 03:16:47 UTC
Thanks for taking the time to create this patch.

When I first read the patch, my first thought was "no!--not another library!" 
But I do have to say the new code looks much cleaner and more readable.  (Part
of the reason for the previous messy code, I think, was Victor needing to break
out the param parsing into several "parseXXXXOption()" methods, in order to keep
the main parse() function less than 150 or so lines of code.   We at the time
were using a code style tool that recommended not having methods > 150 lines, or
maybe it was 300 or something.) 

Issues I currently see with Commons-CLI:
1) A release version is still unavailable--this may indicate it is not well
supported, or hasn't been supported recently. 

2) Not raising errors on duplicate parameters would appear to be a feature that
a robust CLI library should be able to provide.  The absence of this option
might reduce the usage of this library by other apps, and by the way OS works,
the amount of support this library gets.

3) The fact that both Joerg and Peter have also researched and like Commons-CLI
is a good sign.  Bad sign:  Xalan still just hardcodes their CLI [1]--being
linked into the JDK, they don't like the risk of additional libraries apparently.

[1]
http://cvs.apache.org/viewcvs.cgi/xml-xalan/java/src/org/apache/xalan/xslt/Process.java?rev=1.62&view=auto

My instinct, however boring, is to "build on stone" again here--once it is
working, it is "nailed down", and you never have to look at it or worry about it
again.  But this is not that important to me (we can easily revert the code if
it later presents a problem--it's only being used in one class), and it
certainly does make the code look cleaner and more maintainable.  I guess either
way is OK for me.

Thanks,
Glen
Comment 3 Jeremias Maerki 2004-05-05 19:05:25 UTC
Commons CLI is available as 1.0 final [1]. CLI 2.0 is in development although 
no quick progress is visible. 

[1] http://nagoya.apache.org/eyebrowse/ReadMsg?listName=commons-
dev@jakarta.apache.org&msgId=530775

I'm using Commons CLI in Barcode4J and it does a small but fine job.

Being a supporter of code reuse even if it means adding yet another small 
library, I'm +1 for applying this patch.
Comment 4 Peter B. West 2004-05-05 23:10:32 UTC
Some observations on Commons CLI:-

While there are exclusive groups of options, there are no inclusive groups; e.g.
XML and XSLT.

There seems to be no way to control the order of option output in the
HelpFormatter, though the header and footer texts are very useful for clarifying
such issues.  The virtue of HelpFormatter is that usage/help text is generated
directly from the Options object.

Eclipse complains about the (very useful) code structure

options.addOption(OptionBuilder.withLongOpt("xml-file")
       .hasArg().withArgName("file")
       .withDescription("XML input file")
       .create("xml"));
Comment 5 Simon Pepping 2004-05-06 20:34:52 UTC
Peter,

I could not make OptionGroups work; I thought they were still under development.
They are very useful for our input and output options. I had a look at your code
and try again.

About Eclipse, I assume this means that I have to split up the lines like in
your code:

        // The mutually exclusive verbosity group includes the -d and -q flags
        OptionGroup verbosity = new OptionGroup();
        OptionBuilder.withArgName("debug mode");
        OptionBuilder.withLongOpt("full-error-dump");
        OptionBuilder.withDescription("Verbosity: verbose reporting");
        verbosity.addOption(OptionBuilder.create("d"));


Simon
Comment 6 Peter B. West 2004-05-06 23:17:28 UTC
Simon,

If the OptionGroups didn't work for you, then the CLI code is probably buggy.  I
assumed it was working and did not specifically test for clashes in the
OptionGroups.  If so, could you let the CLI guys know?

You're right; the reason my OptionBuilder code is so verbose is to keep Eclipse
quiet.  That's not a compelling reason though.  There is no necessity to use
Eclipse, and if the consensus is that clean compact code is better, let's go
with that, and wait for the Eclipse guys to accommodate such cases.

Peter
Comment 7 Simon Pepping 2004-05-07 20:00:41 UTC
Peter,

No, OptionGroup now works fine. The secret is that you should call
options.addOptionGroup after the OptionGroup has been configured
completely. I attach a new installment of the patch, with
OptionGroups. It really looks nice. Only the error message from the
library, when someone uses two options from a group, is not very
clear: "an option from this group has already been selected: 'xml'".
I think an average user will not be very enlightened by the text 'from
this group'.

Simon
Comment 8 Simon Pepping 2004-05-07 20:01:31 UTC
Created attachment 11457 [details]
The patch as described
Comment 9 Peter B. West 2004-05-10 05:51:39 UTC
I had erroneously used PosixParser instead of GnuParser, an error pointed out to
me by Arjen Duursma, and now corrected in alt-design.  Thanks to Arjen.
Comment 10 Simon Pepping 2004-08-10 19:38:06 UTC
I decided not to switch to Jakarta Commons CLI. For an application like FOP it
is not really useful. FOP has its own well written CLI parser.
Comment 11 Glenn Adams 2012-04-01 13:53:19 UTC
batch transition to closed remaining pre-FOP1.0 resolved bugs