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 d0184d1a8302927112416ac263f93a298110eb2a..5a16f3cf48ba90b203322c83f95f3f9e4dd419a5 100644 (file)
--- 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 c11259b77d3e2c4fec3f3c8ded75dec6b8836fd6..a7f4362732f3bf758af4569d8e6a4f4e93cc865f 100644 (file)
--- 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 568ae932c486862529e17edf984270ab25817489..4b11e1f7c06f3e829a6ecc73adf5c3d2aba27d28 100644 (file)
--- 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 4689f66d3cd4aefe1dcb2b7f2fd32e51e40f11b6..171c668a5afbf964de4f5056bb1f39f40408ef99 100644 (file)
--- 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<const char *> FileNameOptionStorage::extensions() const

diff --git a/src/gromacs/options/filenameoption.h b/src/gromacs/options/filenameoption.h
index 453f4cd5f1dafcb7b190d6206bd13cb96fd27aea..479a35834acb5e878f86fd8d53de7d8a4f075d92 100644 (file)
--- a/src/gromacs/options/filenameoption.h
+++ b/src/gromacs/options/filenameoption.h
@@ -74,7 +74,8 @@ class FileNameOption : public OptionTemplate<std::string, FileNameOption>
         //! 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<std::string, FileNameOption>
          *
          * 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<std::string, FileNameOption>
          * 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<std::string, FileNameOption>
          */
         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<std::string, FileNameOption>
         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 7d22180edba2e80fcbd91f2fa3289f87c1da9432..84a0f8fc21bca37caf331aab38e6d283b33ba722 100644 (file)
--- a/src/gromacs/options/filenameoptionstorage.h
+++ b/src/gromacs/options/filenameoptionstorage.h
@@ -104,6 +104,7 @@ class FileNameOptionStorage : public OptionStorageTemplate<std::string>
         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 cefed8dec389d162315da8a177fcb76a2ee63580..6097d3232c987f2aab9c427e95aeab88d6b2d88e 100644 (file)
--- a/src/gromacs/options/tests/filenameoption.cpp
+++ b/src/gromacs/options/tests/filenameoption.cpp
@@ -45,6 +45,7 @@
 
 #include <gtest/gtest.h>
 
+#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 f0e2ecd5d83a5a9ab83238cc2ea1a29b4c14d456..8e2e5238834b00239cf5b0583b421e9370bb7558 100644 (file)
--- a/src/gromacs/options/tests/filenameoptionmanager.cpp
+++ b/src/gromacs/options/tests/filenameoptionmanager.cpp
@@ -45,6 +45,7 @@
 
 #include <gtest/gtest.h>
 
+#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;