On 15 November 2011 09:08, Vladimir Prus <vladimir@codesourcery.com 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? Thanks, Leo -- Leo Goodstadt University of Oxford, UK