>>>>> "Raif" == Raif S Naffah <[EMAIL PROTECTED]> writes:

Raif> the attached patch --already committed-- replaces command line parsing 
Raif> in the jarsigner tool with the newly added getopt classes.

I have a few comments on this.

Raif>    private Main()
[...]
Raif>      try
Raif>        {
Raif>          tool.processArgs(args);
Raif>          tool.start();
Raif> +        result = 0;
Raif>        }
Raif> +    catch (OptionException x)
Raif> +    {
Raif> +      System.err.println(x.getMessage()); //$NON-NLS-1$
Raif> +      if (tool.cmdLineParser != null)
Raif> +        tool.cmdLineParser.printHelp();
Raif> +    }

Generally speaking GNU tools do not print --help output in response
to an error.  Instead they typically just print a short error message
and a note to try --help.

Parser will do this for you automatically now; I recommend
implementing the new validate() method to do post-parsing option
checks.

Raif>    private void processArgs(String[] args) throws Exception

Raif>      if (args == null || args.length == 0)
Raif> -      HelpPrinter.printHelpAndExit(HELP_PATH);
Raif> +      throw new OptionException(Messages.getString("Main.133")); 
//$NON-NLS-1$

E.g., do this kind of thing in validate().


Also, I noticed that keytool isn't really using the new parser -- just
the help printer.  Is there a way we can extend the parser to handle
the cases needed by keytool?  Honestly I'd prefer not to export the
printHelp functionality from the parser, and this seems to be the only
external user.  Also, since getopt really exists only for classpath
tools, it would be better to just extend it as needed to do what we
really want.  (BTW if we're going to export the printHelp method, it
ought to have javadoc -- I'm trying to document all the exposed API of
getopt.)

Tom

Reply via email to