[program_options] Improving error text in exceptions
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
Leo Goodstadt wrote:
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?
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 +7 (812) 677-68-40
participants (2)
-
Leo Goodstadt -
Vladimir Prus