Patch: Add Option Groups

Patch: Add Option Groups

[Phil Muldoon]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

I wanted to add option groups to fcore, and also make sure that the 
utility options come first.  I've included a draft patch.

This came up in conversation with Tom and Andrew. This required a little 
bit of hacking in bindir/fcore, util/CommandlineParser and 
util/ProcStopUntil. I've added the output and the patch that makes this 
happen. I have not checked it in. Please pay special attention to my 
comments around parse() as I am not sure this is the best place to add 
the default options. They have to be added "late" otherwise they will be 
added at the top of the --help output. There is a lot of weird indenting 
going on in the right help column, but this was happening beforehand and 
is another unrelated (but hopefully soon to be fixed) bug.

./frysk_bin/frysk-core/frysk/bindir/fcore --help

Usage: fcore <PID>

Corefile options:
  -a, -allmaps                   Include ALL process readable maps.
  -s, -segments =PATTERN        use PATTERN as regex to define maps 
inclusion.
  -o, -outputfile <filename>     Sets the name of  the corefile.

Frysk specific options:
  -debug <COMP=LEVEL,...>       Set debug-tracing of COMP to LEVEL.
                                  LEVEL can be [ NONE | FINE | FINEST ];
                                  default is FINE.
                                  Example: debug frysk.rsl=FINE
  -noexe                        Do not attempt to read an executable for a
                                  corefile
  -exe <executable>             Specify the full path of the executable 
to read
  -sysroot <path to sysroot>    Special root directory

Standard options:
  -help       print this help, then exit
  -version    print version number, then exit

Regards

Phil

[-- Attachment #2: options.patch --]
[-- Type: text/x-patch, Size: 7791 bytes --]

diff --git a/frysk-core/frysk/bindir/ChangeLog b/frysk-core/frysk/bindir/ChangeLog
index 3606ba8..60dcaf2 100644
--- a/frysk-core/frysk/bindir/ChangeLog
+++ b/frysk-core/frysk/bindir/ChangeLog
@@ -1,3 +1,8 @@
+2008-03-18  Phil Muldoon  <pmuldoon@redhat.com>
+
+	* fcore.java (addOptions): Add new group.
+	Add options to group and tweak text.
+
 2008-03-17  Andrew Cagney  <cagney@redhat.com>
 
 	* fstep.java: Update; using TaskAttachedObserverXXX.
diff --git a/frysk-core/frysk/bindir/fcore.java b/frysk-core/frysk/bindir/fcore.java
index a5d8862..07190c8 100644
--- a/frysk-core/frysk/bindir/fcore.java
+++ b/frysk-core/frysk/bindir/fcore.java
@@ -48,6 +48,7 @@ import frysk.util.ProcStopUtil;
 import frysk.isa.corefiles.LinuxElfCorefile;
 import frysk.isa.corefiles.LinuxElfCorefileFactory;
 import gnu.classpath.tools.getopt.Option;
+import gnu.classpath.tools.getopt.OptionGroup;
 import gnu.classpath.tools.getopt.OptionException;
 
 
@@ -80,12 +81,12 @@ public class fcore {
      */
     private static void addOptions (ProcStopUtil fcore)
     {
+	
+      OptionGroup group = new OptionGroup("Corefile options");
+      fcore.addGroup(group);
 
-      fcore.addOption(new Option( "allmaps", 'a',
-	                          " Writes all readable maps. Does not elide"
-	                        + " or omit any readable map. Caution: could"
-	                        + " take considerable amount of time to"
-	                        + " construct core file.")
+      group.add(new Option( "allmaps", 'a',
+	                          " Include ALL process readable maps.")
 	  {
 	      public void parsed (String mapsValue) throws OptionException {
 		  try {
@@ -98,9 +99,10 @@ public class fcore {
 	      }
 	  });
       
-      fcore.addOption(new Option("segments", 's',
-				 "Define what segments to include via regex.",
-				 "RegEx") {
+      group.add(new Option("segments", 's',
+				 "Use PATTERN as regex to define "+
+				 "maps inclusion.",
+				 "=PATTERN") {
 	      public void parsed(String regEx) throws OptionException {
 		  try {
 		      mapOptionCount++;
@@ -114,10 +116,9 @@ public class fcore {
       
       
 
-      fcore.addOption(new Option( "outputfile", 'o',
-	                          " Sets the name (not extension) of the core"
-	                        + " file. Default is core.{pid}. The extension"
-				  + " will always be the pid.", "<filename>")
+      group.add(new Option( "outputfile", 'o',
+	                          " Sets the name of  the " +
+	                          "corefile.", "<filename>")
 	  {
 	      public void parsed (String filenameValue) throws OptionException {
 		  try {
@@ -141,8 +142,8 @@ public class fcore {
 	    
 	  
 	    if (mapOptionCount > 1)
-		System.err.println("Please either speciy -stackonly,"+
-		" -allmaps, or -match <pattern> for map writing.");
+		System.err.println("Please either specify "+
+		" -allmaps, or -segments=PATTERN for map writing.");
 	    else {
 		Task[] tasks = (Task[]) proc.getTasks().toArray
 	                   (new Task[proc.getTasks().size()]);
@@ -168,7 +169,7 @@ public class fcore {
 	}
 	
 	public void executeDead(Proc proc) {
-	    System.err.println ("Cannot create core file from dead process");
+	    System.err.println ("Cannot create core file from a dead process.");
 	}
     }
 }
diff --git a/frysk-core/frysk/util/ChangeLog b/frysk-core/frysk/util/ChangeLog
index 1ddb337..ebf40a3 100644
--- a/frysk-core/frysk/util/ChangeLog
+++ b/frysk-core/frysk/util/ChangeLog
@@ -1,5 +1,12 @@
 2008-03-18  Phil Muldoon  <pmuldoon@redhat.com>
 
+	* CommandlineParser.java: (addDefaultOptions): New.
+	(parse): Set default options here.
+	(CommandlineParser): Move default option
+	creation to addDefaultOptions.
+	* ProcStopUtil.java (addGroup): New.
+	
+
 	* CommandlineParser.java (CommandlineParser): Trim
 	Sys Root help output.
 
diff --git a/frysk-core/frysk/util/CommandlineParser.java b/frysk-core/frysk/util/CommandlineParser.java
index 949c061..81ade30 100644
--- a/frysk-core/frysk/util/CommandlineParser.java
+++ b/frysk-core/frysk/util/CommandlineParser.java
@@ -46,6 +46,7 @@ import lib.dwfl.Elf;
 import lib.dwfl.ElfCommand;
 import lib.dwfl.ElfEHeader;
 import gnu.classpath.tools.getopt.Option;
+import gnu.classpath.tools.getopt.OptionGroup;
 import gnu.classpath.tools.getopt.OptionException;
 import gnu.classpath.tools.getopt.Parser;
 import frysk.rsl.LogOption;
@@ -68,27 +69,6 @@ public class CommandlineParser {
 
     public CommandlineParser(String name, String version) {
 	parser = new Parser(name, version, true);
-	parser.add(new LogOption("debug"));
-	add(new Option("noexe", "Do not attempt to read an"+
-		       " executable for a corefile ") {
-		public void parsed(String exeValue) throws OptionException {
-		    extendedCore = false;
-		    explicitExe = null;
-		}
-	    });
-	add(new Option("exe",
-		       "Specify the full path of the executable to read",
-		       "<executable>") {
-		public void parsed(String exeValue) throws OptionException {
-		    extendedCore = true;
-		    explicitExe = exeValue;
-		}
-	    });
-	add(new Option("sysroot", "Special root directory", "<path to sysroot>") {
-		public void parsed(String arg) throws OptionException {
-		    parseSysRoot(arg);
-		}
-	    });
     }
 
     public CommandlineParser(String programName) {
@@ -138,6 +118,11 @@ public class CommandlineParser {
     }
 
     public String[] parse(String[] args) {
+	
+	// Add in default options here.
+	// This is to preserve the user options as "first"
+	// Seems the wrong place to do it
+	addDefaultOptions();
 	try {
 	    fine.log(this, "parse", args);
 	    String[] result = doParse(args);
@@ -259,6 +244,32 @@ public class CommandlineParser {
 
     }
 
+    private void addDefaultOptions () {
+	OptionGroup defaultGroup = new OptionGroup("Frysk specific options");
+	add(defaultGroup);
+	defaultGroup.add(new LogOption("debug"));
+	defaultGroup.add(new Option("noexe", "Do not attempt to read an"+
+				    " executable for a corefile ") {
+		public void parsed(String exeValue) throws OptionException {
+		    extendedCore = false;
+		    explicitExe = null;
+		}
+	    });
+	defaultGroup.add(new Option("exe",
+				    "Specify the full path of the executable to read",
+				    "<executable>") {
+		public void parsed(String exeValue) throws OptionException {
+		    extendedCore = true;
+		    explicitExe = exeValue;
+		}
+	    });
+	defaultGroup.add(new Option("sysroot", "Special root directory", "<path to sysroot>") {
+		public void parsed(String arg) throws OptionException {
+		    parseSysRoot(arg);
+		}
+	    });
+	
+    }
     // @Override
     protected void validate() throws OptionException {
 	// Base implementation does nothing.
@@ -272,6 +283,10 @@ public class CommandlineParser {
 	parser.add(option);
     }
 
+    public void add(OptionGroup option) {
+	parser.add(option);
+    }
+
     public void printHelp() {
 	parser.printHelp();
     }
diff --git a/frysk-core/frysk/util/ProcStopUtil.java b/frysk-core/frysk/util/ProcStopUtil.java
index af231ed..555997b 100644
--- a/frysk-core/frysk/util/ProcStopUtil.java
+++ b/frysk-core/frysk/util/ProcStopUtil.java
@@ -48,6 +48,7 @@ import frysk.proc.ProcBlockObserver;
 import frysk.proc.Task;
 import frysk.util.CommandlineParser;
 import gnu.classpath.tools.getopt.Option;
+import gnu.classpath.tools.getopt.OptionGroup;
 import frysk.rsl.Log;
 
 /**
@@ -113,6 +114,10 @@ public class ProcStopUtil {
     public void addOption (Option option) {
 	parser.add (option);
     }
+
+    public void addGroup (OptionGroup option) {
+	parser.add (option);
+    }
     
     public void execute () {
 	parser.parse(args);
@@ -168,4 +173,4 @@ public class ProcStopUtil {
 	    System.exit(0);
 	}
     }
-}
\ No newline at end of file
+}

Re: Patch: Add Option Groups

[Tom Tromey]

>>>>> "Phil" == Phil Muldoon <pmuldoon@redhat.com> writes:

Phil> I wanted to add option groups to fcore, and also make sure that the
Phil> utility options come first.  I've included a draft patch.

Looks good.

A few nits.

Phil> this happen. I have not checked it in. Please pay special attention to
Phil> my comments around parse() as I am not sure this is the best place to
Phil> add the default options. They have to be added "late" otherwise they
Phil> will be added at the top of the --help output.

Yeah, this is a bit odd, but I guess harmless for the time being.
Maybe it would be better to either push new functionality for this
kind of thing into Parser, or to have users of frysk's
CommandlineParser be a bit smarter about coordinating with the
superclass.

Phil> There is a lot of weird
Phil> indenting going on in the right help column, but this was happening
Phil> beforehand and is another unrelated (but hopefully soon to be fixed)
Phil> bug.

Some of this is accounted for by rogue spaces, e.g.:

Phil> +      group.add(new Option( "allmaps", 'a',
Phil> +	                          " Include ALL process readable maps.")

That space before "Include" is strange.

Phil> +				 "=PATTERN") {

Putting the "=" in here is wrong, fwiw.

Tom

Re: Patch: Add Option Groups

[Andrew Cagney]

Phil,

     public CommandlineParser(String name, String version) {

I'd either:

  create the utility specific group, and then the standard frysk option 
group and all the standard frysk options here; and then have "add" add 
to the utility specific group  - since group order comes first things 
will be as expected

or

  add an abstract method to get the options to CommandLineParser and 
have the extensions implement that - it can then be called before the 
standard frysk options group is created

as you suggest, doing this as a side effect of parse is asking for trouble.

Andrew



     parser = new Parser(name, version, true);
-    parser.add(new LogOption("debug"));
-    add(new Option("noexe", "Do not attempt to read an"+
-               " executable for a corefile ") {
-        public void parsed(String exeValue) throws OptionException {
-            extendedCore = false;
-            explicitExe = null;
-        }
-        });
-    add(new Option("exe",
-               "Specify the full path of the executable to read",
-               "<executable>") {
-        public void parsed(String exeValue) throws OptionException {
-            extendedCore = true;
-            explicitExe = exeValue;
-        }
-        });

Re: Patch: Add Option Groups

[Phil Muldoon]

Andrew Cagney wrote:
> Phil,
>
>     public CommandlineParser(String name, String version) {
>
> I'd either:
>
>  create the utility specific group, and then the standard frysk option 
> group and all the standard frysk options here; and then have "add" add 
> to the utility specific group  - since group order comes first things 
> will be as expected

Don't know the name of the group so far ahead of time. The Option group 
name has to be created at group creation time. And I really want the 
freedom given to the user to create as many groups as they like, so 
complex utilities like ftrace can have many groups.

>
> or
>
>  add an abstract method to get the options to CommandLineParser and 
> have the extensions implement that - it can then be called before the 
> standard frysk options group is created
>

Still going to have to have the build custom options, then build 
standard options call somewhere in CommandlineParser unless we tell the 
user to explicitly to include the default options themselves.  Also 
means a relationship change. Right now CommandlineParser is called, not 
extended.

Nevertheless something like:

public abstract buildCustomOptions

then in subclass

buildCustomOptions {
   OptionGroup foo = new OptionGroup("Foo Group");
   parser.add(foo).
   foo.add(new Option(...))

   OptionGroup foo2= new OptionGrouo("Extended Foo Group");
   parser.add(foo2).
   foo2.add(new Option(...))

   ...
   ...
   ...
   super.buildDefaultOptions()  ;
}

Something like this?

Re: Patch: Add Option Groups

[Phil Muldoon]

Tom Tromey wrote:
> Yeah, this is a bit odd, but I guess harmless for the time being.
> Maybe it would be better to either push new functionality for this
> kind of thing into Parser, or to have users of frysk's
> CommandlineParser be a bit smarter about coordinating with the
> superclass.
>
>   
A lot of the code in the bindir utilities is template code that all 
utilities call. It saves a lot of work, but also takes away flexibility. 
It's all a bit new to me too. But see my reply to Andrew for a few 
thoughts there.

> Phil> There is a lot of weird
> Phil> indenting going on in the right help column, but this was happening
> Phil> beforehand and is another unrelated (but hopefully soon to be fixed)
> Phil> bug.
>
> Some of this is accounted for by rogue spaces, e.g.:
>
> Phil> +      group.add(new Option( "allmaps", 'a',
> Phil> +	                          " Include ALL process readable maps.")
>
>   

Ack well spotted on that one.


> That space before "Include" is strange.
>
> Phil> +				 "=PATTERN") {
> Putting the "=" in here is wrong, fwiw.
>
>   

I was trying to follow grep's help, but the only way I can get an 
option=PATTERN is to put the = in there. Unless I am using the wrong 
option type?

FWIW also it seems that classpath getopt is doing some funky indention 
on help right hand column.

Thanks for the comments!

Regards

Phil

Re: Patch: Add Option Groups

[Tom Tromey]

Phil> I was trying to follow grep's help, but the only way I can get an
Phil> option=PATTERN is to put the = in there. Unless I am using the wrong
Phil> option type?

Nope, getopt accepts either a separate option or a joined option with
an '='; we just chose to print the space.  I don't recall why.

Tom

Re: Patch: Add Option Groups

[Andrew Cagney]

Phil Muldoon wrote:
> Andrew Cagney wrote:
>> Phil,
>>
>>     public CommandlineParser(String name, String version) {
>>
>> I'd either:
>>
>>  create the utility specific group, and then the standard frysk 
>> option group and all the standard frysk options here; and then have 
>> "add" add to the utility specific group  - since group order comes 
>> first things will be as expected
>
> Don't know the name of the group so far ahead of time. The Option 
> group name has to be created at group creation time. And I really want 
> the freedom given to the user to create as many groups as they like, 
> so complex utilities like ftrace can have many groups.
Good point; there's also a getopt "feature" we need to avoid, things 
need to be created using the sequence:

   - create group
   - add group members
   - add group to parser

otherwise, while they show up in the help message, they don't work.  
Quick read of Parser.add(OptionGroup) shows why.

>
>>
>> or
>>
>>  add an abstract method to get the options to CommandLineParser and 
>> have the extensions implement that - it can then be called before the 
>> standard frysk options group is created
>>
>
> Still going to have to have the build custom options, then build 
> standard options call somewhere in CommandlineParser unless we tell 
> the user to explicitly to include the default options themselves.  
> Also means a relationship change. Right now CommandlineParser is 
> called, not extended.
>
Phil and I discussed this in yesterday's meeting and came up with just 
adding a list of OptionGroup[]s to the CommandLine constructor, but more 
change but ends up being much simpler.

Andrew

> Nevertheless something like:
>
> public abstract buildCustomOptions
>
> then in subclass
>
> buildCustomOptions {
>   OptionGroup foo = new OptionGroup("Foo Group");
>   parser.add(foo).
>   foo.add(new Option(...))
>
>   OptionGroup foo2= new OptionGrouo("Extended Foo Group");
>   parser.add(foo2).
>   foo2.add(new Option(...))
>
>   ...
>   ...
>   ...
>   super.buildDefaultOptions()  ;
> }
>
> Something like this?

Re: Patch: Add Option Groups

[Tom Tromey]

>>>>> "Andrew" == Andrew Cagney <cagney@redhat.com> writes:

Andrew> Good point; there's also a getopt "feature" we need to avoid, things
Andrew> need to be created using the sequence:

Andrew>   - create group
Andrew>   - add group members
Andrew>   - add group to parser

Andrew> otherwise, while they show up in the help message, they don't work.
Andrew> Quick read of Parser.add(OptionGroup) shows why.

Could you spell it out?
If there is a bug, I will fix it.

Tom

Re: Patch: Add Option Groups

[Andrew Cagney]

Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Cagney <cagney@redhat.com> writes:
>>>>>>             
>
> Andrew> Good point; there's also a getopt "feature" we need to avoid, things
> Andrew> need to be created using the sequence:
>
> Andrew>   - create group
> Andrew>   - add group members
> Andrew>   - add group to parser
>
> Andrew> otherwise, while they show up in the help message, they don't work.
> Andrew> Quick read of Parser.add(OptionGroup) shows why.
>
> Could you spell it out?
> If there is a bug, I will fix it.
>   

Contrast it with the sequence:

   - create group
  - add group members (these are in help and work)
  - add group to parser
  - add group members (these are in help, yet don't work)

parser.add(group)'s implementation just iterates over the options in the 
group registered at that point (adding them to the lower-level option 
parser);  this means options added to the group after the call, while 
members of the group, are not registered known to lower-level parser 
code.  This leads to:

- -help listing the options (it's iterating over group members)
yet
- the options not appearing to work

I'm not sure if its an interface restriction, or a bug; either way it 
leads to a confusing client problem.

Andrew

> Tom
>   

Re: Patch: Add Option Groups

[Tom Tromey]

>>>>> "Andrew" == Andrew Cagney <cagney@redhat.com> writes:

[ Parser and OptionGroup ]
Andrew> I'm not sure if its an interface restriction, or a bug; either way it
Andrew> leads to a confusing client problem.

I checked in a fix to the Classpath repository.
I've appended it as well.

Tom

ChangeLog:
2008-03-20  Tom Tromey  <tromey@redhat.com>

	* tools/gnu/classpath/tools/getopt/Parser.java (options): Don't
	initialize.
	(add, addFinal): Don't update options.
	(requireOptions): New method.
	(printHelp): Synchronize.  Call requireOptions.
	(parse): Call requireOptions.

Index: tools/gnu/classpath/tools/getopt/Parser.java
===================================================================
RCS file: /cvsroot/classpath/classpath/tools/gnu/classpath/tools/getopt/Parser.java,v
retrieving revision 1.9
diff -u -r1.9 Parser.java
--- tools/gnu/classpath/tools/getopt/Parser.java	22 Sep 2006 01:01:26 -0000	1.9
+++ tools/gnu/classpath/tools/getopt/Parser.java	20 Mar 2008 18:02:58 -0000
@@ -1,5 +1,5 @@
 /* Parser.java - parse command line options
- Copyright (C) 2006 Free Software Foundation, Inc.
+ Copyright (C) 2006, 2008 Free Software Foundation, Inc.
 
  This file is part of GNU Classpath.
 
@@ -66,7 +66,9 @@
 
   private boolean longOnly;
 
-  private ArrayList options = new ArrayList();
+  // All of the options.  This is null initially; users must call
+  // requireOptions before access.
+  private ArrayList options;
 
   private ArrayList optionGroups = new ArrayList();
 
@@ -218,7 +220,6 @@
    */
   public synchronized void add(Option opt)
   {
-    options.add(opt);
     defaultGroup.add(opt);
   }
 
@@ -230,7 +231,6 @@
    */
   protected synchronized void addFinal(Option opt)
   {
-    options.add(opt);
     finalGroup.add(opt);
   }
 
@@ -242,7 +242,6 @@
    */
   public synchronized void add(OptionGroup group)
   {
-    options.addAll(group.options);
     // This ensures that the final group always appears at the end
     // of the options.
     if (optionGroups.isEmpty())
@@ -251,13 +250,29 @@
       optionGroups.add(optionGroups.size() - 1, group);
   }
 
+  // Make sure the 'options' field is properly initialized.
+  private void requireOptions()
+  {
+    if (options != null)
+      return;
+    options = new ArrayList();
+    Iterator it = optionGroups.iterator();
+    while (it.hasNext())
+      {
+	OptionGroup group = (OptionGroup) it.next();
+	options.addAll(group.options);
+      }
+  }
+
   public void printHelp()
   {
     this.printHelp(System.out);
   }
 
-  void printHelp(PrintStream out)
+  synchronized void printHelp(PrintStream out)
   {
+    requireOptions();
+
     if (headerText != null)
       {
         formatText(out, headerText);
@@ -417,6 +432,7 @@
    */
   public synchronized void parse(String[] inArgs, FileArgumentCallback files)
   {
+    requireOptions();
     try
       {
         args = inArgs;

Re: Patch: Add Option Groups

[Andrew Cagney]

Thanks!

I've also applied it locally.

Andrew

Tom Tromey wrote:
>>>>>> "Andrew" == Andrew Cagney <cagney@redhat.com> writes:
>>>>>>             
>
> [ Parser and OptionGroup ]
> Andrew> I'm not sure if its an interface restriction, or a bug; either way it
> Andrew> leads to a confusing client problem.
>
> I checked in a fix to the Classpath repository.
> I've appended it as well.
>
> Tom
>
> ChangeLog:
> 2008-03-20  Tom Tromey  <tromey@redhat.com>
>
> 	* tools/gnu/classpath/tools/getopt/Parser.java (options): Don't
> 	initialize.
> 	(add, addFinal): Don't update options.
> 	(requireOptions): New method.
> 	(printHelp): Synchronize.  Call requireOptions.
> 	(parse): Call requireOptions.
>
> Index: tools/gnu/classpath/tools/getopt/Parser.java
> ===================================================================
> RCS file: /cvsroot/classpath/classpath/tools/gnu/classpath/tools/getopt/Parser.java,v
> retrieving revision 1.9
> diff -u -r1.9 Parser.java
> --- tools/gnu/classpath/tools/getopt/Parser.java	22 Sep 2006 01:01:26 -0000	1.9
> +++ tools/gnu/classpath/tools/getopt/Parser.java	20 Mar 2008 18:02:58 -0000
> @@ -1,5 +1,5 @@
>  /* Parser.java - parse command line options
> - Copyright (C) 2006 Free Software Foundation, Inc.
> + Copyright (C) 2006, 2008 Free Software Foundation, Inc.
>  
>   This file is part of GNU Classpath.
>  
> @@ -66,7 +66,9 @@
>  
>    private boolean longOnly;
>  
> -  private ArrayList options = new ArrayList();
> +  // All of the options.  This is null initially; users must call
> +  // requireOptions before access.
> +  private ArrayList options;
>  
>    private ArrayList optionGroups = new ArrayList();
>  
> @@ -218,7 +220,6 @@
>     */
>    public synchronized void add(Option opt)
>    {
> -    options.add(opt);
>      defaultGroup.add(opt);
>    }
>  
> @@ -230,7 +231,6 @@
>     */
>    protected synchronized void addFinal(Option opt)
>    {
> -    options.add(opt);
>      finalGroup.add(opt);
>    }
>  
> @@ -242,7 +242,6 @@
>     */
>    public synchronized void add(OptionGroup group)
>    {
> -    options.addAll(group.options);
>      // This ensures that the final group always appears at the end
>      // of the options.
>      if (optionGroups.isEmpty())
> @@ -251,13 +250,29 @@
>        optionGroups.add(optionGroups.size() - 1, group);
>    }
>  
> +  // Make sure the 'options' field is properly initialized.
> +  private void requireOptions()
> +  {
> +    if (options != null)
> +      return;
> +    options = new ArrayList();
> +    Iterator it = optionGroups.iterator();
> +    while (it.hasNext())
> +      {
> +	OptionGroup group = (OptionGroup) it.next();
> +	options.addAll(group.options);
> +      }
> +  }
> +
>    public void printHelp()
>    {
>      this.printHelp(System.out);
>    }
>  
> -  void printHelp(PrintStream out)
> +  synchronized void printHelp(PrintStream out)
>    {
> +    requireOptions();
> +
>      if (headerText != null)
>        {
>          formatText(out, headerText);
> @@ -417,6 +432,7 @@
>     */
>    public synchronized void parse(String[] inArgs, FileArgumentCallback files)
>    {
> +    requireOptions();
>      try
>        {
>          args = inArgs;
>