[boost] Re: CLA/Config file library opinion

From: Vladimir Prus (ghost_at_[hidden])
Date: 2003-05-27 06:25:24

Gennadiy Rozental wrote:

>> > 1. Terminology
>>
>> I'm not claiming the definitions are perfect, but I do claim it's not
>> 100% broken.
> 
> Try to provide a definition for term 'option' the way it's used in your
> library. You will immediately find conflict.

"option" is (name,value) pair
"argument" is an command line element which is not option.

>> > 2. Layered design.
> 
> You would not argue specific subpoints of this issue. 

"You"? Did you mean "I"?

> IMO All of my
> concerns are unfortunate consequences of the way you designed your
> library. IMO there should be one typed storage for the parsed arguments
> (instead of 3 like you have) linked with formal parameters description.

You are not correct. There are two storage mechanism for parsed data. First
('options_and_arguments') stores *only* strings. Second ('variables_map')
stores typed values. So, there's one typed storage.

> Plus second layer managing composition of parameter originated by
> different sources.

That's 'variables_map'.

> The question was: could I use, for example, / as a separator and retrieve
> values from variable_map

The question is still vague... "/" as separator in command line? Yes, you
can.

> 
>> > This has many
>> > problems:
>> >    a. Limit number of supported styles die to bitmap limitation
>>
>> The number of directly supported styles will always be limited. For all
>> others, users will be compelled to write they code --be it a function,
>> or a policy.
> 
> It's only limited in your design.

You've missed the word "directly". You can't have all possible styles work
out of the box, because the number of possible styles is infitite. 

>> >    b. Force same style for all parameters in command line (I could not
>> > define /h --my_long_param)
>>
>> Do you need it in practice?
> 
> Why not. Maybe different modules require parameters in different style.

I feel this to be artifical example, and would not be surprised if no user
of program_options will ever want it. And if it wants... custom parsers 
can be used. 

>> Secondly, "macaroni", as you put it here, sounds a bit... unconstructive.
>> You don't even try to name the functions that you find poorly written.
> 
> Whatever way you name it, it will always be if this style is on if that
> style is on and so on. I do not need to  look onto specific
> implementation- it will always be this way. 
> Also IIRC there some
> conflicting styles that are unreasonably difficult to implement within
> bounds of one parser - and no need to do so.

For now, I can only say that your suggestion is to implement command line
parsing as you've did it. OK, my implementation still works. Should you
sumbit your library I'll be able to look how did you manage to support 
sticky options, for example.

>>   desc.add_options()
>>        // first argument is options name
>>        // second argument is parameter name
>>        // third argument is description
>>        ("output", "file", "output file")
>>        ....
>>        ;
>>
>> This means that one comment line is added for first argument, and then
>> you can very conveniently specify option name.
> 
> Poor maintenance programmer looked on this code and spread hands puzzled:
> What is option name and what is parameter name???? Which one should I use
> to identify argument? Where the value assigned? What is default value?

Sorry, but this kind of questions can be answered only in docs.

>> I subscribe to the view in Boost coding guidelines by David Abrahams,
> where
>> he says that inlining is either second or third (don't remember exactly)
>> most misused feature in C++.
>> If a library can be implemented externally, it should. Linking to a
> library
>> is minor problem. Inlining/templating will only
>> - add compilation time. E.g. profile build of a simple program which uses
>> BGL takes several minutes for me. Yes, program_options is not as large,
> but
>> what if you add 20 small libraries...
> 
> Execution Monitor incomparably smaller. Weren't you the one who requested
> inline version of it?

No. At least I don't remeber having asked for it. 

>> - add distribution size. E.g. if program_options is accepted in Boost,
> next
> 
> It's really minor point. We use CLA parsing only once in application.

Who's "we"? My project has at least 3 command line applications and there's
no need to compile program_options three times. 

>> > 7. Overblown interfaces
>> >
>> > In my taste interface to many major classes unreasonably overblown,
> which
>> > is not a kind of practice we should promote. See cmdline and
>> > option_description interface for example.
>>
>> I can only say that my taste differs. The level of cohesion of
>> option_description seems pretty high for me.
> 
> class cmdline access methods:
>   option_name()
>   option_index()
>   raw_option_name();
>   option_value();
>   option_values();
>   argument();
>   arguments();
> 
> You obviously struggle  from need to provide on one hand container
> interface, on the other iterator one. 
> Plus instead of N methods returning
> properties of option there should be one that returns foe example 'option'
> structure.

... which 'option' structure would contain the same set of methods? Let's
consider the needs of 'cmdline' clients. You have

   cmdline cmd(....);
   while(++cmd) {
        if (cmd.at_option()) {
                ...
        }
   }

With iterators, that would become

   for(cmdline_iterator i(...), e; i != e; ++i) {
        if (i->is_option()) {

Wait! Again, what's 'value_type'. Some imaginary 'option_or_argument' class?
boost::variant? boost::any? Yep, that would be an interator, but you gain
nothing. It will be only input iterator, and with such a specialized
'value_type', that to use that iterator, you need to known that you're
dealing with command line --- there's no place for genericity.

I'd probably buy your argument if you'd say that one can devise class
'option' and make command line parser, config file parser and something
else parser iterator, with 'option' as the value type. That would be nice,
but it can't be done. cmdline has addition 'raw_option_name' member
function, and config_file cannot return arguments. The interfaces are a bit
different.

>> > 8. Separators handling
>> >
>> > Library silently assign special meaning to space symbol. The way
>> > parsing is designed there is no way to use parameter names with spaces
>> > inside.
>>
>> Right -- in command line. Do you need it there?
> 
> I did not get you answer. What I need is an ability to write like this:
> 
> my_prog --service name=>Repository ...
> 
>  Here parameter name is service name, separator is string "=>", value is
> 'Repository'.

Nope. It's not supported by cmdline class.

>> > 9. Multi-pass parsing support
>> >
>> > I did not find any support for the multi-pass parsing. What I mean is
> that
>> > every parser parse CLAs and eats what it understand and leave the rest.
>> > It's important feature that require special support inside the library
>>
>> Could you please clarify why it's important feature? The proposed library
>> allows to combine options_description instances, parse the command line,
>> and then pass the result to interested modules.
> 
> It's important for me as Boost.Test developer ;-). I need to swallow
> Boost.Test parameters and pass the rest to the user program. I also met
> the same logic several times in past (with Visibroker for example). And
> you methods would not work here.

There's always the Unix solution: separate Boost.Test options and options to
the main program with "--" ;-) 

If that's not sufficient (which I doubt), I can think of a more general
solution. Basically, 'cmdline' class can allow unregistered options. With
a bit of tweaking, it can return unparsed tokens.

>> If you're use error codes, you'd have
>> to check it after calling each function.
> 
> No. I would not. In majority of the cases I do not bother whether any
> intermediate step fails, parsing or validation fails. Whatever happened
> during CLA declaration, parsing and validation I just want to get a
> notification about an error during CLA parsing procedure and have and
> access to the error message. IOW it would look like this:
> 
>   cla::parser p;
>   p << parameter<int>( "name" )
> ....
>   if( ! p.parse(argc,argv) ) {
>       cout << p.error_msg() << endl;
>       p.usage();
>       return 1;
>   }

I don't see here handling of response files or config files... if you were
to add it, would you write the same "if" block a couple of times?

>> > 11 Alternative search algorithms
> 
>> I've expressed this concern some time ago: I feel this example is either
>> artificial, or is not directly related to command line parsing. If you
> want
>> to parse arbitrary syntax, you need a parser.
> 
> Exactly. I need flexible enough parser that could accommodate practically
> any parsing. And I argue that it's not difficult to achieve.

I'd say that most users of program_options would not expect it be be
full-blown parser. As for difficulty of achieving something --- well...
request a review for your library and I'll try to comment on how good your
idea works. (I actually have some concerns about your library already, but
since it has no docs, and you did not posted some of the advertised
policies, I won't discuss it now).

>> >    Moreover in many interfaces library assumes existence on long name,
>> >    short
>> > name, which may not be the case even with name based parameter
>> > identification.
>>
>> I don't understand what you mean here.
> 
> I may not have long name, but only short one, like with old getopt had.

It's not a problem. You'd still use the long name for identification
purposes, but disable long names in command line.

> I may not have short name but only long one, like the style I am
> personally prefer.

What's problem with this case? It's supported out-of-box.

>> > 12. Usage of pointers
>> > The decision to use pointers instead of references for const string and
>> > value location passing looks incorrect to me.
>>
>> Can you clarify what "const string". Why the decision is incorrect?
> 
> Why T* is used instead of T&?

I've explained my position in other posts.

> Why char const* is used instead of std::string const&?

Did you read the rationale? It's explained there.

>> > 15. External parsers support.
>> >    Library only supports one extra external parser per parsing. That
>> >    may be
>> > very limiting in case if I want to implement SAX like, callback based
> CLA
>> > parsing.
>>
>> I don't understand what you mean. Could you clarify?
> 
> I do not want to store parsed values inside the parser at all. Instead I
> want to register for every parameter callback function.

That's another example I don't see any use for. What will those callback
functions do. Can you given some more details why it might be needed?

- Volodya