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

[Tobias Grosser]

On 05/02/2013 10:14 PM, Daniel Liew wrote:
> 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.

Hi Daniel,

thanks for addressing my comments. The new patch looks already very
nice. I tested it and it continues to work as expected. I have a last
set of minor stylistic comments, but the patch seems to be OK after
those have been addressed.  Maybe you can repost the patch with the last
fixes, such that Andrew can have another look.

Thanks,
Tobias

> +Grouping options into categories
> +--------------------------------
> +
> +If our program has a large number of options it may become difficult for users
> +of your tool to navigate the output of ``-help``. To alleviate this problem we
... of our tool ...

Either address the reader using 'you' or 'us'. Mixing those forms in a
single sentence seems confusing.

> +can declare option categories by statically declaring a `cl::OptionCategory`_
> +and then place our options into these categories using the `cl::cat`_ option
> +attribute.
> +
> +.. code-block:: c++
> +
> +  cl::OptionCategory StageSelectionCat("Stage Selection Options",
> +                                       "These control which stages are run.");
> +
> +  cl::opt<bool> preprocessor("E",cl::desc("Run preprocessor stage."),
> +                             cl::cat(StageSelectionCat));
> +
> +  cl::opt<bool> noLink("c",cl::desc("Run all stages except linking."),
> +                       cl::cat(StageSelectionCat));
> +

I would call the previous options PreProcessor and NoLink according to:
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

>  Option Modifiers
>  ----------------
>
> @@ -1385,6 +1438,27 @@ For example:
>
>    cl::extrahelp("\nADDITIONAL HELP:\n\n  This is the extra help\n");
>
> +.. _cl::OptionCategory:
> +
> +The ``cl::OptionCategory`` class
> +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> +
> +The ``cl::OptionCategory`` class is a simple class for declaring
> +Option categories.
> +
> +.. code-block:: c++
> +
> +  namespace cl {
> +    class OptionCategory;
> +  }
> +
> +A option category must have a name and optionally a description which are passed
> +to the constructor as ``const char*``.
> +
> +Note that declaring an option category before parsing options(e.g. statically)
                                                                 ^ space?

I would add a space between options and '('.

>  //===----------------------------------------------------------------------===//
>  // Basic, shared command line option processing machinery.
> @@ -528,6 +540,7 @@ static void ExpandResponseFiles(unsigned argc, const char*const* argv,
>    }
>  }
>
> +

You add an unnecessary newline here.

>  static cl::opt<bool>
>  PrintOptions("print-options",
> @@ -1306,6 +1447,24 @@ PrintAllOptions("print-all-options",
>                  cl::desc("Print all option values after command line parsing"),
>                  cl::Hidden, cl::init(false));
>
> +void HelpPrinterWrapper::operator=(bool Value) {
> +  if (Value == false)
> +    return;
> +
> +  // Decide which printer to invoke. If more than one option category is
> +  // registered then it is useful to show the categorized help instead of
> +  // uncategorized help.
> +  if (RegisteredOptionCategories->size() > 1) {
> +    // Using categorized printer so should unhide -help-list so user can have
                                    ^^^^ This 'so' seems useless and a
little confusing.

Cheers,
Tobi