Re: [boost] [program_options] Improving error text in exceptions

Subject: Re: [boost] [program_options] Improving error text in exceptions
From: Vladimir Prus (vladimir_at_[hidden])
Date: 2011-11-16 02:07:58

Leo Goodstadt wrote:

> On 15 November 2011 09:08, Vladimir Prus <vladimir_at_[hidden] wrote:
> 
>> > Leo Goodstadt wrote:
>> > I would very much like to make the error messages in program_options
>> > exceptions more consistent and informative.
>>
>> I've looked over the error messages you have proposed, and I think it's
>> an overall improvement. I would appreciate patch to make this change.
>> Thanks!
> 
> Cool. Will do that. The error messages meant for programmers I shall
> leave alone.
> 
> This exercise is so tedious that I doubt anyone else is going to do
> more than tinker with the message text so I am keen to do an
> exhaustive job.
> 
> As I mentioned, for command line messages,
> <option '--foo_bar' is ambiguous>
> seems much clearer than than
> <option 'foo_bar' is ambiguous>.
> 
> However, it would be inappropriate to add "--" to config file and dos
> style options.
> 
> The easiest thing would be to add a "prefix" parameter to the
> set_option_name() functions in errors.hpp
> These are called inside store() in variables_map.cpp.
> 
> To add "--" or "/" prefixes as appropriate, is it sufficient to look
> at basic_option::original_tokens and basic_option::string_key?
> 
> A) Use no prefix if original_tokens[0] is unprefixed,
> B) Use the parsed prefix (original_tokens[0]) if
> basic_option::string_key.length() = 1
> C) Prefer "--" for long option names
> 
> The same logic would apply to
> 
> 1) exceptions thrown from cmdline::finish_option() in cmdline.cpp
> 2) options_description::find() (in options_description.cpp) called
> from store() (in variables_map.cpp)
> 3) options_description::find_nothrow() (in options_description.cpp) called from
> cmdline::run(),
> cmdline::finish_option(),
> parse_short_option(), and
> parse_disguised_long_option() (in cmdline.cpp)
> (Despite its name, find_nothrow() can throw()!)
> 
> The difficult case is supporting the correct prefix for
> "required_option" in variables_map::notify() (variables_map.cpp).
> 
> I propose that where missing options can be specified in both
> configuration file and on the command line, the command line should
> take precedence, and "--option_name" displayed in the error message.
> 
> To support this, "basic_parsed_options" needs an extra prefix field,
> which would be set appropriately by
> basic_command_line_parser<charT>::run().
> 
> Does this make sense?

Leo,

to be honest, I am not sure this is not going too far. Sure, if we
have original tokens, it would be best to report the error using those
original tokens, so that for command-line usage, the error message can
be immediately associated with whatever is typed.

For the required options, it feels like adding "--" can actually confuse.
E.g. the parsing style might not even allow "--" at all. Maybe, the wording
can be adjusted depending on whether we have original tokens. E.g.

   --foo is ambigiouous

vs.

    no value specified for 'foo'

depending on whether you have original tokens?

Thanks,

-- 
Vladimir Prus
CodeSourcery / Mentor Graphics
[phone]