From: Teemu Murtola Date: Sun, 27 Jul 2014 04:42:53 +0000 (+0300) Subject: Support custom default extension for FileNameOption X-Git-Url: http://biod.pnpi.spb.ru/gitweb/?p=alexxy%2Fgromacs.git;a=commitdiff_plain;h=df876d6c7b594ded534d9b888e84141a054da11d Support custom default extension for FileNameOption Add FileNameOption.defaultType() to allow changing the default extension used for file types that accept multiple extensions. About 1% of the existing t_filenm declarations seem to use such a feature. Make parse_common_args() deduce the value for this from the value of t_filenm.fn, if it contains an extension. Extend unit tests to cover these cases. Change-Id: Idf8d1c6b70b88d58bbf12398bd16f57412eab065 --- diff --git a/src/gromacs/commandline/pargs.cpp b/src/gromacs/commandline/pargs.cpp index d0184d1a83..5a16f3cf48 100644 --- a/src/gromacs/commandline/pargs.cpp +++ b/src/gromacs/commandline/pargs.cpp @@ -329,16 +329,23 @@ void OptionsAdapter::filenmToOptions(Options *options, t_filenm *fnm) const bool bMultiple = ((fnm->flag & ffMULT) != 0); const char *const name = &fnm->opt[1]; const char * defName = fnm->fn; + int defType = -1; if (defName == NULL) { defName = ftp2defnm(fnm->ftp); } + else if (std::strchr(defName, '.') != NULL) + { + defType = fn2ftp(defName); + GMX_RELEASE_ASSERT(defType != efNR, + "File name option specifies an invalid extension"); + } fileNameOptions_.push_back(FileNameData(fnm)); FileNameData &data = fileNameOptions_.back(); data.optionInfo = options->addOption( FileNameOption(name).storeVector(&data.values) - .defaultBasename(defName).legacyType(fnm->ftp) - .legacyOptionalBehavior() + .defaultBasename(defName).defaultType(defType) + .legacyType(fnm->ftp).legacyOptionalBehavior() .readWriteFlags(bRead, bWrite).required(!bOptional) .libraryFile(bLibrary).multiValue(bMultiple) .description(ftp2desc(fnm->ftp))); diff --git a/src/gromacs/commandline/tests/pargs.cpp b/src/gromacs/commandline/tests/pargs.cpp index c11259b77d..a7f4362732 100644 --- a/src/gromacs/commandline/tests/pargs.cpp +++ b/src/gromacs/commandline/tests/pargs.cpp @@ -378,6 +378,23 @@ TEST_F(ParseCommonArgsTest, ParsesFileArgsWithDefaultFileName) done_filenms(nfile(), fnm); } +TEST_F(ParseCommonArgsTest, ParseFileArgsWithCustomDefaultExtension) +{ + t_filenm fnm[] = { + { efTRX, "-o1", "conf1.gro", ffWRITE }, + { efTRX, "-o2", "conf2.pdb", ffWRITE }, + { efTRX, "-o3", "conf3.gro", ffWRITE } + }; + const char *const cmdline[] = { + "test", "-o2", "-o3", "test" + }; + parseFromArray(cmdline, PCA_CAN_SET_DEFFNM, fnm, gmx::EmptyArrayRef()); + EXPECT_STREQ("conf1.gro", opt2fn("-o1", nfile(), fnm)); + EXPECT_STREQ("conf2.pdb", opt2fn("-o2", nfile(), fnm)); + EXPECT_STREQ("test.gro", opt2fn("-o3", nfile(), fnm)); + done_filenms(nfile(), fnm); +} + /******************************************************************** * Tests for file name options (input files, dependent on file system contents) */ diff --git a/src/gromacs/options.h b/src/gromacs/options.h index 568ae932c4..4b11e1f7c0 100644 --- a/src/gromacs/options.h +++ b/src/gromacs/options.h @@ -146,6 +146,7 @@ #ifndef GMX_OPTIONS_H #define GMX_OPTIONS_H +#include "fileio/filenm.h" #include "options/basicoptions.h" #include "options/filenameoption.h" #include "options/filenameoptionmanager.h" diff --git a/src/gromacs/options/filenameoption.cpp b/src/gromacs/options/filenameoption.cpp index 4689f66d3c..171c668a5a 100644 --- a/src/gromacs/options/filenameoption.cpp +++ b/src/gromacs/options/filenameoption.cpp @@ -202,7 +202,7 @@ FileTypeHandler::isValidType(int fileType) const FileNameOptionStorage::FileNameOptionStorage(const FileNameOption &settings, FileNameOptionManager *manager) : MyBase(settings), info_(this), manager_(manager), fileType_(-1), - bRead_(settings.bRead_), bWrite_(settings.bWrite_), + defaultExtension_(""), bRead_(settings.bRead_), bWrite_(settings.bWrite_), bLibrary_(settings.bLibrary_) { GMX_RELEASE_ASSERT(!hasFlag(efOption_MultipleTimes), @@ -224,10 +224,31 @@ FileNameOptionStorage::FileNameOptionStorage(const FileNameOption &settings, } } } + FileTypeHandler typeHandler(fileType_); + if (settings.defaultType_ >= 0 && settings.defaultType_ < efNR) + { + // This also assures that the default type is not a generic type. + GMX_RELEASE_ASSERT(typeHandler.isValidType(settings.defaultType_), + "Default type for a file option is not an accepted " + "type for the option"); + FileTypeHandler defaultHandler(settings.defaultType_); + defaultExtension_ = defaultHandler.extension(0); + } + else if (typeHandler.extensionCount() > 0) + { + defaultExtension_ = typeHandler.extension(0); + } if (settings.defaultBasename_ != NULL) { std::string defaultValue(settings.defaultBasename_); - defaultValue.append(defaultExtension()); + int type = fn2ftp(settings.defaultBasename_); + GMX_RELEASE_ASSERT(type == efNR || type == settings.defaultType_, + "Default basename has an extension that does not " + "match the default type"); + if (type == efNR) + { + defaultValue.append(defaultExtension()); + } setDefaultValueIfSet(defaultValue); if (isRequired() || settings.bLegacyOptionalBehavior_) { @@ -388,12 +409,7 @@ bool FileNameOptionStorage::isTrajectoryOption() const const char *FileNameOptionStorage::defaultExtension() const { - FileTypeHandler typeHandler(fileType_); - if (typeHandler.extensionCount() == 0) - { - return ""; - } - return typeHandler.extension(0); + return defaultExtension_; } std::vector FileNameOptionStorage::extensions() const diff --git a/src/gromacs/options/filenameoption.h b/src/gromacs/options/filenameoption.h index 453f4cd5f1..479a35834a 100644 --- a/src/gromacs/options/filenameoption.h +++ b/src/gromacs/options/filenameoption.h @@ -74,7 +74,8 @@ class FileNameOption : public OptionTemplate //! Initializes an option with the given name. explicit FileNameOption(const char *name) : MyBase(name), optionType_(eftUnknown), legacyType_(-1), - defaultBasename_(NULL), bLegacyOptionalBehavior_(false), + defaultBasename_(NULL), defaultType_(-1), + bLegacyOptionalBehavior_(false), bRead_(false), bWrite_(false), bLibrary_(false) { } @@ -138,7 +139,8 @@ class FileNameOption : public OptionTemplate * * Use this method instead of defaultValue() or defaultValueIfSet() to * set a default value for a file name option. No extension needs to - * be provided; it is automatically added based on filetype(). + * be provided; it is automatically added based on filetype() or + * defaultType(). * The behavior is also adjusted based on required(): if the option is * required, the value given to defaultBasename() is treated as for * both defaultValue() and defaultValueIfSet(), otherwise it is treated @@ -147,10 +149,11 @@ class FileNameOption : public OptionTemplate * For input files that accept multiple extensions, the extension is * completed to the default extension on creation of the option or at * time of parsing an option without a value. - * The extension may change during Options::finish(), as this is the - * time when the default names are checked against the file system to - * provide an extension that matches an existing file if that is - * possible. + * + * If FileNameOptionManager is used, the extension may change during + * Options::finish(), as this is the time when the default names are + * checked against the file system to provide an extension that matches + * an existing file if that is possible. * * If FileNameOptionManager is used, and * FileNameOptionManager::addDefaultFileNameOption() is used, and the @@ -159,6 +162,20 @@ class FileNameOption : public OptionTemplate */ MyClass &defaultBasename(const char *basename) { defaultBasename_ = basename; return me(); } + /*! \brief + * Sets a default type/extension for the file option. + * + * For options that accept multiple types of files (e.g., + * eftTrajectory), this method sets the default extension used + * for completing defaultBasename(), as well as the default extension + * used by FileNameOptionManager to complete various file names. + * + * The value should be one of the enumerated `ef*` values from + * filenm.h, and be a valid type for the type specified with + * filetype(). + */ + MyClass &defaultType(int filetype) + { defaultType_ = filetype; return me(); } private: // Use defaultBasename() instead. @@ -172,6 +189,7 @@ class FileNameOption : public OptionTemplate OptionFileType optionType_; int legacyType_; const char *defaultBasename_; + int defaultType_; bool bLegacyOptionalBehavior_; bool bRead_; bool bWrite_; diff --git a/src/gromacs/options/filenameoptionstorage.h b/src/gromacs/options/filenameoptionstorage.h index 7d22180edb..84a0f8fc21 100644 --- a/src/gromacs/options/filenameoptionstorage.h +++ b/src/gromacs/options/filenameoptionstorage.h @@ -104,6 +104,7 @@ class FileNameOptionStorage : public OptionStorageTemplate FileNameOptionInfo info_; FileNameOptionManager *manager_; int fileType_; + const char *defaultExtension_; bool bRead_; bool bWrite_; bool bLibrary_; diff --git a/src/gromacs/options/tests/filenameoption.cpp b/src/gromacs/options/tests/filenameoption.cpp index cefed8dec3..6097d3232c 100644 --- a/src/gromacs/options/tests/filenameoption.cpp +++ b/src/gromacs/options/tests/filenameoption.cpp @@ -45,6 +45,7 @@ #include +#include "gromacs/fileio/filenm.h" #include "gromacs/options/options.h" #include "gromacs/options/optionsassigner.h" #include "gromacs/utility/exceptions.h" @@ -132,6 +133,46 @@ TEST(FileNameOptionTest, HandlesOptionalDefaultValueWithoutExtension) EXPECT_EQ("testfile.ndx", value); } +TEST(FileNameOptionTest, HandlesRequiredCustomDefaultExtension) +{ + gmx::Options options(NULL, NULL); + std::string value; + ASSERT_NO_THROW_GMX(options.addOption( + FileNameOption("f").store(&value).required() + .filetype(gmx::eftTrajectory).outputFile() + .defaultBasename("testfile") + .defaultType(efPDB))); + EXPECT_EQ("testfile.pdb", value); + + gmx::OptionsAssigner assigner(&options); + EXPECT_NO_THROW_GMX(assigner.start()); + EXPECT_NO_THROW_GMX(assigner.finish()); + EXPECT_NO_THROW_GMX(options.finish()); + + EXPECT_EQ("testfile.pdb", value); +} + +TEST(FileNameOptionTest, HandlesOptionalCustomDefaultExtension) +{ + gmx::Options options(NULL, NULL); + std::string value; + ASSERT_NO_THROW_GMX(options.addOption( + FileNameOption("f").store(&value) + .filetype(gmx::eftTrajectory).outputFile() + .defaultBasename("testfile") + .defaultType(efPDB))); + EXPECT_TRUE(value.empty()); + + gmx::OptionsAssigner assigner(&options); + EXPECT_NO_THROW_GMX(assigner.start()); + EXPECT_NO_THROW_GMX(assigner.startOption("f")); + EXPECT_NO_THROW_GMX(assigner.finishOption()); + EXPECT_NO_THROW_GMX(assigner.finish()); + EXPECT_NO_THROW_GMX(options.finish()); + + EXPECT_EQ("testfile.pdb", value); +} + TEST(FileNameOptionTest, GivesErrorOnUnknownFileSuffix) { gmx::Options options(NULL, NULL); diff --git a/src/gromacs/options/tests/filenameoptionmanager.cpp b/src/gromacs/options/tests/filenameoptionmanager.cpp index f0e2ecd5d8..8e2e523883 100644 --- a/src/gromacs/options/tests/filenameoptionmanager.cpp +++ b/src/gromacs/options/tests/filenameoptionmanager.cpp @@ -45,6 +45,7 @@ #include +#include "gromacs/fileio/filenm.h" #include "gromacs/options/filenameoption.h" #include "gromacs/options/options.h" #include "gromacs/options/optionsassigner.h" @@ -103,6 +104,25 @@ TEST_F(FileNameOptionManagerTest, AddsMissingExtension) EXPECT_EQ("testfile.xtc", value); } +TEST_F(FileNameOptionManagerTest, AddsMissingCustomDefaultExtension) +{ + std::string value; + ASSERT_NO_THROW_GMX(options_.addOption( + FileNameOption("f").store(&value) + .filetype(gmx::eftTrajectory).outputFile() + .defaultType(efPDB))); + + gmx::OptionsAssigner assigner(&options_); + EXPECT_NO_THROW_GMX(assigner.start()); + EXPECT_NO_THROW_GMX(assigner.startOption("f")); + EXPECT_NO_THROW_GMX(assigner.appendValue("testfile")); + EXPECT_NO_THROW_GMX(assigner.finishOption()); + EXPECT_NO_THROW_GMX(assigner.finish()); + EXPECT_NO_THROW_GMX(options_.finish()); + + EXPECT_EQ("testfile.pdb", value); +} + TEST_F(FileNameOptionManagerTest, GivesErrorOnMissingInputFile) { std::string value;