diff options
25 files changed, 142 insertions, 257 deletions
diff --git a/src/mongo/db/mongod_options.cpp b/src/mongo/db/mongod_options.cpp index d98ca671a44..90dfc698b99 100644 --- a/src/mongo/db/mongod_options.cpp +++ b/src/mongo/db/mongod_options.cpp @@ -46,9 +46,6 @@ namespace mongo { - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - MongodGlobalParams mongodGlobalParams; extern DiagLog _diaglog; @@ -246,8 +243,13 @@ namespace mongo { "n pretouch threads for applying master/slave operations") .hidden(); + // This is a deprecated option that we are supporting for backwards compatibility + // The first value for this option can be either 'dbpath' or 'run'. + // If it is 'dbpath', mongod prints the dbpath and exits. Any extra values are ignored. + // If it is 'run', mongod runs normally. Providing extra values is an error. options->addOptionChaining("command", "command", moe::StringVector, "command") - .hidden(); + .hidden() + .positional(1, 3); options->addOptionChaining("cacheSize", "cacheSize", moe::Long, "cache size (in MB) for rec store") @@ -281,12 +283,6 @@ namespace mongo { options->addOptionChaining("opIdMem", "opIdMem", moe::Switch, "DEPRECATED") .hidden(); - - ret = options->addPositionalOption(POD("command", moe::String, 3)); - if (!ret.isOK()) { - return ret; - } - return Status::OK(); } diff --git a/src/mongo/db/server_options.cpp b/src/mongo/db/server_options.cpp index 4c935ee2884..9786ebef31b 100644 --- a/src/mongo/db/server_options.cpp +++ b/src/mongo/db/server_options.cpp @@ -98,9 +98,6 @@ namespace { } // namespace - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - ServerGlobalParams serverGlobalParams; Status addGeneralServerOptions(moe::OptionSection* options) { diff --git a/src/mongo/dbtests/framework_options.cpp b/src/mongo/dbtests/framework_options.cpp index 778575d617e..82f07068396 100644 --- a/src/mongo/dbtests/framework_options.cpp +++ b/src/mongo/dbtests/framework_options.cpp @@ -34,9 +34,6 @@ namespace mongo { Status addTestFrameworkOptions(moe::OptionSection* options) { - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - options->addOptionChaining("help", "help,h", moe::Switch, "show this usage information"); options->addOptionChaining("dbpath", "dbpath", moe::String, @@ -73,18 +70,14 @@ namespace mongo { options->addOptionChaining("suites", "suites", moe::StringVector, "test suites to run") - .hidden(); + .hidden() + .positional(1, -1); options->addOptionChaining("nopreallocj", "nopreallocj", moe::Switch, "disable journal prealloc") .hidden(); - Status ret = options->addPositionalOption(POD("suites", moe::String, -1)); - if (!ret.isOK()) { - return ret; - } - return Status::OK(); } diff --git a/src/mongo/s/mongos_options.cpp b/src/mongo/s/mongos_options.cpp index 6781531a6e6..1fbdbb61b27 100644 --- a/src/mongo/s/mongos_options.cpp +++ b/src/mongo/s/mongos_options.cpp @@ -31,9 +31,6 @@ namespace mongo { - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - MongosGlobalParams mongosGlobalParams; Status addMongosOptions(moe::OptionSection* options) { diff --git a/src/mongo/shell/shell_options.cpp b/src/mongo/shell/shell_options.cpp index 1f35360a940..e344868064a 100644 --- a/src/mongo/shell/shell_options.cpp +++ b/src/mongo/shell/shell_options.cpp @@ -33,9 +33,6 @@ namespace mongo { Status addMongoShellOptions(moe::OptionSection* options) { - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - options->addOptionChaining("shell", "shell", moe::Switch, "run the shell after executing files"); @@ -86,10 +83,12 @@ namespace mongo { #endif options->addOptionChaining("dbaddress", "dbaddress", moe::String, "dbaddress") - .hidden(); + .hidden() + .positional(1, 1); options->addOptionChaining("files", "files", moe::StringVector, "files") - .hidden(); + .hidden() + .positional(2, -1); // for testing, kill op will also be disabled automatically if the tests starts a mongo // program @@ -101,15 +100,6 @@ namespace mongo { .hidden(); - ret = options->addPositionalOption(POD("dbaddress", moe::String, 1)); - if (!ret.isOK()) { - return ret; - } - ret = options->addPositionalOption(POD("files", moe::String, -1)); - if (!ret.isOK()) { - return ret; - } - return Status::OK(); } diff --git a/src/mongo/tools/bsondump_options.cpp b/src/mongo/tools/bsondump_options.cpp index 308abc40199..aa924c48e63 100644 --- a/src/mongo/tools/bsondump_options.cpp +++ b/src/mongo/tools/bsondump_options.cpp @@ -23,9 +23,6 @@ namespace mongo { BSONDumpGlobalParams bsonDumpGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addBSONDumpOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -40,11 +37,11 @@ namespace mongo { options->addOptionChaining("type", "type", moe::String, "type of output: json,debug") .setDefault(moe::Value(std::string("json"))); + options->addOptionChaining("file", "file", moe::String, "path to BSON file to dump") + .hidden() + .setSources(moe::SourceCommandLine) + .positional(1, 1); - ret = options->addPositionalOption(POD( "file", moe::String, 1 )); - if(!ret.isOK()) { - return ret; - } return Status::OK(); } diff --git a/src/mongo/tools/bsondump_options_test.cpp b/src/mongo/tools/bsondump_options_test.cpp index 7211129db03..3632245dd9b 100644 --- a/src/mongo/tools/bsondump_options_test.cpp +++ b/src/mongo/tools/bsondump_options_test.cpp @@ -244,7 +244,7 @@ namespace { else if (iterator->_dottedName == "file") { ASSERT_EQUALS(iterator->_singleName, "file"); ASSERT_EQUALS(iterator->_type, moe::String); - ASSERT_EQUALS(iterator->_description, "hidden description"); + ASSERT_EQUALS(iterator->_description, "path to BSON file to dump"); ASSERT_EQUALS(iterator->_isVisible, false); ASSERT_TRUE(iterator->_default.isEmpty()); ASSERT_TRUE(iterator->_implicit.isEmpty()); diff --git a/src/mongo/tools/mongobridge_options.cpp b/src/mongo/tools/mongobridge_options.cpp index dec9e65117c..cfcc511cbf3 100644 --- a/src/mongo/tools/mongobridge_options.cpp +++ b/src/mongo/tools/mongobridge_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoBridgeGlobalParams mongoBridgeGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoBridgeOptions(moe::OptionSection* options) { options->addOptionChaining("help", "help", moe::Switch, "produce help message"); diff --git a/src/mongo/tools/mongodump_options.cpp b/src/mongo/tools/mongodump_options.cpp index 13812da2ded..806db39a0d3 100644 --- a/src/mongo/tools/mongodump_options.cpp +++ b/src/mongo/tools/mongodump_options.cpp @@ -24,9 +24,6 @@ namespace mongo { MongoDumpGlobalParams mongoDumpGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoDumpOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { diff --git a/src/mongo/tools/mongoexport_options.cpp b/src/mongo/tools/mongoexport_options.cpp index 5b3cdc75ffd..ace4f2e8af4 100644 --- a/src/mongo/tools/mongoexport_options.cpp +++ b/src/mongo/tools/mongoexport_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoExportGlobalParams mongoExportGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoExportOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { diff --git a/src/mongo/tools/mongofiles_options.cpp b/src/mongo/tools/mongofiles_options.cpp index 731a7c3426e..067a8b75533 100644 --- a/src/mongo/tools/mongofiles_options.cpp +++ b/src/mongo/tools/mongofiles_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoFilesGlobalParams mongoFilesGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoFilesOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -57,15 +54,18 @@ namespace mongo { options->addOptionChaining("replace", "replace,r", moe::Switch, "Remove other files with same name after PUT"); + options->addOptionChaining("command", "command", moe::String, + "gridfs command to run") + .hidden() + .setSources(moe::SourceCommandLine) + .positional(1, 1); + + options->addOptionChaining("file", "file", moe::String, + "'gridfs filename' with a special meaning for various commands") + .hidden() + .setSources(moe::SourceCommandLine) + .positional(2, 2); - ret = options->addPositionalOption(POD( "command", moe::String, 1 )); - if(!ret.isOK()) { - return ret; - } - ret = options->addPositionalOption(POD( "file", moe::String, 1 )); - if(!ret.isOK()) { - return ret; - } return Status::OK(); } diff --git a/src/mongo/tools/mongofiles_options_test.cpp b/src/mongo/tools/mongofiles_options_test.cpp index 71dc7b4ace8..0571b431ad5 100644 --- a/src/mongo/tools/mongofiles_options_test.cpp +++ b/src/mongo/tools/mongofiles_options_test.cpp @@ -378,7 +378,7 @@ namespace { else if (iterator->_dottedName == "command") { ASSERT_EQUALS(iterator->_singleName, "command"); ASSERT_EQUALS(iterator->_type, moe::String); - ASSERT_EQUALS(iterator->_description, "hidden description"); + ASSERT_EQUALS(iterator->_description, "gridfs command to run"); ASSERT_EQUALS(iterator->_isVisible, false); ASSERT_TRUE(iterator->_default.isEmpty()); ASSERT_TRUE(iterator->_implicit.isEmpty()); @@ -390,7 +390,7 @@ namespace { else if (iterator->_dottedName == "file") { ASSERT_EQUALS(iterator->_singleName, "file"); ASSERT_EQUALS(iterator->_type, moe::String); - ASSERT_EQUALS(iterator->_description, "hidden description"); + ASSERT_EQUALS(iterator->_description, "'gridfs filename' with a special meaning for various commands"); ASSERT_EQUALS(iterator->_isVisible, false); ASSERT_TRUE(iterator->_default.isEmpty()); ASSERT_TRUE(iterator->_implicit.isEmpty()); diff --git a/src/mongo/tools/mongoimport_options.cpp b/src/mongo/tools/mongoimport_options.cpp index e63ab15ddf2..b14ddee8837 100644 --- a/src/mongo/tools/mongoimport_options.cpp +++ b/src/mongo/tools/mongoimport_options.cpp @@ -24,9 +24,6 @@ namespace mongo { MongoImportGlobalParams mongoImportGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoImportOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -60,7 +57,8 @@ namespace mongo { "type of file to import. default: json (json,csv,tsv)"); options->addOptionChaining("file", "file", moe::String, - "file to import from; if not specified stdin is used"); + "file to import from; if not specified stdin is used") + .positional(1, 1); options->addOptionChaining("drop", "drop", moe::Switch, "drop collection first "); @@ -86,11 +84,6 @@ namespace mongo { .hidden(); - ret = options->addPositionalOption(POD( "file", moe::String, 1 )); - if(!ret.isOK()) { - return ret; - } - return Status::OK(); } diff --git a/src/mongo/tools/mongooplog_options.cpp b/src/mongo/tools/mongooplog_options.cpp index 0a17d395857..c72d7ab363a 100644 --- a/src/mongo/tools/mongooplog_options.cpp +++ b/src/mongo/tools/mongooplog_options.cpp @@ -24,9 +24,6 @@ namespace mongo { MongoOplogGlobalParams mongoOplogGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoOplogOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { diff --git a/src/mongo/tools/mongorestore_options.cpp b/src/mongo/tools/mongorestore_options.cpp index 8ef847b7f74..5153aa070a6 100644 --- a/src/mongo/tools/mongorestore_options.cpp +++ b/src/mongo/tools/mongorestore_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoRestoreGlobalParams mongoRestoreGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoRestoreOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -77,7 +74,8 @@ namespace mongo { options->addOptionChaining("dir", "dir", moe::String, "directory to restore from") .hidden() - .setDefault(moe::Value(std::string("dump"))); + .setDefault(moe::Value(std::string("dump"))) + .positional(1, 1); // left in for backwards compatibility @@ -86,11 +84,6 @@ namespace mongo { .hidden(); - ret = options->addPositionalOption(POD("dir", moe::String, 1)); - if(!ret.isOK()) { - return ret; - } - return Status::OK(); } diff --git a/src/mongo/tools/mongostat_options.cpp b/src/mongo/tools/mongostat_options.cpp index 3b3a636b306..d57637b21c6 100644 --- a/src/mongo/tools/mongostat_options.cpp +++ b/src/mongo/tools/mongostat_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoStatGlobalParams mongoStatGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoStatOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -52,11 +49,11 @@ namespace mongo { options->addOptionChaining("all", "all", moe::Switch, "all optional fields"); + options->addOptionChaining("sleep", "sleep", moe::Int, "seconds to sleep between samples") + .hidden() + .setSources(moe::SourceCommandLine) + .positional(1, 1); - ret = options->addPositionalOption(POD( "sleep", moe::Int, 1 )); - if(!ret.isOK()) { - return ret; - } return Status::OK(); } diff --git a/src/mongo/tools/mongostat_options_test.cpp b/src/mongo/tools/mongostat_options_test.cpp index e42a370c1fc..373feb67b60 100644 --- a/src/mongo/tools/mongostat_options_test.cpp +++ b/src/mongo/tools/mongostat_options_test.cpp @@ -343,7 +343,7 @@ namespace { else if (iterator->_dottedName == "sleep") { ASSERT_EQUALS(iterator->_singleName, "sleep"); ASSERT_EQUALS(iterator->_type, moe::Int); - ASSERT_EQUALS(iterator->_description, "hidden description"); + ASSERT_EQUALS(iterator->_description, "seconds to sleep between samples"); ASSERT_EQUALS(iterator->_isVisible, false); ASSERT_TRUE(iterator->_default.isEmpty()); ASSERT_TRUE(iterator->_implicit.isEmpty()); diff --git a/src/mongo/tools/mongotop_options.cpp b/src/mongo/tools/mongotop_options.cpp index 1f140d1bb43..c629ec6c64a 100644 --- a/src/mongo/tools/mongotop_options.cpp +++ b/src/mongo/tools/mongotop_options.cpp @@ -23,9 +23,6 @@ namespace mongo { MongoTopGlobalParams mongoTopGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addMongoTopOptions(moe::OptionSection* options) { Status ret = addGeneralToolOptions(options); if (!ret.isOK()) { @@ -40,11 +37,11 @@ namespace mongo { options->addOptionChaining("locks", "locks", moe::Switch, "use db lock info instead of top"); + options->addOptionChaining("sleep", "sleep", moe::Int, "seconds to sleep between samples") + .hidden() + .setSources(moe::SourceCommandLine) + .positional(1, 1); - ret = options->addPositionalOption(POD( "sleep", moe::Int, 1 )); - if(!ret.isOK()) { - return ret; - } return Status::OK(); } diff --git a/src/mongo/tools/mongotop_options_test.cpp b/src/mongo/tools/mongotop_options_test.cpp index 9eb8826520b..7020164e794 100644 --- a/src/mongo/tools/mongotop_options_test.cpp +++ b/src/mongo/tools/mongotop_options_test.cpp @@ -294,7 +294,7 @@ namespace { else if (iterator->_dottedName == "sleep") { ASSERT_EQUALS(iterator->_singleName, "sleep"); ASSERT_EQUALS(iterator->_type, moe::Int); - ASSERT_EQUALS(iterator->_description, "hidden description"); + ASSERT_EQUALS(iterator->_description, "seconds to sleep between samples"); ASSERT_EQUALS(iterator->_isVisible, false); ASSERT_TRUE(iterator->_default.isEmpty()); ASSERT_TRUE(iterator->_implicit.isEmpty()); diff --git a/src/mongo/tools/tool_options.cpp b/src/mongo/tools/tool_options.cpp index 3c93c8a8898..e623ca9dd54 100644 --- a/src/mongo/tools/tool_options.cpp +++ b/src/mongo/tools/tool_options.cpp @@ -36,9 +36,6 @@ namespace mongo { ToolGlobalParams toolGlobalParams; BSONToolGlobalParams bsonToolGlobalParams; - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addGeneralToolOptions(moe::OptionSection* options) { options->addOptionChaining("help", "help", moe::Switch, "produce help message"); diff --git a/src/mongo/util/net/ssl_options.cpp b/src/mongo/util/net/ssl_options.cpp index 4dd4e436726..882244c8754 100644 --- a/src/mongo/util/net/ssl_options.cpp +++ b/src/mongo/util/net/ssl_options.cpp @@ -23,9 +23,6 @@ namespace mongo { - typedef moe::OptionDescription OD; - typedef moe::PositionalOptionDescription POD; - Status addSSLServerOptions(moe::OptionSection* options) { options->addOptionChaining("ssl.sslOnNormalPorts", "sslOnNormalPorts", moe::Switch, "use ssl on configured ports"); diff --git a/src/mongo/util/options_parser/option_description.h b/src/mongo/util/options_parser/option_description.h index 88016a05f2f..cee9a0f2d2c 100644 --- a/src/mongo/util/options_parser/option_description.h +++ b/src/mongo/util/options_parser/option_description.h @@ -51,9 +51,9 @@ namespace optionenvironment { }; /** - * The OptionDescription and PositionalOptionDescription classes are containers for information - * about the options we are expecting either on the command line or in config files. These - * should be registered in an OptionSection instance and passed to an OptionsParser. + * The OptionDescription class is a container for information about the options we are expecting + * either on the command line or in config files. These should be registered in an + * OptionSection instance and passed to an OptionsParser. */ class OptionDescription { public: @@ -155,19 +155,5 @@ namespace optionenvironment { int _positionalEnd; // The ending position if this is a positional option. -1 if unlimited. }; - class PositionalOptionDescription { - public: - PositionalOptionDescription(const std::string& name, - const OptionType type, - int count = 1) - : _name(name), - _type(type), - _count(count) { } - - std::string _name; // Name used to access the value of this option after parsing - OptionType _type; // Storage type of the positional argument (required by boost) - int _count; // Max number of times this option can be specified. -1 = unlimited - }; - } // namespace optionenvironment } // namespace mongo diff --git a/src/mongo/util/options_parser/option_section.cpp b/src/mongo/util/options_parser/option_section.cpp index 6a7f8c268f2..26853f30f63 100644 --- a/src/mongo/util/options_parser/option_section.cpp +++ b/src/mongo/util/options_parser/option_section.cpp @@ -71,82 +71,6 @@ namespace optionenvironment { return _options.back(); } - Status OptionSection::addPositionalOption(const PositionalOptionDescription& positionalOption) { - - std::list<OptionDescription>::iterator oditerator; - - // Get place where this positional option should be - int nextPosition = 1; - for (oditerator = _options.begin(); - oditerator != _options.end(); oditerator++) { - - // This option is positional, so we need to advance nextPosition - if (oditerator->_positionalStart != -1) { - - // Verify that the name for this positional option does not conflict with the name - // for any positional option we have already registered - if (positionalOption._name == oditerator->_dottedName || - positionalOption._name == oditerator->_singleName) { - StringBuilder sb; - sb << "Attempted to register duplicate positional option: " - << positionalOption._name; - return Status(ErrorCodes::InternalError, sb.str()); - } - - // Verify that we are not trying to register a positional option with a section that - // already has one with an infinite count - if (oditerator->_positionalEnd == -1) { - StringBuilder sb; - sb << "Error, found an exisiting option with infinite count: " - << positionalOption._name; - return Status(ErrorCodes::InternalError, sb.str()); - } - - nextPosition = oditerator->_positionalEnd + 1; - } - } - - int positionalStart = nextPosition; - int positionalEnd = nextPosition; - - if (positionalOption._count == -1) { - positionalEnd = -1; - } - else { - positionalEnd = nextPosition + (positionalOption._count - 1); - } - - // Don't register this positional option if it has already been registered to support - // positional options that we also want to be visible command line flags - // - // TODO: More robust way to do this. This whole function only works if we register the - // flag first. Make everything use the chaining interface and remove this function. - for (oditerator = _options.begin(); oditerator != _options.end(); oditerator++) { - if (positionalOption._name == oditerator->_dottedName) { - oditerator->positional(positionalStart, positionalEnd); - return Status::OK(); - } - - if (positionalOption._name == oditerator->_singleName) { - oditerator->positional(positionalStart, positionalEnd); - return Status::OK(); - } - } - - try { - addOptionChaining(positionalOption._name, positionalOption._name, - positionalOption._type, "hidden description") - .hidden() - .setSources(SourceCommandLine) - .positional(positionalStart, positionalEnd); - } - catch (DBException &e) { - return e.toStatus(); - } - - return Status::OK(); - } - // Stuff for dealing with Boost namespace { @@ -446,6 +370,41 @@ namespace optionenvironment { return Status::OK(); } + /* + * The way we specify positional options in our interface differs from the way boost does it, so + * we have to convert them here. + * + * For example, to specify positionals such that you can run "./exec [pos1] [pos2] [pos2]": + * + * Our interface: + * + * options.addOptionChaining("pos2", "pos2", moe::StringVector, "Pos2") + * .hidden() <- doesn't show up in help + * .sources(moe::SourceCommandLine) <- only allowed on command line + * .positional(2, <- start position + * 3); <- end position + * options.addOptionChaining("pos1", "pos1", moe::String, "Pos1") + * .hidden() <- doesn't show up in help + * .sources(moe::SourceCommandLine) <- only allowed on command line + * .positional(1, <- start position + * 1); <- end position + * // Note that order doesn't matter + * + * Boost's interface: + * + * boostHiddenOptions->add_options()("pos1", po::value<std::string>(), "Pos1") + * ("pos2", po::value<std::string>(), "Pos2") + * + * boostPositionalOptions->add("pos1", 1); <- count of option (number of times it appears) + * boostPositionalOptions->add("pos2", 2); <- count of option (number of times it appears) + * // Note that order does matter + * + * Because of this, we have to perform the conversion in this function. The tasks performed by + * this function are: + * + * 1. Making sure the ranges are valid as a whole (no overlap or holes) + * 2. Convert to the boost options and add them in the correct order + */ Status OptionSection::getBoostPositionalOptions( po::positional_options_description* boostPositionalOptions) const { @@ -465,6 +424,16 @@ namespace optionenvironment { foundAtPosition = false; std::list<OptionDescription>::iterator poditerator; for (poditerator = positionalOptions.begin(); poditerator != positionalOptions.end();) { + + if (poditerator->_positionalStart < nextPosition) { + StringBuilder sb; + sb << "Found option with overlapping positional range: " + << " Expected next option at position: " << nextPosition + << ", but \"" << poditerator->_dottedName << "\" starts at position: " + << poditerator->_positionalStart; + return Status(ErrorCodes::InternalError, sb.str()); + } + if (poditerator->_positionalStart == nextPosition) { foundAtPosition = true; diff --git a/src/mongo/util/options_parser/option_section.h b/src/mongo/util/options_parser/option_section.h index 4519dc83eb1..9535bf52741 100644 --- a/src/mongo/util/options_parser/option_section.h +++ b/src/mongo/util/options_parser/option_section.h @@ -27,10 +27,11 @@ namespace optionenvironment { namespace po = boost::program_options; - /** A container for OptionDescription instances and PositionalOptionDescription instances as - * well as other OptionSection instances. Provides a description of all options that are - * supported to be passed in to an OptionsParser. Has utility functions to support the various - * formats needed by the parsing process + /** + * A container for OptionDescription instances as well as other OptionSection instances. + * Provides a description of all options that are supported to be passed in to an + * OptionsParser. Has utility functions to support the various formats needed by the parsing + * process * * The sections and section names only matter in the help string. For sections in a JSON * config, look at the dots in the dottedName of the relevant OptionDescription @@ -48,7 +49,7 @@ namespace optionenvironment { * options.addOptionChaining("help", "help", moe::Switch, "Display Help"); * * // Register our positional options with our OptionSection - * options.addPositionalOption(moe::PositionalOptionDescription("command", moe::String)); + * options.addOptionChaining("command", "command", moe::String, "Command").positional(1, 1); * * // Add a subsection * subSection.addOptionChaining("port", "port", moe::Int, "Port"); @@ -102,16 +103,6 @@ namespace optionenvironment { const std::string& singleName, const OptionType type, const std::string& description); - /** - * Add a positional option to this section. Also adds a normal hidden option with the same - * name as the PositionalOptionDescription because that is the mechanism boost program - * options uses. Unfortunately this means that positional options can also be accessed by - * name in the config files and via command line flags - * - * returns bad status on errors, such as attempting to register an option with the same name - * as another option - */ - Status addPositionalOption(const PositionalOptionDescription& positionalOption); // These functions are used by the OptionsParser to make calls into boost::program_options Status getBoostOptions(po::options_description* boostOptions, diff --git a/src/mongo/util/options_parser/options_parser_test.cpp b/src/mongo/util/options_parser/options_parser_test.cpp index c1167ab4b82..7b1a69c0bc6 100644 --- a/src/mongo/util/options_parser/options_parser_test.cpp +++ b/src/mongo/util/options_parser/options_parser_test.cpp @@ -77,10 +77,15 @@ namespace { TEST(Registration, DuplicatePositional) { moe::OptionSection testOpts; - ASSERT_OK(testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::Int))); - ASSERT_NOT_OK(testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::Int))); + try { + testOpts.addOptionChaining("positional", "positional", moe::Int, "Positional") + .positional(1, 1); + testOpts.addOptionChaining("positional", "positional", moe::Int, "Positional") + .positional(1, 1); + FAIL("Was able to register duplicate positional option"); + } + catch (::mongo::DBException &e) { + } } TEST(Registration, BadRangesPositional) { @@ -289,7 +294,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", moe::String)); + testOpts.addOptionChaining("positional", "positional", moe::String, "Positional") + .positional(1, 1); std::vector<std::string> argv; argv.push_back("binaryname"); @@ -309,7 +315,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", moe::String)); + testOpts.addOptionChaining("positional", "positional", moe::String, "Positional") + .positional(1, 1); std::vector<std::string> argv; argv.push_back("binaryname"); @@ -326,7 +333,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", moe::String)); + testOpts.addOptionChaining("positional", "positional", moe::String, "Positional") + .positional(1, 1); testOpts.addOptionChaining("port", "port", moe::Int, "Port"); std::vector<std::string> argv; @@ -348,35 +356,13 @@ namespace { ASSERT_EQUALS(port, 5); } - TEST(Parsing, PositionalAlreadyRegistered) { - moe::OptionsParser parser; - moe::Environment environment; - - moe::OptionSection testOpts; - testOpts.addOptionChaining("positional", "positional", moe::String, - "Positional"); - ASSERT_OK(testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", moe::String))); - - std::vector<std::string> argv; - argv.push_back("binaryname"); - argv.push_back("positional"); - std::map<std::string, std::string> env_map; - - moe::Value value; - ASSERT_OK(parser.run(testOpts, argv, env_map, &environment)); - ASSERT_OK(environment.get(moe::Key("positional"), &value)); - std::string positional; - ASSERT_OK(value.get(&positional)); - ASSERT_EQUALS(positional, "positional"); - } - TEST(Parsing, PositionalMultiple) { moe::OptionsParser parser; moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::StringVector, 2)); + testOpts.addOptionChaining("positional", "positional", moe::StringVector, "Positional") + .positional(1, 2); std::vector<std::string> argv; argv.push_back("binaryname"); @@ -400,8 +386,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::StringVector, 2)); + testOpts.addOptionChaining("positional", "positional", moe::StringVector, "Positional") + .positional(1, 2); std::vector<std::string> argv; argv.push_back("binaryname"); @@ -418,8 +404,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::StringVector, -1)); + testOpts.addOptionChaining("positional", "positional", moe::StringVector, "Positional") + .positional(1, -1); std::vector<std::string> argv; argv.push_back("binaryname"); @@ -452,8 +438,8 @@ namespace { moe::Environment environment; moe::OptionSection testOpts; - testOpts.addPositionalOption(moe::PositionalOptionDescription("positional", - moe::StringVector, 2)); + testOpts.addOptionChaining("positional", "positional", moe::StringVector, "Positional") + .positional(1, 2); testOpts.addOptionChaining("port", "port", moe::Int, "Port"); std::vector<std::string> argv; @@ -2178,6 +2164,27 @@ namespace { testOpts.addOptionChaining("positional1", "positional1", moe::String, "Positional") .positional(1, 1); testOpts.addOptionChaining("positional3", "positional2", moe::StringVector, "Positional") + .positional(1, 2); + testOpts.addOptionChaining("port", "port", moe::Int, "Port"); + + std::vector<std::string> argv; + argv.push_back("binaryname"); + std::map<std::string, std::string> env_map; + + moe::Value value; + std::vector<std::string>::iterator positionalit; + + ASSERT_NOT_OK(parser.run(testOpts, argv, env_map, &environment)); + } + + TEST(ChainingInterface, PositionalOverlappingRangeInfinite) { + moe::OptionsParser parser; + moe::Environment environment; + + moe::OptionSection testOpts; + testOpts.addOptionChaining("positional1", "positional1", moe::String, "Positional") + .positional(1, 1); + testOpts.addOptionChaining("positional3", "positional2", moe::StringVector, "Positional") .positional(1, -1); testOpts.addOptionChaining("port", "port", moe::Int, "Port"); |