Summary: | [PATCH] Use of Jakarta Commons CLI library for command line processing | ||
---|---|---|---|
Product: | Fop - Now in Jira | Reporter: | Simon Pepping <spepping> |
Component: | general | Assignee: | fop-dev |
Status: | CLOSED WONTFIX | ||
Severity: | normal | ||
Priority: | P3 | ||
Version: | trunk | ||
Target Milestone: | --- | ||
Hardware: | Other | ||
OS: | other | ||
Attachments: |
The patch as described
The patch as described |
Description
Simon Pepping
2004-05-04 19:31:25 UTC
Created attachment 11427 [details]
The patch as described
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 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. 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")); 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 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 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 Created attachment 11457 [details]
The patch as described
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. 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. batch transition to closed remaining pre-FOP1.0 resolved bugs |