Modernize read_inpfile
authorMark Abraham <mark.j.abraham@gmail.com>
Wed, 21 Dec 2016 05:19:22 +0000 (16:19 +1100)
committerMark Abraham <mark.j.abraham@gmail.com>
Mon, 13 Mar 2017 12:40:44 +0000 (13:40 +0100)
Used std::string and gmx::TextReader to simplify this. It is likely no
longer as efficient because it now makes several std::string objects
per line, but this is not a significant part of the execution time of
e.g. grompp.

Move COMMENTSIGN to cstringutil.cpp, now that this is an
implementation detail of string-handling code, rather than also used
in readinp.cpp.

Moved responsibility for stripping of comments to TextReader, reworked
its whitespace-trimming behaviour, and introduced some tests for that
class.

Introduced some TODO items to reconsider multiple behaviours where
read_inpfile ignores what could be malformed input. It is used for
grompp, xpm2ps and membed, however, so it is not immediately clear
what fixes might be appropriate, and we might anyway remove this
code shortly.

Introduced catches for exceptions that might be thrown while calling
read_inpfile (and related code that might change soon).

Added tests for newly introduced split-and-trim functionality that
supports read_inpfile.

Refs #2074

Change-Id: Id9c46d60a3ec7ecdcdb9529bba2fdb68ce241914

17 files changed:
src/gromacs/commandline/cmdlinehelpmodule.cpp
src/gromacs/fileio/readinp.cpp
src/gromacs/fileio/readinp.h
src/gromacs/gmxana/gmx_xpm2ps.cpp
src/gromacs/gmxpreprocess/grompp.cpp
src/gromacs/gmxpreprocess/readir.cpp
src/gromacs/utility/cstringutil.cpp
src/gromacs/utility/cstringutil.h
src/gromacs/utility/stringutil.cpp
src/gromacs/utility/stringutil.h
src/gromacs/utility/tests/CMakeLists.txt
src/gromacs/utility/tests/stringutil.cpp
src/gromacs/utility/tests/textreader.cpp [new file with mode: 0644]
src/gromacs/utility/textreader.cpp
src/gromacs/utility/textreader.h
src/programs/mdrun/membed.cpp
src/testutils/stringtest.h

index 3341491f4bf8e4e6db6adeec11f5c95563eb7ff6..3157da330549b06e9e08833fe6d1f8b1286d1747 100644 (file)
@@ -553,7 +553,8 @@ HelpExportReStructuredText::HelpExportReStructuredText(
 {
     TextReader   linksFile("links.dat");
     std::string  line;
-    while (linksFile.readLineTrimmed(&line))
+    linksFile.setTrimTrailingWhiteSpace(true);
+    while (linksFile.readLine(&line))
     {
         links_.addLink("[REF]." + line + "[ref]",
                        formatString(":ref:`.%s <%s>`", line.c_str(), line.c_str()),
index e4e8d7d3e63ff2d7c2c2405048969b97428415c4..aeaa00c2b6311c2dacdb76d1c4b8513839ccefe3 100644 (file)
 #include "gromacs/utility/cstringutil.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
-#include "gromacs/utility/futil.h"
 #include "gromacs/utility/keyvaluetreebuilder.h"
 #include "gromacs/utility/programcontext.h"
 #include "gromacs/utility/qsort_threadsafe.h"
 #include "gromacs/utility/smalloc.h"
+#include "gromacs/utility/stringutil.h"
+#include "gromacs/utility/textreader.h"
+#include "gromacs/utility/textstream.h"
 
-t_inpfile *read_inpfile(const char *fn, int *ninp,
+t_inpfile *read_inpfile(gmx::TextInputStream *stream, const char *fn, int *ninp,
                         warninp_t wi)
 {
-    FILE      *in;
-    char       buf[STRLEN], lbuf[STRLEN], rbuf[STRLEN], warn_buf[STRLEN];
-    char      *ptr, *cptr;
     t_inpfile *inp = nullptr;
-    int        nin, lc, i, j, k;
-
 
     if (debug)
     {
         fprintf(debug, "Reading MDP file %s\n", fn);
     }
 
-    in = gmx_ffopen(fn, "r");
-
-    nin = lc  = 0;
-    do
+    int             indexOfLineReadFromFile = 0;
+    int             countOfUniqueKeysFound  = 0;
+    std::string     line;
+    gmx::TextReader reader(stream);
+    reader.setTrimTrailingWhiteSpace(true);
+    reader.setTrimTrailingComment(true, ';');
+    while (reader.readLine(&line))
     {
-        ptr = fgets2(buf, STRLEN-1, in);
-        lc++;
-        set_warning_line(wi, fn, lc);
-        if (ptr)
+        indexOfLineReadFromFile++;
+        set_warning_line(wi, fn, indexOfLineReadFromFile);
+
+        if (line.empty())
         {
-            // TODO This parsing should be using strip_comment, trim,
-            // strchr, etc. rather than re-inventing wheels.
+            continue;
+        }
 
-            /* Strip comment */
-            if ((cptr = std::strchr(buf, COMMENTSIGN)) != nullptr)
+        auto tokens = gmx::splitAndTrimDelimitedString(line, '=');
+        if (tokens.size() < 2)
+        {
+            // TODO this seems like it silently ignores the user accidentally deleting an equals sign...
+            if (debug)
             {
-                *cptr = '\0';
+                fprintf(debug, "No = on line %d in file %s, ignored\n", indexOfLineReadFromFile, fn);
             }
-            /* Strip spaces */
-            trim(buf);
-
-            for (j = 0; (buf[j] != '=') && (buf[j] != '\0'); j++)
+            continue;
+        }
+        if (tokens.size() > 2)
+        {
+            // TODO ignoring such lines does not seem like good behaviour
+            if (debug)
             {
-                ;
+                fprintf(debug, "Multiple equals signs on line %d in file %s, ignored\n", indexOfLineReadFromFile, fn);
             }
-            if (buf[j] == '\0')
+            continue;
+        }
+        if (tokens[0].empty())
+        {
+            // TODO ignoring such lines does not seem like good behaviour
+            if (debug)
             {
-                if (j > 0)
-                {
-                    if (debug)
-                    {
-                        fprintf(debug, "No = on line %d in file %s, ignored\n", lc, fn);
-                    }
-                }
+                fprintf(debug, "Empty left hand side on line %d in file %s, ignored\n", indexOfLineReadFromFile, fn);
             }
-            else
+            continue;
+        }
+        if (tokens[1].empty())
+        {
+            // TODO ignoring such lines does not seem like good behaviour
+            if (debug)
             {
-                for (i = 0; (i < j); i++)
-                {
-                    lbuf[i] = buf[i];
-                }
-                lbuf[i] = '\0';
-                trim(lbuf);
-                if (lbuf[0] == '\0')
-                {
-                    if (debug)
-                    {
-                        fprintf(debug, "Empty left hand side on line %d in file %s, ignored\n", lc, fn);
-                    }
-                }
-                else
-                {
-                    for (i = j+1, k = 0; (buf[i] != '\0'); i++, k++)
-                    {
-                        rbuf[k] = buf[i];
-                    }
-                    rbuf[k] = '\0';
-                    trim(rbuf);
-                    if (rbuf[0] == '\0')
-                    {
-                        if (debug)
-                        {
-                            fprintf(debug, "Empty right hand side on line %d in file %s, ignored\n", lc, fn);
-                        }
-                    }
-                    else
-                    {
-                        /* Now finally something sensible; check for duplicates */
-                        int found_index = search_einp(nin, inp, lbuf);
-
-                        if (found_index == -1)
-                        {
-                            /* add a new item */
-                            srenew(inp, ++nin);
-                            inp[nin-1].inp_count  = 1;
-                            inp[nin-1].count      = 0;
-                            inp[nin-1].bObsolete  = FALSE;
-                            inp[nin-1].bSet       = FALSE;
-                            inp[nin-1].name       = gmx_strdup(lbuf);
-                            inp[nin-1].value      = gmx_strdup(rbuf);
-                        }
-                        else
-                        {
-                            sprintf(warn_buf,
-                                    "Parameter \"%s\" doubly defined\n",
-                                    lbuf);
-                            warning_error(wi, warn_buf);
-                        }
-                    }
-                }
+                fprintf(debug, "Empty right hand side on line %d in file %s, ignored\n", indexOfLineReadFromFile, fn);
             }
+            continue;
         }
-    }
-    while (ptr);
 
-    gmx_ffclose(in);
+        /* Now finally something sensible; check for duplicates */
+        int found_index = search_einp(countOfUniqueKeysFound, inp, tokens[0].c_str());
+
+        if (found_index == -1)
+        {
+            /* add a new item */
+            srenew(inp, ++countOfUniqueKeysFound);
+            inp[countOfUniqueKeysFound-1].inp_count  = 1;
+            inp[countOfUniqueKeysFound-1].count      = 0;
+            inp[countOfUniqueKeysFound-1].bObsolete  = FALSE;
+            inp[countOfUniqueKeysFound-1].bSet       = FALSE;
+            inp[countOfUniqueKeysFound-1].name       = gmx_strdup(tokens[0].c_str());
+            inp[countOfUniqueKeysFound-1].value      = gmx_strdup(tokens[1].c_str());
+        }
+        else
+        {
+            auto message = gmx::formatString("Parameter \"%s\" doubly defined\n",
+                                             tokens[0].c_str());
+            warning_error(wi, message.c_str());
+        }
+    }
+    /* This preserves the behaviour of the old code, which issues some
+       warnings after completing parsing. Regenerating regressiontest
+       warning files is not worth the effort. */
+    indexOfLineReadFromFile++;
+    set_warning_line(wi, fn, indexOfLineReadFromFile);
 
     if (debug)
     {
         fprintf(debug, "Done reading MDP file, there were %d entries in there\n",
-                nin);
+                countOfUniqueKeysFound);
     }
 
-    *ninp = nin;
+    *ninp = countOfUniqueKeysFound;
 
     return inp;
 }
index 311c27f4e958311e3742e16ccdbf7cb7beeae758..ca020eaf52b5365aa39207569ed601cd03857130 100644 (file)
@@ -47,6 +47,7 @@ typedef warninp *warninp_t;
 namespace gmx
 {
 class KeyValueTreeObject;
+class TextInputStream;
 }
 
 typedef struct t_inpfile {
@@ -62,12 +63,17 @@ typedef struct t_inpfile {
    Initally read in with read_inpfile, then filled in with missing values
    through get_eint, get_ereal, etc. */
 
-t_inpfile *read_inpfile(const char *fn, int *ninp,
+/*! \brief Create and return an array of \c ninp t_inpfile structs
+ * from "key = value" lines in \c stream corresponding to file \c fn.
+ *
+ * \param[in]  stream          Text stream to read.
+ * \param[in]  fn              Filename corresponding to \c reader.
+ * \param[out] ninp            Length of returned array.
+ * \param[out] wi              Handler for context-sensitive warnings.
+ * \throws     std::bad_alloc  If out of memory.
+ * \throws     Anything the stream underlying \c reader can throw. */
+t_inpfile *read_inpfile(gmx::TextInputStream *stream, const char *fn, int *ninp,
                         warninp_t wi);
-/* Create & populate a t_inpfile struct from values in file fn.
-   fn = the file name
-   ninp = the number of read parameters
-   cppopts = the cpp-style options for #include paths and #defines */
 
 gmx::KeyValueTreeObject flatKeyValueTreeFromInpFile(int ninp, t_inpfile inp[]);
 
index 80fa58f8ba309c568f9493abc2b7c8d41fb675bc..b9b7b08b7211e01fe8b1141675cbbd846701e6bf 100644 (file)
@@ -54,7 +54,9 @@
 #include "gromacs/gmxana/gmx_ana.h"
 #include "gromacs/utility/arraysize.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
+#include "gromacs/utility/filestream.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/smalloc.h"
@@ -124,7 +126,8 @@ void get_params(const char *mpin, const char *mpout, t_psrec *psr)
 
     if (mpin != nullptr)
     {
-        inp = read_inpfile(mpin, &ninp, wi);
+        gmx::TextInputFile stream(mpin);
+        inp = read_inpfile(&stream, mpin, &ninp, wi);
     }
     else
     {
@@ -799,7 +802,11 @@ void ps_mat(const char *outf, int nmat, t_matrix mat[], t_matrix mat2[],
 
     /* memory leak: */
     libm2p = m2p ? gmxlibfn(m2p) : m2p;
-    get_params(libm2p, m2pout, &psrec);
+    try
+    {
+        get_params(libm2p, m2pout, &psrec);
+    }
+    GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR;
 
     psr = &psrec;
 
index e9c4e0b9c686d572c852c1b99cf708f3ebcaf1a7..e8724a7482afd5b53ad25b21baa18f5aec0246bc 100644 (file)
@@ -93,6 +93,7 @@
 #include "gromacs/trajectory/trajectoryframe.h"
 #include "gromacs/utility/arraysize.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/gmxassert.h"
@@ -1833,6 +1834,8 @@ int gmx_grompp(int argc, char *argv[])
 
     /* Initiate some variables */
     gmx::MDModules mdModules;
+    t_inputrec     irInstance;
+    t_inputrec    *ir = &irInstance;
     snew(opts, 1);
     snew(opts->include, STRLEN);
     snew(opts->define, STRLEN);
@@ -1842,9 +1845,11 @@ int gmx_grompp(int argc, char *argv[])
     /* PARAMETER file processing */
     mdparin = opt2fn("-f", NFILE, fnm);
     set_warning_line(wi, mdparin, -1);
-    t_inputrec  irInstance;
-    t_inputrec *ir = &irInstance;
-    get_ir(mdparin, opt2fn("-po", NFILE, fnm), &mdModules, ir, opts, WriteMdpHeader::yes, wi);
+    try
+    {
+        get_ir(mdparin, opt2fn("-po", NFILE, fnm), &mdModules, ir, opts, WriteMdpHeader::yes, wi);
+    }
+    GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR;
 
     if (bVerbose)
     {
index 4e69c48d9ff53dd521f9315b69c6603a2ae3e9e7..a2f40e27b83e2490fa9b54ca2abb2fd3b65dd626 100644 (file)
@@ -71,6 +71,7 @@
 #include "gromacs/utility/cstringutil.h"
 #include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
+#include "gromacs/utility/filestream.h"
 #include "gromacs/utility/gmxassert.h"
 #include "gromacs/utility/ikeyvaluetreeerror.h"
 #include "gromacs/utility/keyvaluetree.h"
@@ -1821,7 +1822,8 @@ void get_ir(const char *mdparin, const char *mdparout,
     t_expanded *expand = ir->expandedvals;
 
     init_inputrec_strings();
-    inp = read_inpfile(mdparin, &ninp, wi);
+    gmx::TextInputFile stream(mdparin);
+    inp = read_inpfile(&stream, mdparin, &ninp, wi);
 
     snew(dumstr[0], STRLEN);
     snew(dumstr[1], STRLEN);
index d28c4820a09483c8e8083c90248c58f43b6e6acf..cdd186b1038b72660abd26e2b5602e5b84ac3868 100644 (file)
@@ -56,6 +56,9 @@
 #include "gromacs/utility/smalloc.h"
 #include "gromacs/utility/sysinfo.h"
 
+//! Comment sign to use.
+#define COMMENTSIGN ';'
+
 int continuing(char *s)
 {
     int sl;
index 44f6012500776de136e848c46924bce35a2471a6..3223951b58f1345630803ea04a3c8459cd220cb0 100644 (file)
@@ -3,7 +3,7 @@
  *
  * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
  * Copyright (c) 2001-2004, The GROMACS development team.
- * Copyright (c) 2012,2013,2014,2015, by the GROMACS development team, led by
+ * Copyright (c) 2012,2013,2014,2015,2017, 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.
@@ -56,8 +56,6 @@ extern "C" {
 
 /** Continuation character. */
 #define CONTINUE    '\\'
-/** Comment sign to use. */
-#define COMMENTSIGN ';'
 /** Standard size for char* string buffers. */
 #define STRLEN 4096
 
@@ -77,7 +75,7 @@ int continuing(char *s);
  */
 char *fgets2(char *s, int n, FILE *stream);
 
-/** Remove portion of a line after a ::COMMENTSIGN.  */
+/** Remove portion of a line after a ';' comment sign.  */
 void strip_comment(char *line);
 
 /** Make a string uppercase. */
index 663982f7de6e6cc77ec1929be1848841502559ac..74a9753ec27c29912c40335e3b1b184bfd3b1ea3 100644 (file)
@@ -216,6 +216,15 @@ std::vector<std::string> splitDelimitedString(const std::string &str, char delim
     return result;
 }
 
+std::vector<std::string> splitAndTrimDelimitedString(const std::string &str, char delim)
+{
+    std::vector<std::string> result;
+
+    result = splitDelimitedString(str, delim);
+    std::transform(result.begin(), result.end(), result.begin(), stripString);
+    return result;
+}
+
 namespace
 {
 
index 56beb1cc924b943ff9df132dd132b294fb1d2590..b8961bb383c95fd6cdaaedf5cd32fe1ad5baeb99 100644 (file)
@@ -168,7 +168,6 @@ std::string stripSuffixIfPresent(const std::string &str, const char *suffix);
  * \throws    std::bad_alloc if out of memory.
  */
 std::string stripString(const std::string &str);
-
 /*! \brief
  * Formats a string (snprintf() wrapper).
  *
@@ -351,11 +350,27 @@ std::vector<std::string> splitString(const std::string &str);
  * \returns   \p str split into tokens at delimiter.
  * \throws    std::bad_alloc if out of memory.
  *
- * Unlike splitString(), consencutive delimiters will generate empty tokens, as
+ * Unlike splitString(), consecutive delimiters will generate empty tokens, as
  * will leading or trailing delimiters.
  * Empty input will return an empty vector.
  */
 std::vector<std::string> splitDelimitedString(const std::string &str, char delim);
+/*! \brief
+ * Splits \c str to tokens separated by delimiter \c delim. Removes
+ * leading and trailing whitespace from those strings with std::isspace.
+ *
+ * \param[in] str   String to process.
+ * \param[in] delim Delimiter to use for splitting.
+ * \returns   \p str split into tokens at delimiter, with whitespace stripped.
+ * \throws    std::bad_alloc if out of memory.
+ *
+ * Unlike splitString(), consecutive delimiters will generate empty tokens, as
+ * will leading or trailing delimiters.
+ * Empty input will return an empty vector.
+ * Input with only whitespace will return a vector of size 1,
+ * that contains an empty token.
+ */
+std::vector<std::string> splitAndTrimDelimitedString(const std::string &str, char delim);
 
 /*! \brief
  * Replace all occurrences of a string with another string.
index 7c24db6b1064b94a7decc200589bb2f54a8f0f6f..5fa46959d7a97c242c5156d29dac4df26c206bf7 100644 (file)
@@ -43,5 +43,6 @@ gmx_add_unit_test(UtilityUnitTests utility-test
                   mutex.cpp
                   path.cpp
                   stringutil.cpp
+                  textreader.cpp
                   textwriter.cpp
                   )
index bf974093d69fbd8520f1ea1397b5f75ab2c05c31..cc8fd7a25819b39d78853e36483b354cd56bc87e 100644 (file)
 #include "testutils/refdata.h"
 #include "testutils/stringtest.h"
 
+namespace gmx
+{
+namespace test
+{
 namespace
 {
 
@@ -138,6 +142,26 @@ TEST(StringUtilityTest, SplitDelimitedString)
     EXPECT_THAT(gmx::splitDelimitedString("", ';'), IsEmpty());
 }
 
+TEST(StringUtilityTest, SplitAndTrimDelimitedString)
+{
+    using ::testing::ElementsAre;
+    using ::testing::IsEmpty;
+    EXPECT_THAT(splitAndTrimDelimitedString("", ';'), IsEmpty());
+    EXPECT_THAT(splitAndTrimDelimitedString(" \t\n ", ';'), ElementsAre(""));
+    EXPECT_THAT(splitAndTrimDelimitedString("foo", ';'), ElementsAre("foo"));
+    EXPECT_THAT(splitAndTrimDelimitedString(" foo ", ';'), ElementsAre("foo"));
+    EXPECT_THAT(splitAndTrimDelimitedString("foo;bar", ';'), ElementsAre("foo", "bar"));
+    EXPECT_THAT(splitAndTrimDelimitedString(";foo;bar", ';'), ElementsAre("", "foo", "bar"));
+    EXPECT_THAT(splitAndTrimDelimitedString("foo;bar;", ';'), ElementsAre("foo", "bar", ""));
+    EXPECT_THAT(splitAndTrimDelimitedString(";foo;bar;", ';'), ElementsAre("", "foo", "bar", ""));
+    EXPECT_THAT(splitAndTrimDelimitedString("foo;;bar", ';'), ElementsAre("foo", "", "bar"));
+    EXPECT_THAT(splitAndTrimDelimitedString("foo  ;  bar ", ';'), ElementsAre("foo", "bar"));
+    EXPECT_THAT(splitAndTrimDelimitedString("  ; foo ;  bar ", ';'), ElementsAre("", "foo", "bar"));
+    EXPECT_THAT(splitAndTrimDelimitedString(" foo  ;  bar ; ", ';'), ElementsAre("foo", "bar", ""));
+    EXPECT_THAT(splitAndTrimDelimitedString(" ;  foo\n ;  bar ;  ", ';'), ElementsAre("", "foo", "bar", ""));
+    EXPECT_THAT(splitAndTrimDelimitedString(" foo  ; ; \tbar", ';'), ElementsAre("foo", "", "bar"));
+}
+
 /********************************************************************
  * Tests for formatString()
  */
@@ -407,3 +431,5 @@ TEST_F(TextLineWrapperTest, WrapsCorrectlyWithExtraWhitespace)
 }
 
 } // namespace
+} // namespace
+} // namespace
diff --git a/src/gromacs/utility/tests/textreader.cpp b/src/gromacs/utility/tests/textreader.cpp
new file mode 100644 (file)
index 0000000..aa1b3e8
--- /dev/null
@@ -0,0 +1,280 @@
+/*
+ * This file is part of the GROMACS molecular simulation package.
+ *
+ * Copyright (c) 2017, 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.
+ *
+ * GROMACS is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * GROMACS is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with GROMACS; if not, see
+ * http://www.gnu.org/licenses, or write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA.
+ *
+ * If you want to redistribute modifications to GROMACS, please
+ * consider that scientific software is very special. Version
+ * control is crucial - bugs must be traceable. We will be happy to
+ * consider code for inclusion in the official distribution, but
+ * derived work must not be called official GROMACS. Details are found
+ * in the README & COPYING files - if they are missing, get the
+ * official version at http://www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the research papers on the package. Check out http://www.gromacs.org.
+ */
+/*! \internal \file
+ * \brief
+ * Tests for gmx::TextReader.
+ *
+ * \author Mark Abraham <mark.j.abraham@gmail.com>
+ * \ingroup module_utility
+ */
+#include "gmxpre.h"
+
+#include "gromacs/utility/textreader.h"
+
+#include <functional>
+#include <string>
+#include <vector>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "gromacs/utility/basedefinitions.h"
+#include "gromacs/utility/stringstream.h"
+#include "gromacs/utility/stringutil.h"
+
+#include "testutils/refdata.h"
+#include "testutils/stringtest.h"
+
+namespace gmx
+{
+namespace test
+{
+namespace
+{
+
+//! Convenience name.
+using Container = std::vector<std::string>;
+//! Convenience type for callbacks.
+using TestCallbackFunc = void(*)(TextReader &);
+
+//! Helper struct.
+struct TextReaderTestParams
+{
+    //! Input data.
+    const Container  input;
+    //! Callback to configure the reader with the behaviour being tested.
+    TestCallbackFunc callback;
+    //! Output to expect from the configured reader acting on the \c input.
+    const Container  expectedOutput;
+};
+
+//! Test fixture.
+class TextReaderTest : public ::testing::TestWithParam<TextReaderTestParams>
+{
+};
+
+TEST_P(TextReaderTest, UsingDifferentConfigurations)
+{
+    const auto &params = GetParam();
+
+    // Prepare the reader with the input lines.
+    StringInputStream stream(params.input);
+    TextReader        reader(&stream);
+    // Configure the intended reading behaviour.
+    params.callback(reader);
+    // Read the input and store what is read.
+    Container   readLines;
+    std::string line;
+    while (reader.readLine(&line))
+    {
+        readLines.push_back(line);
+    }
+    // Check the results
+    EXPECT_THAT(readLines, ::testing::ElementsAreArray(params.expectedOutput));
+}
+
+//! Test input data. Some configurations will remove comments delimited by '#'.
+const Container g_inputs =
+{
+    "",
+    " \t ",
+    "expected text",
+    " expected text ",
+    "expected text \t",
+    " \t expected text",
+    " \t expected text \t",
+    "expected text#",
+    "expected text\t #",
+    "expected text# ",
+    "expected text   # not expected ",
+    "#",
+    "\t #",
+    "   # not expected ",
+};
+
+/*! \brief A case of expected output data that is produced from two
+ * different configurations.
+ *
+ * Note that the implementation of StringInputStream joins the input
+ * container with "\n", so the inputs are always changed before being
+ * read. The name of this variable reflects that TextReader does not
+ * change them during reading. */
+const Container g_unchangedOutputs =
+{
+    "\n",
+    " \t \n",
+    "expected text\n",
+    " expected text \n",
+    "expected text \t\n",
+    " \t expected text\n",
+    " \t expected text \t\n",
+    "expected text#\n",
+    "expected text\t #\n",
+    "expected text# \n",
+    "expected text   # not expected \n",
+    "#\n",
+    "\t #\n",
+    "   # not expected \n",
+};
+INSTANTIATE_TEST_CASE_P(ParsesLinesDifferently, TextReaderTest,
+                            ::testing::Values(TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      GMX_UNUSED_VALUE(r);
+                                                  },
+                                                  g_unchangedOutputs
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimLeadingWhiteSpace(true);
+                                                  },
+                                                  { "",
+                                                    "",
+                                                    "expected text\n",
+                                                    "expected text \n",
+                                                    "expected text \t\n",
+                                                    "expected text\n",
+                                                    "expected text \t\n",
+                                                    "expected text#\n",
+                                                    "expected text\t #\n",
+                                                    "expected text# \n",
+                                                    "expected text   # not expected \n",
+                                                    "#\n",
+                                                    "#\n",
+                                                    "# not expected \n", }
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimTrailingWhiteSpace(true);
+                                                  },
+                                                  { "",
+                                                    "",
+                                                    "expected text",
+                                                    " expected text",
+                                                    "expected text",
+                                                    " \t expected text",
+                                                    " \t expected text",
+                                                    "expected text#",
+                                                    "expected text\t #",
+                                                    "expected text#",
+                                                    "expected text   # not expected",
+                                                    "#",
+                                                    "\t #",
+                                                    "   # not expected", }
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimTrailingWhiteSpace(true);
+                                                      r.setTrimLeadingWhiteSpace(true);
+                                                  },
+                                                  { "",
+                                                    "",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text#",
+                                                    "expected text\t #",
+                                                    "expected text#",
+                                                    "expected text   # not expected",
+                                                    "#",
+                                                    "#",
+                                                    "# not expected", }
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimTrailingComment(true, '#');
+                                                  },
+                                                  { "\n",
+                                                    " \t \n",
+                                                    "expected text\n",
+                                                    " expected text \n",
+                                                    "expected text \t\n",
+                                                    " \t expected text\n",
+                                                    " \t expected text \t\n",
+                                                    "expected text",
+                                                    "expected text\t ",
+                                                    "expected text",
+                                                    "expected text   ",
+                                                    "",
+                                                    "\t ",
+                                                    "   ", }
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimTrailingComment(true, '#');
+                                                      r.setTrimTrailingComment(false, 0);
+                                                  },
+                                                  g_unchangedOutputs
+                                              },
+                                              TextReaderTestParams {
+                                                  g_inputs,
+                                                  [](TextReader &r)
+                                                  {
+                                                      r.setTrimTrailingComment(true, '#');
+                                                      r.setTrimTrailingWhiteSpace(true);
+                                                  },
+                                                  { "",
+                                                    "",
+                                                    "expected text",
+                                                    " expected text",
+                                                    "expected text",
+                                                    " \t expected text",
+                                                    " \t expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "expected text",
+                                                    "",
+                                                    "",
+                                                    "",   }
+                                              }
+                                              ));
+
+} // namespace
+} // namespace
+} // namespace
index dd727bb93cfd5cd0628a698b23763ed88533399e..f0a14e82f5a245cfc7ff6b1cea79c242471380f0 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015, by the GROMACS development team, led by
+ * Copyright (c) 2015,2017, 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.
@@ -65,15 +65,30 @@ std::string TextReader::readFileToString(const std::string &filename)
     return readFileToString(filename.c_str());
 }
 
+//! Implementation class
 class TextReader::Impl
 {
     public:
+        //! Constructor.
         explicit Impl(const TextInputStreamPointer &stream)
-            : stream_(stream)
+            : stream_(stream), trimLeadingWhiteSpace_(false), trimTrailingWhiteSpace_(false),
+              trimTrailingComment_(false), commentChar_(0)
         {
         }
 
+        //! Stream used by this reader.
         TextInputStreamPointer stream_;
+        //! Whether leading whitespace should be removed.
+        bool                   trimLeadingWhiteSpace_;
+        //! Whether trailing whitespace should be removed.
+        bool                   trimTrailingWhiteSpace_;
+        //! Whether a trailing comment should be removed.
+        bool                   trimTrailingComment_;
+        /*! \brief Character that denotes the start of a comment on a line.
+         *
+         * Zero until TextReader::setTrimTrailingComment is called to
+         * activate such trimming with a given character. */
+        char                   commentChar_;
 };
 
 TextReader::TextReader(const std::string &filename)
@@ -95,25 +110,68 @@ TextReader::~TextReader()
 {
 }
 
-bool TextReader::readLine(std::string *line)
+bool TextReader::readLine(std::string *linePtr)
 {
-    return impl_->stream_->readLine(line);
-}
-
-bool TextReader::readLineTrimmed(std::string *line)
-{
-    if (!readLine(line))
+    if (!impl_->stream_->readLine(linePtr))
     {
         return false;
     }
-    const size_t endPos = line->find_last_not_of(" \t\r\n");
-    if (endPos != std::string::npos)
+    auto      &line              = *linePtr;
+    const char whiteSpaceChars[] = " \t\r\n";
+    if (impl_->trimLeadingWhiteSpace_)
+    {
+        const size_t endPos = line.find_first_not_of(whiteSpaceChars);
+        if (endPos == std::string::npos)
+        {
+            line.resize(0);
+        }
+        else
+        {
+            line = line.substr(endPos, std::string::npos);
+        }
+    }
+    if (impl_->trimTrailingComment_)
     {
-        line->resize(endPos + 1);
+        auto commentPos = line.find(impl_->commentChar_);
+        if (commentPos != std::string::npos)
+        {
+            line.resize(commentPos);
+        }
+    }
+    if (impl_->trimTrailingWhiteSpace_)
+    {
+        const size_t endPos = line.find_last_not_of(whiteSpaceChars);
+        if (endPos == std::string::npos)
+        {
+            line.resize(0);
+        }
+        else
+        {
+            line.resize(endPos + 1);
+        }
     }
     return true;
 }
 
+void TextReader::setTrimLeadingWhiteSpace(bool doTrimming)
+{
+    impl_->trimLeadingWhiteSpace_ = doTrimming;
+}
+
+void TextReader::setTrimTrailingWhiteSpace(bool doTrimming)
+{
+    impl_->trimTrailingWhiteSpace_ = doTrimming;
+}
+
+void TextReader::setTrimTrailingComment(bool doTrimming, char commentChar)
+{
+    impl_->trimTrailingComment_ = doTrimming;
+    if (impl_->trimTrailingComment_)
+    {
+        impl_->commentChar_ = commentChar;
+    }
+}
+
 std::string TextReader::readAll()
 {
     std::string result;
index 3d425ec9d1f7516c4708fa7b2a89bdeeea172efa..df713459de6a8938f5e32ce281adb4d3a70ae264 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2015, by the GROMACS development team, led by
+ * Copyright (c) 2015,2017, 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.
@@ -133,22 +133,37 @@ class TextReader
                // ...
            }
            \endcode
+         *
+         * Behaviours such as trimming whitespace or comments can be
+         * configured by calling other methods before this one.
          */
         bool readLine(std::string *line);
-        /*! \brief
-         * Reads a single line from the stream.
+        /*! \brief Sets whether the reader should trim leading whitespace
+         * from a line before returning it.
          *
-         * \param[out] line    String to receive the line.
-         * \returns    false if nothing was read because the file ended.
+         * \param[in] doTrimming  Whether trimming should be active.
+         */
+        void setTrimLeadingWhiteSpace(bool doTrimming);
+        /*! \brief Sets whether the reader should trim trailing whitespace
+         * from a line before returning it.
          *
-         * On error or when false is returned, \p line will be empty.
-         * Works as readLine(), except that trailing whitespace will be removed
-         * from \p line.
+         * Note that comment trimming will precede whitespace trimming
+         * when both are active.
          *
-         * \see readLine()
+         * \param[in] doTrimming  Whether trimming should be active.
          */
-        bool readLineTrimmed(std::string *line);
-
+        void setTrimTrailingWhiteSpace(bool doTrimming);
+        /*! \brief Sets whether the reader should trim at trailing
+         * comment from a line before returning it.
+         *
+         * Note that comment trimming will precede whitespace trimming
+         * when both are active.
+         *
+         * \param[in]  commentChar  The character that begins a comment.
+         *
+         * \param[in] doTrimming  Whether trimming should be active.
+         */
+        void setTrimTrailingComment(bool doTrimming, char commentChar);
         /*! \brief
          * Reads all remaining lines from the stream as a single string.
          *
index 39d648bb29605eada28de9236c8de9092ada59a0..1d28534ed64118a0ad7f03d84cf3c15a61f05f08 100644 (file)
@@ -55,7 +55,9 @@
 #include "gromacs/topology/mtop_util.h"
 #include "gromacs/topology/topology.h"
 #include "gromacs/utility/cstringutil.h"
+#include "gromacs/utility/exceptions.h"
 #include "gromacs/utility/fatalerror.h"
+#include "gromacs/utility/filestream.h"
 #include "gromacs/utility/futil.h"
 #include "gromacs/utility/smalloc.h"
 
@@ -221,7 +223,9 @@ static void get_input(const char *membed_input, real *xy_fac, real *xy_max, real
 
     wi = init_warning(TRUE, 0);
 
-    inp = read_inpfile(membed_input, &ninp, wi);
+    gmx::TextInputFile stream(membed_input);
+    inp = read_inpfile(&stream, membed_input, &ninp, wi);
+
     ITYPE ("nxy", *it_xy, 1000);
     ITYPE ("nz", *it_z, 0);
     RTYPE ("xyinit", *xy_fac, 0.5);
@@ -1026,7 +1030,6 @@ gmx_membed_t *init_membed(FILE *fplog, int nfile, const t_filenm fnm[], gmx_mtop
     gmx_membed_t             *membed    = nullptr;
 
     /* input variables */
-    const char *membed_input;
     real        xy_fac           = 0.5;
     real        xy_max           = 1.0;
     real        z_fac            = 1.0;
@@ -1055,9 +1058,13 @@ gmx_membed_t *init_membed(FILE *fplog, int nfile, const t_filenm fnm[], gmx_mtop
     if (MASTER(cr))
     {
         /* get input data out membed file */
-        membed_input = opt2fn("-membed", nfile, fnm);
-        get_input(membed_input, &xy_fac, &xy_max, &z_fac, &z_max, &it_xy, &it_z, &probe_rad, &low_up_rm,
-                  &maxwarn, &pieces, &bALLOW_ASYMMETRY);
+        try
+        {
+            get_input(opt2fn("-membed", nfile, fnm),
+                      &xy_fac, &xy_max, &z_fac, &z_max, &it_xy, &it_z, &probe_rad, &low_up_rm,
+                      &maxwarn, &pieces, &bALLOW_ASYMMETRY);
+        }
+        GMX_CATCH_ALL_AND_EXIT_WITH_FATAL_ERROR;
 
         if (!EI_DYNAMICS(inputrec->eI) )
         {
index 7f475b8e316c2b7b7de9b4644ed3c098863f7cc6..ca0322e6ca5f61379923852303bd25b8f7a690bf 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * This file is part of the GROMACS molecular simulation package.
  *
- * Copyright (c) 2012,2013,2014,2015, by the GROMACS development team, led by
+ * Copyright (c) 2012,2013,2014,2015,2017, 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.
@@ -65,6 +65,10 @@ class TestReferenceChecker;
  * If this flag is not given, they check the strings using the XML reference
  * framework (see TestReferenceData).
  *
+ * Tests that need simple checking of a string, or container of
+ * strings, should consider the normal implementation in
+ * TestReferenceChecker.
+ *
  * \inlibraryapi
  * \ingroup module_testutils
  */