From e54b9c33c7c2f01cd5d64d03a09ad3c81f5b3001 Mon Sep 17 00:00:00 2001
From: Teemu Murtola <teemu.murtola@gmail.com>
Date: Wed, 5 Feb 2014 21:59:17 +0200
Subject: [PATCH] Make FileNameOption behave more like old filenm parser

If a required file name option is provided without a value, then it is a
no-op.  Previously, FileNameOption raised an exception for such usage.
Add a test to cover this case.

Related to #642.

Change-Id: I89d6cc8ee5d5bf2915bb6841317b402ade00b99b
---
 ...eHelpWriterTest_HandlesLongFileOptions.xml |  6 +++---
 ...dLineHelpWriterTest_HandlesOptionTypes.xml |  4 ++--
 src/gromacs/options/filenameoption.cpp        | 11 ++++------
 src/gromacs/options/filenameoption.h          |  5 +++--
 src/gromacs/options/tests/filenameoption.cpp  | 21 ++++++++++++++++++-
 5 files changed, 32 insertions(+), 15 deletions(-)

diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml
index b01af043e3..44497d3a08 100644
--- a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml
+++ b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml
@@ -4,14 +4,14 @@
   <String Name="HelpText"><![CDATA[
 SYNOPSIS
 
-gmx [-f <.xtc/.trr/...>] [-f2 <.xtc/.trr/...>] [-lib [<.xtc/.trr/...>]]
+gmx [-f [<.xtc/.trr/...>]] [-f2 [<.xtc/.trr/...>]] [-lib [<.xtc/.trr/...>]]
     [-longfileopt [<.dat>]] [-longfileopt2 [<.dat>]]
 
 FILE OPTIONS
 
- -f     <.xtc/.trr/...> (path/to/long/trajectory/name.xtc) (Input)
+ -f     [<.xtc/.trr/...>] (path/to/long/trajectory/name.xtc) (Input)
      File name option with a long value: xtc trr cpt trj gro g96 pdb g87 tng
- -f2    <.xtc/.trr/...> (path/to/long/trajectory.xtc) (Input)
+ -f2    [<.xtc/.trr/...>] (path/to/long/trajectory.xtc) (Input)
      File name option with a long value: xtc trr cpt trj gro g96 pdb g87 tng
  -lib   [<.xtc/.trr/...>] (path/to/long/trajectory/name.xtc) (Input, Opt., Lib.)
      File name option with a long value and type: xtc trr cpt trj gro g96 pdb
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml
index e81c9f60ab..f24fd8342c 100644
--- a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml
+++ b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml
@@ -4,14 +4,14 @@
   <String Name="HelpText"><![CDATA[
 SYNOPSIS
 
-gmx [-f <.xtc/.trr/...>] [-mult [<.xtc/.trr/...> [...]]] [-lib [<.dat>]]
+gmx [-f [<.xtc/.trr/...>]] [-mult [<.xtc/.trr/...> [...]]] [-lib [<.dat>]]
     [-io [<.dat>]] [-o <.xvg>] [-[no]bool] [-[no]hidden] [-int <int>]
     [-ivec <vector>] [-double <real>] [-dvec <vector>] [-time <time>]
     [-string <string>] [-enum <enum>]
 
 FILE OPTIONS
 
- -f     <.xtc/.trr/...> (traj.xtc) (Input)
+ -f     [<.xtc/.trr/...>] (traj.xtc) (Input)
      Input file description: xtc trr cpt trj gro g96 pdb g87 tng
  -mult  [<.xtc/.trr/...> [...]] (traj.xtc) (Input, Opt.)
      Multiple file description: xtc trr cpt trj gro g96 pdb g87 tng
diff --git a/src/gromacs/options/filenameoption.cpp b/src/gromacs/options/filenameoption.cpp
index ae40a3a6cf..31dff32dc4 100644
--- a/src/gromacs/options/filenameoption.cpp
+++ b/src/gromacs/options/filenameoption.cpp
@@ -273,15 +273,12 @@ FileNameOptionStorage::FileNameOptionStorage(const FileNameOption &settings)
 {
     if (settings.defaultBasename_ != NULL)
     {
+        std::string defaultValue =
+            completeFileName(settings.defaultBasename_, filetype_, false);
+        setDefaultValueIfSet(defaultValue);
         if (isRequired())
         {
-            setDefaultValue(completeFileName(settings.defaultBasename_,
-                                             filetype_, false));
-        }
-        else
-        {
-            setDefaultValueIfSet(completeFileName(settings.defaultBasename_,
-                                                  filetype_, false));
+            setDefaultValue(defaultValue);
         }
     }
 }
diff --git a/src/gromacs/options/filenameoption.h b/src/gromacs/options/filenameoption.h
index 489f56d887..0b8bc093c5 100644
--- a/src/gromacs/options/filenameoption.h
+++ b/src/gromacs/options/filenameoption.h
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2012, by the GROMACS development team, led by
+ * Copyright (c) 2012,2014, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -113,7 +113,8 @@ class FileNameOption : public OptionTemplate<std::string, FileNameOption>
          * be provided; it is automatically added based on filetype().
          * The behavior is also adjusted based on required(): if the option is
          * required, the value given to defaultBasename() is treated as for
-         * defaultValue(), otherwise it is treated as for defaultValueIfSet().
+         * both defaultValue() and defaultValueIfSet(), otherwise it is treated
+         * as for defaultValueIfSet().
          */
         MyClass &defaultBasename(const char *basename)
         { defaultBasename_ = basename; return me(); }
diff --git a/src/gromacs/options/tests/filenameoption.cpp b/src/gromacs/options/tests/filenameoption.cpp
index ee3770d509..76956c6950 100644
--- a/src/gromacs/options/tests/filenameoption.cpp
+++ b/src/gromacs/options/tests/filenameoption.cpp
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2012,2013, by the GROMACS development team, led by
+ * Copyright (c) 2012,2013,2014, by the GROMACS development team, led by
  * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl,
  * and including many others, as listed in the AUTHORS file in the
  * top-level source directory and at http://www.gromacs.org.
@@ -95,6 +95,25 @@ TEST(FileNameOptionTest, HandlesRequiredDefaultValueWithoutExtension)
     EXPECT_EQ("testfile.dat", value);
 }
 
+TEST(FileNameOptionTest, HandlesRequiredOptionWithoutValue)
+{
+    gmx::Options           options(NULL, NULL);
+    std::string            value;
+    ASSERT_NO_THROW_GMX(options.addOption(
+                                FileNameOption("f").store(&value).required()
+                                    .filetype(gmx::eftGenericData).outputFile()
+                                    .defaultBasename("testfile")));
+
+    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.dat", value);
+}
+
 TEST(FileNameOptionTest, HandlesOptionalDefaultValueWithoutExtension)
 {
     gmx::Options           options(NULL, NULL);
-- 
2.22.0