From 5f01f7bf3f13485d403ffcbae0fbdc66923df02a Mon Sep 17 00:00:00 2001 From: Sascha Ochsenknecht Date: Sat, 5 Dec 2009 08:08:45 +0000 Subject: [PATCH] better detection of ambiguous options, see Ticket #3423 [SVN r58152] --- include/boost/program_options/errors.hpp | 6 ++-- src/options_description.cpp | 35 +++++++++++-------- src/value_semantic.cpp | 6 ++++ test/exception_test.cpp | 44 ++++++++++++++---------- 4 files changed, 56 insertions(+), 35 deletions(-) diff --git a/include/boost/program_options/errors.hpp b/include/boost/program_options/errors.hpp index 3da1709..9dee1d8 100644 --- a/include/boost/program_options/errors.hpp +++ b/include/boost/program_options/errors.hpp @@ -78,17 +78,19 @@ namespace boost { namespace program_options { ambiguous_option(const std::string& name, const std::vector& alternatives) : error(std::string("ambiguous option ").append(name)) - , alternatives(alternatives) + , m_alternatives(alternatives) , m_option_name(name) {} ~ambiguous_option() throw() {} const std::string& get_option_name() const throw(); + + const std::vector& alternatives() const throw(); private: // TODO: copy ctor might throw - std::vector alternatives; + std::vector m_alternatives; std::string m_option_name; }; diff --git a/src/options_description.cpp b/src/options_description.cpp index 821f52f..a9acacf 100644 --- a/src/options_description.cpp +++ b/src/options_description.cpp @@ -273,6 +273,8 @@ namespace boost { namespace program_options { { shared_ptr found; vector approximate_matches; + vector full_matches; + // We use linear search because matching specified option // name with the declared option name need to take care about // case sensitivity and trailing '*' and so we can't use simple map. @@ -284,26 +286,29 @@ namespace boost { namespace program_options { if (r == option_description::no_match) continue; - // If we have a full patch, and an approximate match, - // ignore approximate match instead of reporting error. - // Say, if we have options "all" and "all-chroots", then - // "--all" on the command line should select the first one, - // without ambiguity. - // - // For now, we don't check the situation when there are - // two full matches. - if (r == option_description::full_match) - { - return m_options[i].get(); + { + full_matches.push_back(m_options[i]->key(name)); + } + else + { + // FIXME: the use of 'key' here might not + // be the best approach. + approximate_matches.push_back(m_options[i]->key(name)); } found = m_options[i]; - // FIXME: the use of 'key' here might not - // be the best approach. - approximate_matches.push_back(m_options[i]->key(name)); } - if (approximate_matches.size() > 1) + if (full_matches.size() > 1) + boost::throw_exception( + ambiguous_option(name, full_matches)); + + // If we have a full match, and an approximate match, + // ignore approximate match instead of reporting error. + // Say, if we have options "all" and "all-chroots", then + // "--all" on the command line should select the first one, + // without ambiguity. + if (full_matches.empty() && approximate_matches.size() > 1) boost::throw_exception( ambiguous_option(name, approximate_matches)); diff --git a/src/value_semantic.cpp b/src/value_semantic.cpp index d24a09f..3da5baf 100644 --- a/src/value_semantic.cpp +++ b/src/value_semantic.cpp @@ -227,6 +227,12 @@ namespace boost { namespace program_options { { return m_option_name; } + + const std::vector& + ambiguous_option::alternatives() const throw() + { + return m_alternatives; + } void multiple_values::set_option_name(const std::string& option_name) diff --git a/test/exception_test.cpp b/test/exception_test.cpp index 39de749..31de6be 100644 --- a/test/exception_test.cpp +++ b/test/exception_test.cpp @@ -19,6 +19,30 @@ using namespace std; #include "minitest.hpp" +void test_ambiguous() +{ + options_description desc; + desc.add_options() + ("cfgfile,c", value()->multitoken(), "the config file") + ("output,c", value(), "the output file") + ("output,o", value(), "the output file") + ; + + const char* cmdline[] = {"program", "-c", "file", "-o", "anotherfile"}; + + variables_map vm; + try { + store(parse_command_line(sizeof(cmdline)/sizeof(const char*), + const_cast(cmdline), desc), vm); + } + catch (ambiguous_option& e) + { + BOOST_CHECK_EQUAL(e.alternatives().size(), 2); + BOOST_CHECK_EQUAL(e.get_option_name(), "-c"); + BOOST_CHECK_EQUAL(e.alternatives()[0], "cfgfile"); + BOOST_CHECK_EQUAL(e.alternatives()[1], "output"); + } +} void test_unknown_option() { @@ -101,27 +125,11 @@ void test_multiple_occurrences() int main(int /*ac*/, char** /*av*/) { + test_ambiguous(); test_unknown_option(); test_multiple_values(); test_multiple_occurrences(); - bool helpflag = false; - - options_description desc; - desc.add_options() - ("cfgfile,c", value(), "the configfile") - ("help,h", value(&helpflag)->implicit_value(true), "help") - ; - - const char* cmdline[] = {"program", "--cfgfile", "hugo", "-h", "yes" }; - - variables_map vm; - store(parse_command_line(sizeof(cmdline)/sizeof(const char*), - const_cast(cmdline), desc), vm); - notify(vm); - - cout << " help: " << (helpflag ? "true" : "false") << endl; - - return 0; } +