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

[Daniel Liew]

Oh dear there was a mistake in the first patch I gave you. I forgot to
remove comments relating to -help-cat and -help-cat-hidden (which was
part my old implementation)

I've attached the amended patch and also the patch for
cl::getRegisteredOptions() feature (now has documentation as part of the
patch and line endings cleaned up)

Sorry for the confusion.

Thanks,
Dan.

On 02/05/13 20:13, Daniel Liew wrote:
> Attached is my patch for adding command line option categories with the
> documentation and with fixes to the style issues you previously mentioned.
> 
> On 02/05/13 18:36, Tobias Grosser wrote:
>> On 05/02/2013 07:27 PM, Daniel Liew wrote:
>>> Thanks for taking the time to review. Please see my responses below.
>>>>> +  //Unhide -help-list if necessary
>>>>         ^ Space?                      ^ Point?
>>> Did you mean 'Point' as in 'what is the point?' or missing fullstop?
>>
>> I meant 'fullstop'. Sorry for my english.
>>
>>>>> +  // Make sure we inherit our base class's operator=()
>>>>> +  using HelpPrinter::operator=;
>>>>> +protected:
>>>>> +  virtual void printOptions(StrOptionPairVector &Opts, size_t
>>>>> MaxArgLen) {
>>>>> +    std::vector<OptionCategory*> SortedCategories;
>>>>> +    std::map<OptionCategory*,std::vector<Option*> >
>>>>> CategorizedOptions;
>>>>> +
>>>>> +    // Find the different option groups and put into vector ready for
>>>>> sorting.
>>>>
>>>> Thi sentence sounds grammatically a little uncommon.
>>>
>>> I'm not sure which sentence you are referring to "Make sure.." or "Find
>>> the different...".  I'll assume that later, would prefer it to be
>>> phrased as...
>>
>> The later.
>>
>>> "Collect registered option groups in a vector in preparation for
>>> sorting."?
>>
>> This sounds good.
>>
>>>>> +    for (OptionCatSet::const_iterator
>>>>> +         I = RegisteredOptionCategories->begin(),
>>>>> +         E = RegisteredOptionCategories->end();
>>>>> +         I != E; ++I)
>>>>> +      SortedCategories.push_back(*I);
>>>>
>>>> I propose clang-format to format this loop.
>>>
>>> I'll see what clang-format spits out. Was it just this loop above you
>>> are concerned about?
>>
>> Just run it over all your code and see which differences it proposes.
>> Not all may be useful, but if there is no good reason to format it
>> differently, I would go for clang-format's choice.
>>
>>>> The argument 'ToUnhide' seems to be unused?
>>>
>>> Oops, yes that was left over from me trying to decide how the -help-list
>>> option should be automatically unhidden.
>>>
>>> I ended up putting the behaviour in a function unHideHelpListOption
>>> cl::ParseCommandLineOptions() instead of the HelpPrinterWrapper class.
>>>
>>> Maybe it would be cleaner if I put that behaviour back into the
>>> HelpPrinterWrapper class (but without the ToUnHide argument)?
>>
>> I don't have a strong opinion here. Do what you believe is better.
>>
>>>> Can you add a short explanation what uncategorized/categorized help
>>>> printers are? Basically that this is a help printer which changes
>>>> behavior depending on the existance of more than one option category.
>>>
>>> I'll add something like that.
>>
>> Thanks,
>> Tobias
>>
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-FIXED-Support-command-line-option-categories.patch
Type: text/x-patch
Size: 18931 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130502/7b146463/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Implemented-public-interface-for-modifying-registere.patch
Type: text/x-patch
Size: 5292 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130502/7b146463/attachment-0001.bin>