[boost] Re: boost:program_options use case and review

From: Vladimir Prus (ghost_at_[hidden])
Date: 2003-06-02 07:58:03

Hi Misha,

Misha Bergal wrote:

> I have 2 use cases for program options library. I would say that they are
> fairly advanced but nevertheless typical ones:
> Due to severe time constraints I am going to describe just the first use
> case, which I think will highlight significant number of
> boost::program_options benefits and problems .

Thanks for finding the time for review!

> ----------------- USE CASE -----------------------------
> 1. Program options of Perforce version control "p4" client.
> p4 program options come from (in the order of descending priority)
> 1. Command line
> 2.standard  input
> 3. config file
> 4. Windows registry
> 
> 1. The command line has the general format:
> p4 [global-options] <command> [command-options]
[...]
> Please note, that "-c" option has semantic is different between
> global-options and command options

While I don't think such difference in semantic makes good UI, I won't argue
this point here. A little bit later, I'll consider how to support it, as
well as support options with the same meaning everywhere.

> 2.Describe options
> 
>  global_options_desc.add_options()
>       ( "c", parameters( "<client>", &go.client ), "some comments here"  )
>        // Problem #1.
>        // The first parameter is a mini-language. Usually C++ tends to
>        avoid
> minilanguages.
>        // For example: iostreams don't use "w+", "rw", "a", although
> iostreams are not always
>        // the example of practices to follow.
>        // Still, for users minilanguage might be unexpected here

I disagree. For iostreams, all those flags are not used all that often. For
program_options they are likely to be present quite often.

>        // Note: need to make sure that parameter can take optional<>

Good point. Added to todo.

>  3. Read - lowest priority first
> 
>  // 3.1. Read registry and config file
>  // Problem #2 - the names in the registry and config file are not the
>  same
> as the names given in options_description

Yep. It was noted by Gennadiy already, and I'm considering where to add
remaning facility. As you've mentioned in use case description above, the
names in config file can actually be derived from command line names.

>  // --- custom written ---
> mtn::copy( config_file( ".p4config" ), inserter( go ) ); // insert options
> in go (variables_map, or lexical_map)
>  // --- end custom written ---

I assume "mtn::copy" works with containers, right? Still, for this to work,
"config_file" must be container which value_type is something more fancy
than string, while I'd prefer it to be purely syntax parser.

>  // 3.2. Read command line
>  // 3.2.1 Separate global options from command options
>  options_and_arguments oa = parse_command_line(argc, argv, desc);
>  // Problem #3: Would be nice if options_and_arguments was a  sequence
>  command_options_and_arguments = mtn::find_if( options_and_arguments,

I guess the variable declared should be called "command"? Else I don't
understand what's 'command' in the line below.

> &is_argument );
>  global_options_and_arguments  = mtn::iterator_range(
> options_and_arguments.begin(), command.begin() );

I think there are some problems here. In order to parse subprogram options,
you need to register them. Else, how the parser is to tell if "-c foo" is
a option with value, or an options without value and an argument? But you
cannot register subprogram options, for example becuse "-c" is both global
option and a subcommand option.

So, you have to stop parsing options on the first argument, and assume all
other tokens are arguments. I.e. add a style option which tells that
non-options token ends arguments. If such facility is added, then subcommand
options are retrievable via options_and_arguments::arguments() method.

You can pass them to subcommand which can declare it's own
options_description instance (which can include "-c" with different
meaning), and parse it.

Imagine a different approach: all options mean the same everywhere. Some
subcommands however, don't use certain options. Ok, we'll have

   options_description desc()
        // declare all options
        ;

   options_and_arguments oa = parse_command_line(..., desc, ...);
   std::string subcommand = oa.arguments()[0];
   std::set<std::string> allowed = allowed_options[subcommand];
   // validate that all options are allowed for this subcommand.
   
   // everything else is as usual.
   
So, the added complexity is declaring list of options that each subcommand
takes, and a small loop to do checking.

>  // 3.2.2. Find out whether I need to read a rest of arguments from
>  standard
> input
>  // Problem #4: Seems that it is too late to ask boost::program_options to
> do that
>  // May be the arguments need to be read from stdin and the command line
> parsed again?

Yes, I think that's the right way. In fact, it's how response files can be
supported. Hartmut Kaiser already used this approach in Wave and I was
planning to add such support, but haven't yet.

> 
>   // 3.2.3 Store global_options
>  store( global_options_and_arguments , go, desc);
>  or // Problem #5: need to check how to store into  struct global_options
> {}

Per other review comments, I plan to adopt the following 
- the locations to store options will be specified in 'parameter' function,
  just as now
- to actually store values in location, one would have to create
'variables_map' instance and call some function on it. The motivation is
that variables_map class supports chaining, which implements priority of
options. So that priority has the effect on variables assingment, we have
to operate on variables_map.

> Current problems:

> 2. No STL conformity - boost::po objects and data structures are hard to
> operate with.

Let me see. I'm currently planning to hide both 'cmdline' and 'config_file'
classes from public interface. This leaves three classes: variables_map
(derived from std::map), options_description and options_and_arguments.
As far as the second is concerned, the best candidate for container concept
is AssociativeContainer, however, the 'clear'/'erase' seem unnecassary.
Likewise, iteration is not needed. 
As far as the third is concerned, there are two vectors: for options
and arguments -- which you can process as you like. Making the whole class
model of Sequence, or Container would add little or nothing, IMO.

> 3. Need to see how additional storage is supported (lexical_map,
> non-container storage)

Hope the above explains non-container storage. To comment on lexical_map, I
should know more details. It it's like std::map<string, string>, then I
wonder what happens if there are several occurences of the same option.

> 4. It doesn't seem that library can help with config files in this use
> case

You are right, as I've explained above.

> 0 .More sleep :-)
> 1. Some ideas regarding 2,3,4
> 2. Produce a working implementation (make it an boost::program_options
> example - seems like a significant amount of work, may be not now?).

I can try to implement my approach --- with the same meaning of options
everywhere. I can also try to implement the other one, provided I'm
convinced it is reasonable.

Thanks,
Volodya