[PATCH] Extra Command Line library functionality (option grouping and runtime inspection/modification)

[Daniel Liew]

On 06/05/13 08:39, Tobias Grosser wrote:
> On 05/05/2013 01:07 AM, Dan Liew wrote:
>> Sorry Andrew I didn't read your e-mail correctly the first time. I've
>> added some primitive tests to "unittests/Support/CommandLineTest.cpp" in
>> each patch.
> 
> I found only a single primitive test. Was that your definition of 'some'
> or did I miss some tests?

Sorry I should of said that I added a single test in each patch

TEST(CommandLineTest, UseOptionCategory) - first patch

and

TEST(CommandLineTest, ModifyExisitingOption) - second patch


>> I have also modified the first patch so that empty option categories are
>> not printed in the output of -help, they are however printed in the
>> output of -help-hidden.
> 
> I looked again through the patch. It looks good and works as expected. I
> also tested the empty option category handling and its behavior looks good.

Great.

>> The modified patches are attached.
>>
>> Something to note is I discovered a problem with one of the tests
>> (CommandLineTest.ParseEnvironmentToLocalVar) in "CommandLineTest.cpp".
>> This test creates a command line option on the stack. This option is
>> subsequently destroyed on leaving the test but the command line system
>> still has a pointer to that option. If an attempt is made to access
>> registered options (e.g. through ParseCommandLineOptions() or
>> getRegisteredOptions() ) after this test the program will SEGFAULT.
>>
>> I worked around this by having my tests that need to access registered
>> options rule run before the test
>> CommandLineTest.ParseEnvironmentToLocalVar.
>>
>> This could be fixed in general by implementing destructors for the
>> Option objects that unregisters them from the command line system but
>> this probably adds unnecessary overhead as the options are intended to
>> be declared statically anyway.
> 
> This seems reasonable, but I defer to Andrew's expertise regarding this
> point as well as what can be tested reasonably.

Okay. Are you happy with the patches as they are Andrew? If so could you
please commit them on my behalf.

> Cheers,
> Tobias

Thanks,
Dan.