Rewrote command line help writer.
authorTeemu Murtola <teemu.murtola@gmail.com>
Thu, 26 Apr 2012 05:59:37 +0000 (08:59 +0300)
committerTeemu Murtola <teemu.murtola@gmail.com>
Thu, 3 May 2012 04:32:49 +0000 (07:32 +0300)
- Rewrote printing code for options using TextTableFormatter. Should now
  produce at least as good output as the old code invoked by
  parse_common_args() while keeping most of the layout, and in some
  cases does significantly better (partly because of the item below).
- Split selection options into a separate table with its own formatting
  to make more space for long values that they typically have.
- Added test code that exercises different parts of the formatting code
  and allows one to easily see the output (with -stdout cmd-line
  parameter to the test executable).  Without the -stdout parameter,
  each test is marked as failed if anything in the output changes,
  allowing them to work as regression tests.
- Added line wrapping for descriptions.
- Markup substitution ([TT], [PAR] etc.) is not yet done. Will add that
  separately, since it requires changes to the line wrapping code to
  strip some whitespace...

There are some TODOs in the code, but they mostly concern better
handling of corner or more complex cases.

IssueID #666

Change-Id: I538fbaac27572a518d24dff7b0f30f8746a540af

src/gromacs/commandline/cmdlinehelpwriter.cpp
src/gromacs/commandline/tests/CMakeLists.txt
src/gromacs/commandline/tests/cmdlinehelpwriter.cpp [new file with mode: 0644]
src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml [new file with mode: 0644]
src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongOptions.xml [new file with mode: 0644]
src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesMultipleSections.xml [new file with mode: 0644]
src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml [new file with mode: 0644]
src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesSelectionOptions.xml [new file with mode: 0644]
src/gromacs/options/optioninfo.h
src/gromacs/options/optionsvisitor.cpp

index 74fdc4e0d6ce985536dcb6010a25fef198d35bc5..6ea6074f79bed85a88bcd138aa12b70749315e32 100644 (file)
@@ -46,6 +46,8 @@
 #include "gromacs/options/filenameoptioninfo.h"
 #include "gromacs/options/options.h"
 #include "gromacs/options/optionsvisitor.h"
+#include "gromacs/selection/selectionoptioninfo.h"
+#include "gromacs/utility/format.h"
 
 #include "cmdlinehelpwriter-impl.h"
 
@@ -55,6 +57,12 @@ namespace gmx
 namespace
 {
 
+std::string substituteMarkup(const std::string &text)
+{
+    // TODO: Implement.
+    return text;
+}
+
 /********************************************************************
  * DescriptionWriter
  */
@@ -81,14 +89,15 @@ void DescriptionWriter::visitSubSection(const Options &section)
 {
     if (!section.description().empty())
     {
-        fprintf(fp_, "\n");
         const std::string &title = section.title();
         if (!title.empty())
         {
             fprintf(fp_, "%s\n\n", title.c_str());
         }
-        // TODO: Wrap lines and do markup substitutions.
-        fprintf(fp_, "%s\n\n", section.description().c_str());
+        TextLineWrapper wrapper;
+        wrapper.setLineLength(78);
+        std::string description(substituteMarkup(section.description()));
+        fprintf(fp_, "%s\n\n", wrapper.wrapToString(description).c_str());
     }
     OptionsIterator(section).acceptSubSections(this);
 }
@@ -107,22 +116,28 @@ class FileParameterWriter : public OptionsTypeVisitor<FileNameOptionInfo>
 {
     public:
         //! Creates a helper object for writing file parameters.
-        explicit FileParameterWriter(FILE *fp)
-            : fp_(fp), bFirst_(true)
-        {
-        }
+        explicit FileParameterWriter(FILE *fp);
 
         //! Returns true if anything was written out.
-        bool didOutput() const { return !bFirst_; }
+        bool didOutput() const { return formatter_.didOutput(); }
 
         virtual void visitSubSection(const Options &section);
         virtual void visitOptionType(const FileNameOptionInfo &option);
 
     private:
         FILE                   *fp_;
-        bool                    bFirst_;
+        TextTableFormatter      formatter_;
 };
 
+FileParameterWriter::FileParameterWriter(FILE *fp)
+    : fp_(fp)
+{
+    formatter_.addColumn("Option",      6, false);
+    formatter_.addColumn("Filename",    12, false);
+    formatter_.addColumn("Type",        12, false);
+    formatter_.addColumn("Description", 45, true);
+}
+
 void FileParameterWriter::visitSubSection(const Options &section)
 {
     OptionsIterator iterator(section);
@@ -132,66 +147,82 @@ void FileParameterWriter::visitSubSection(const Options &section)
 
 void FileParameterWriter::visitOptionType(const FileNameOptionInfo &option)
 {
-    if (bFirst_)
-    {
-        fprintf(fp_, "%6s %12s  %-12s %s\n",
-                "Option", "Filename", "Type", "Description");
-        fprintf(fp_, "------------------------------------------------------------\n");
-        bFirst_ = false;
-    }
-
-    std::string optionLine("-");
-    optionLine.reserve(30 + option.description().size());
-    optionLine.append(option.name()).append(" ");
-    if (optionLine.size() < 11)
-    {
-        optionLine.resize(11, ' ');
-    }
-    bool bTypePrinted = false;
-    size_t lineStart = 0;
+    int firstShortValue = 0; // The first value after which the type fits.
+    int firstLongValue = -1; // First value that overlaps description column.
+    int lastLongValue = -1;  // Last value like the above.
+
+    // Get the values to write and check where text overflows the columns.
+    formatter_.clear();
+    std::string name(formatString("-%s", option.name().c_str()));
+    formatter_.addColumnLine(0, name);
     for (int i = 0; i < option.valueCount(); ++i)
     {
-        if (i > 0)
+        std::string value(option.formatValue(i));
+        formatter_.addColumnLine(1, value);
+        if (value.length() > 12U && i == firstShortValue)
         {
-            optionLine.append("\n");
-            lineStart = optionLine.size();
-            optionLine.append(11, ' ');
+            firstShortValue = i + 1;
         }
-        optionLine.append(option.formatValue(i)).append(" ");
-        // TODO: Do eliding
-        if (optionLine.size() <= lineStart + 21)
+        if (value.length() > 25U)
         {
-            optionLine.resize(lineStart + 21, ' ');
-            if (!bTypePrinted)
+            if (firstLongValue == -1)
             {
-                optionLine.append(option.type()).append(" ");
-                bTypePrinted = true;
+                firstLongValue = i;
             }
+            lastLongValue = i;
         }
     }
-    if (!bTypePrinted)
+    std::string type;
+    if (option.isInputOutputFile())
+    {
+        type = "In/Out";
+    }
+    else if (option.isInputFile())
+    {
+        type = "Input";
+    }
+    else if (option.isOutputFile())
+    {
+        type = "Output";
+    }
+    if (!option.isRequired())
+    {
+        type += ", Opt.";
+    }
+    if (option.isLibraryFile())
     {
-        optionLine.append("\n");
-        lineStart = optionLine.size();
-        optionLine.append(21, ' ');
-        optionLine.append(option.type()).append(" ");
+        type += ", Lib.";
     }
-    if (optionLine.size() > lineStart + 34)
+    bool bLongType = (type.length() > 12U);
+    formatter_.addColumnLine(2, type);
+    formatter_.addColumnLine(3, substituteMarkup(option.description()));
+
+    // Compute layout.
+    if (name.length() > 6U || firstShortValue > 0)
     {
-        if (!option.description().empty())
+        formatter_.setColumnFirstLineOffset(1, 1);
+        // Assume that the name is <20 chars, so that the type fits
+        if (firstLongValue >= 0)
         {
-            optionLine.append("\n");
-            optionLine.append(34, ' ');
+            ++firstLongValue;
+            ++lastLongValue;
         }
     }
-    else
+    int firstDescriptionLine = 0;
+    if (bLongType)
     {
-        optionLine.resize(lineStart + 34, ' ');
+        firstDescriptionLine = 1;
     }
-    // TODO: Markup substitution.
-    optionLine.append(option.description());
-    // TODO: Wrap lines.
-    fprintf(fp_, "%s\n", optionLine.c_str());
+    formatter_.setColumnFirstLineOffset(3, firstDescriptionLine);
+    if (firstLongValue >= 0 && formatter_.lastColumnLine(3) >= firstLongValue)
+    {
+        firstDescriptionLine = lastLongValue + 1;
+        formatter_.setColumnFirstLineOffset(3, firstDescriptionLine);
+    }
+
+    // Do the formatting.
+    std::string row = formatter_.formatRow();
+    fprintf(fp_, "%s", row.c_str());
 }
 
 
@@ -208,25 +239,31 @@ class ParameterWriter : public OptionsVisitor
 {
     public:
         //! Creates a helper object for writing non-file parameters.
-        explicit ParameterWriter(FILE *fp)
-            : fp_(fp), bFirst_(true), bShowHidden_(false)
-        {
-        }
+        explicit ParameterWriter(FILE *fp);
 
         //! Sets the writer to show hidden options.
         void setShowHidden(bool bSet) { bShowHidden_ = bSet; }
         //! Returns true if anything was written out.
-        bool didOutput() const { return !bFirst_; }
+        bool didOutput() const { return formatter_.didOutput(); }
 
         virtual void visitSubSection(const Options &section);
         virtual void visitOption(const OptionInfo &option);
 
     private:
         FILE                   *fp_;
-        bool                    bFirst_;
+        TextTableFormatter      formatter_;
         bool                    bShowHidden_;
 };
 
+ParameterWriter::ParameterWriter(FILE *fp)
+    : fp_(fp), bShowHidden_(false)
+{
+    formatter_.addColumn("Option",      12, false);
+    formatter_.addColumn("Type",         6, false);
+    formatter_.addColumn("Value",        6, false);
+    formatter_.addColumn("Description", 51, true);
+}
+
 void ParameterWriter::visitSubSection(const Options &section)
 {
     OptionsIterator iterator(section);
@@ -237,53 +274,101 @@ void ParameterWriter::visitSubSection(const Options &section)
 void ParameterWriter::visitOption(const OptionInfo &option)
 {
     if (option.isType<FileNameOptionInfo>()
+        || option.isType<SelectionOptionInfo>()
         || (!bShowHidden_ && option.isHidden()))
     {
         return;
     }
 
-    if (bFirst_)
+    formatter_.clear();
+    bool bIsBool = option.isType<BooleanOptionInfo>();
+    std::string name(formatString("-%s%s", bIsBool ? "[no]" : "",
+                                           option.name().c_str()));
+    formatter_.addColumnLine(0, name);
+    formatter_.addColumnLine(1, option.type());
+    if (name.length() > 12U)
     {
-        fprintf(fp_, "%-12s %-6s %-6s  %s\n",
-                "Option", "Type", "Value", "Description");
-        fprintf(fp_, "----------------------------------------------------\n");
-        bFirst_ = false;
+        formatter_.setColumnFirstLineOffset(1, 1);
     }
-
-    std::string optionLine("-");
-    optionLine.reserve(30 + option.description().size());
-    if (option.isType<BooleanOptionInfo>())
-    {
-        optionLine.append("[no]");
-    }
-    optionLine.append(option.name()).append(" ");
-    if (optionLine.size() < 13)
-    {
-        optionLine.resize(13, ' ');
-    }
-    optionLine.append(option.type()).append(" ");
-    if (optionLine.size() < 20)
-    {
-        optionLine.resize(20, ' ');
-    }
-    optionLine.append(option.formatValues()).append(" ");
-    if (optionLine.size() > 28)
+    // TODO: Better handling of multiple long values
+    std::string values;
+    for (int i = 0; i < option.valueCount(); ++i)
     {
-        // TODO: Wrap lines / do eliding
-        if (!option.description().empty())
+        if (i != 0)
         {
-            optionLine.append("\n");
-            optionLine.append(28, ' ');
+            values.append(" ");
         }
+        values.append(option.formatValue(i));
+    }
+    formatter_.addColumnLine(2, values);
+    formatter_.addColumnLine(3, substituteMarkup(option.description()));
+    if (values.length() > 6U)
+    {
+        formatter_.setColumnFirstLineOffset(3, 1);
     }
-    else
+
+    std::string row = formatter_.formatRow();
+    fprintf(fp_, "%s", row.c_str());
+}
+
+
+/********************************************************************
+ * SelectionParameterWriter
+ */
+
+/*! \internal \brief
+ * Helper object for writing help for selection parameters.
+ *
+ * \ingroup module_commandline
+ */
+class SelectionParameterWriter : public OptionsTypeVisitor<SelectionOptionInfo>
+{
+    public:
+        //! Creates a helper object for writing selection parameters.
+        explicit SelectionParameterWriter(FILE *fp);
+
+        //! Returns true if anything was written out.
+        bool didOutput() const { return formatter_.didOutput(); }
+
+        virtual void visitSubSection(const Options &section);
+        virtual void visitOptionType(const SelectionOptionInfo &option);
+
+    private:
+        FILE                   *fp_;
+        TextTableFormatter      formatter_;
+};
+
+SelectionParameterWriter::SelectionParameterWriter(FILE *fp)
+    : fp_(fp)
+{
+    formatter_.addColumn("Selection",   10, false);
+    formatter_.addColumn("Description", 67, true);
+}
+
+void SelectionParameterWriter::visitSubSection(const Options &section)
+{
+    OptionsIterator iterator(section);
+    iterator.acceptSubSections(this);
+    iterator.acceptOptions(this);
+}
+
+void SelectionParameterWriter::visitOptionType(const SelectionOptionInfo &option)
+{
+    formatter_.clear();
+    std::string name(formatString("-%s", option.name().c_str()));
+    formatter_.addColumnLine(0, name);
+    formatter_.addColumnLine(1, substituteMarkup(option.description()));
+    std::string row = formatter_.formatRow();
+    fprintf(fp_, "%s", row.c_str());
+
+    // TODO: What to do with selection variables?
+    // They are not printed as values for any option.
+    for (int i = 0; i < option.valueCount(); ++i)
     {
-        optionLine.resize(28, ' ');
+        std::string value(option.formatValue(i));
+        // TODO: Wrapping
+        fprintf(fp_, "    %s\n", value.c_str());
     }
-    // TODO: Markup substitution.
-    optionLine.append(option.description());
-    // TODO: Wrap lines.
-    fprintf(fp_, "%s\n", optionLine.c_str());
 }
 
 } // namespace
@@ -327,7 +412,7 @@ void CommandLineHelpWriter::writeHelp(FILE *fp)
     if (impl_->bShowDescriptions_)
     {
         fprintf(fp, "DESCRIPTION\n"
-                    "-----------\n");
+                    "-----------\n\n");
         DescriptionWriter(fp).visitSubSection(impl_->options_);
     }
     {
@@ -347,6 +432,14 @@ void CommandLineHelpWriter::writeHelp(FILE *fp)
             fprintf(fp, "\n");
         }
     }
+    {
+        SelectionParameterWriter writer(fp);
+        writer.visitSubSection(impl_->options_);
+        if (writer.didOutput())
+        {
+            fprintf(fp, "\n");
+        }
+    }
 }
 
 } // namespace gmx
index 862bd11aca226ce3c9152d5bf3f39a18aff9276f..db11bf5fd281baf2d6a659f7496fbcbe4e0e1d12 100644 (file)
@@ -1,2 +1,4 @@
-add_gtest_test(CommandLineUnitTests commandline-test
-               cmdlineparser.cpp)
+if (TESTUTILS_HAVE_REFDATA)
+    add_gtest_test(CommandLineUnitTests commandline-test
+                   cmdlinehelpwriter.cpp cmdlineparser.cpp)
+endif ()
diff --git a/src/gromacs/commandline/tests/cmdlinehelpwriter.cpp b/src/gromacs/commandline/tests/cmdlinehelpwriter.cpp
new file mode 100644 (file)
index 0000000..7636827
--- /dev/null
@@ -0,0 +1,316 @@
+/*
+ *
+ *                This source code is part of
+ *
+ *                 G   R   O   M   A   C   S
+ *
+ *          GROningen MAchine for Chemical Simulations
+ *
+ * Written by David van der Spoel, Erik Lindahl, Berk Hess, and others.
+ * Copyright (c) 1991-2000, University of Groningen, The Netherlands.
+ * Copyright (c) 2001-2009, The GROMACS development team,
+ * check out http://www.gromacs.org for more information.
+
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * If you want to redistribute modifications, 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 www.gromacs.org.
+ *
+ * To help us fund GROMACS development, we humbly ask that you cite
+ * the papers on the package - you can find them in the top README file.
+ *
+ * For more info, check our website at http://www.gromacs.org
+ */
+/*! \internal \file
+ * \brief
+ * Tests gmx::CommandLineHelpWriter.
+ *
+ * These tests fail for any change in the output, and it should be reviewed
+ * whether the change was intentional.
+ * For development, the tests can be run with a '-stdout' command-line option
+ * to print out the help to stdout instead of using the XML reference
+ * framework.
+ *
+ * \author Teemu Murtola <teemu.murtola@cbr.su.se>
+ * \ingroup module_commandline
+ */
+#include <boost/scoped_ptr.hpp>
+#include <gtest/gtest.h>
+
+#include "gromacs/legacyheaders/types/simple.h"
+
+#include "gromacs/commandline/cmdlinehelpwriter.h"
+#include "gromacs/options/basicoptions.h"
+#include "gromacs/options/filenameoption.h"
+#include "gromacs/options/options.h"
+#include "gromacs/selection/selectionoption.h"
+#include "gromacs/selection/selectionoptioninfo.h"
+#include "gromacs/selection/selectioncollection.h"
+#include "gromacs/utility/file.h"
+
+#include "testutils/refdata.h"
+#include "testutils/testoptions.h"
+
+namespace
+{
+
+using gmx::test::TestReferenceData;
+using gmx::test::TestReferenceChecker;
+
+class CommandLineHelpWriterTest : public ::testing::Test
+{
+    public:
+        static void SetUpTestCase();
+
+        static bool                     s_bWriteToStdOut;
+
+        CommandLineHelpWriterTest();
+        ~CommandLineHelpWriterTest();
+
+        TestReferenceChecker &checker();
+
+        void checkHelp(gmx::CommandLineHelpWriter *writer);
+
+        std::string                     helpfile_;
+
+    private:
+        TestReferenceData       data_;
+        boost::scoped_ptr<TestReferenceChecker> checker_;
+};
+
+bool CommandLineHelpWriterTest::s_bWriteToStdOut = false;
+
+void CommandLineHelpWriterTest::SetUpTestCase()
+{
+    gmx::Options options(NULL, NULL);
+    options.addOption(gmx::BooleanOption("stdout").store(&s_bWriteToStdOut));
+    gmx::test::parseTestOptions(&options);
+}
+
+CommandLineHelpWriterTest::CommandLineHelpWriterTest()
+{
+    const ::testing::TestInfo *test_info =
+        ::testing::UnitTest::GetInstance()->current_test_info();
+    // TODO: Make a common helper method, which would also specify directory
+    helpfile_ = std::string(test_info->test_case_name())
+        + "_" + test_info->name() + ".txt";
+}
+
+CommandLineHelpWriterTest::~CommandLineHelpWriterTest()
+{
+    std::remove(helpfile_.c_str());
+}
+
+TestReferenceChecker &CommandLineHelpWriterTest::checker()
+{
+    if (checker_.get() == NULL)
+    {
+        checker_.reset(new TestReferenceChecker(data_.rootChecker()));
+    }
+    return *checker_;
+}
+
+void CommandLineHelpWriterTest::checkHelp(gmx::CommandLineHelpWriter *writer)
+{
+    gmx::File file(helpfile_, "w");
+    writer->writeHelp(file.handle());
+    file.close();
+
+    std::string text = gmx::File::readToString(helpfile_);
+    if (s_bWriteToStdOut)
+    {
+        printf("%s", text.c_str());
+    }
+    else
+    {
+        checker().checkStringBlock(text, "HelpText");
+    }
+}
+
+
+/********************************************************************
+ * Tests start here
+ */
+
+/*
+ * Tests help printing for each option type, but doesn't contain much
+ * variablity in the options.
+ */
+TEST_F(CommandLineHelpWriterTest, HandlesOptionTypes)
+{
+    using namespace gmx;
+
+    Options options("test", "Short Description");
+    options.addOption(BooleanOption("bool").description("Boolean option")
+                        .defaultValue(true));
+    options.addOption(BooleanOption("hidden").description("Hidden option")
+                        .hidden().defaultValue(true));
+    options.addOption(IntegerOption("int").description("Integer option")
+                        .defaultValue(2));
+    ivec intvec = {1, 2, 3};
+    options.addOption(IntegerOption("ivec").description("Integer vector option")
+                        .vector().store(intvec));
+    options.addOption(DoubleOption("double").description("Double option")
+                        .defaultValue(2.5));
+    dvec dblvec = {1.1, 2.3, 3.2};
+    options.addOption(DoubleOption("dvec").description("Double vector option")
+                        .vector().store(dblvec));
+    options.addOption(DoubleOption("time").description("Time option")
+                        .defaultValue(10.0));
+    options.addOption(StringOption("string").description("String option")
+                        .defaultValue("test"));
+    const char * const enumValues[] = {"no", "opt1", "opt2", NULL};
+    options.addOption(StringOption("enum").description("Enum option")
+                        .enumValue(enumValues).defaultEnumIndex(0));
+
+    options.addOption(FileNameOption("f")
+                        .description("Input file description")
+                        .filetype(eftTrajectory).inputFile().required()
+                        .defaultValue("traj"));
+    options.addOption(FileNameOption("lib")
+                        .description("Library file description")
+                        .filetype(eftGenericData).inputFile().libraryFile()
+                        .defaultValueIfSet("libdata"));
+    options.addOption(FileNameOption("io")
+                        .description("Input/Output file description")
+                        .filetype(eftGenericData).inputOutputFile());
+    options.addOption(FileNameOption("o")
+                        .description("Output file description")
+                        .filetype(eftPlot).outputFile());
+
+    options.addOption(SelectionOption("sel").description("Selection option"));
+
+    CommandLineHelpWriter writer(options);
+    writer.setShowHidden(true);
+    checkHelp(&writer);
+}
+
+/*
+ * Tests help printing with file name options with various values that don't
+ * fit into the allocated columns.
+ */
+TEST_F(CommandLineHelpWriterTest, HandlesLongFileOptions)
+{
+    using gmx::FileNameOption;
+    using gmx::eftGenericData;
+    using gmx::eftTrajectory;
+
+    gmx::Options options(NULL, NULL);
+    options.addOption(FileNameOption("f")
+                        .description("File name option with a long value")
+                        .filetype(eftTrajectory).inputFile().required()
+                        .defaultValue("path/to/long/trajectory/name"));
+    options.addOption(FileNameOption("f2")
+                        .description("File name option with a long value")
+                        .filetype(eftTrajectory).inputFile().required()
+                        .defaultValue("path/to/long/trajectory"));
+    options.addOption(FileNameOption("lib")
+                        .description("File name option with a long value and type")
+                        .filetype(eftTrajectory).inputFile().libraryFile()
+                        .defaultValue("path/to/long/trajectory/name"));
+    options.addOption(FileNameOption("longfileopt")
+                        .description("File name option with a long name")
+                        .filetype(eftGenericData).inputFile()
+                        .defaultValue("deffile"));
+    options.addOption(FileNameOption("longfileopt2")
+                        .description("File name option with multiple long fields")
+                        .filetype(eftGenericData).inputFile().libraryFile()
+                        .defaultValue("path/to/long/file/name"));
+
+    gmx::CommandLineHelpWriter writer(options);
+    checkHelp(&writer);
+}
+
+/*
+ * Tests help printing with general options with various values that don't
+ * fit into the allocated columns.
+ */
+TEST_F(CommandLineHelpWriterTest, HandlesLongOptions)
+{
+    using gmx::BooleanOption;
+    using gmx::DoubleOption;
+    using gmx::StringOption;
+
+    gmx::Options options(NULL, NULL);
+    options.addOption(BooleanOption("longboolean")
+                        .description("Boolean option with a long name")
+                        .defaultValue(true));
+    dvec dblvec = {1.135, 2.32, 3.2132};
+    options.addOption(DoubleOption("dvec").description("Double vector option")
+                        .vector().store(dblvec));
+    std::vector<std::string> values;
+    values.push_back("A very long string value that overflows even the description column");
+    values.push_back("Another very long string value that overflows even the description column");
+    options.addOption(StringOption("string")
+                        .description("String option with very long values (may "
+                                     "be less relevant with selections having "
+                                     "their own option type)")
+                        .storeVector(&values));
+
+    gmx::CommandLineHelpWriter writer(options);
+    checkHelp(&writer);
+}
+
+/*
+ * Tests help printing with selection options with values.
+ */
+TEST_F(CommandLineHelpWriterTest, HandlesSelectionOptions)
+{
+    using gmx::SelectionOption;
+
+    gmx::Options options(NULL, NULL);
+    options.addOption(SelectionOption("refsel").required()
+                        .description("Reference selection option"));
+    options.addOption(SelectionOption("sel").required().valueCount(2)
+                        .description("Selection option"));
+    gmx::SelectionCollection selections;
+    setSelectionCollectionForOptions(&options, &selections);
+    options.finish();
+    selections.parseRequestedFromString(
+            "resname SOL;"
+            "surface = within 0.5 of resname SOL;"
+            "group \"Protein\" and surface;"
+            "group \"Protein\" and not surface;");
+
+    gmx::CommandLineHelpWriter writer(options);
+    checkHelp(&writer);
+}
+
+/*
+ * Tests help printing for multiple sections.
+ */
+TEST_F(CommandLineHelpWriterTest, HandlesMultipleSections)
+{
+    using namespace gmx;
+
+    Options options("test", "Main Title");
+    Options subSect1("subsect1", "Subsection 1 Title");
+    Options subSect2("subsect2", "Subsection 2 Title");
+    Options subSect3("subsect3", NULL);
+    options.addSubSection(&subSect1);
+    options.addSubSection(&subSect2);
+    options.addSubSection(&subSect3);
+    options.setDescription("Description for main section.");
+    subSect1.setDescription("Description for subsection 1.");
+    subSect2.setDescription("Description for subsection 2.");
+    subSect3.setDescription("Description for subsection 3.");
+    options.addOption(IntegerOption("main")
+                        .description("Option in main section"));
+    subSect1.addOption(IntegerOption("sub1")
+                         .description("Option in subsection 1"));
+    subSect2.addOption(IntegerOption("sub2")
+                         .description("Option in subsection 2"));
+
+    CommandLineHelpWriter writer(options);
+    writer.setShowDescriptions(true);
+    checkHelp(&writer);
+}
+
+} // namespace
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongFileOptions.xml
new file mode 100644 (file)
index 0000000..b2f53b0
--- /dev/null
@@ -0,0 +1,20 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="HelpText"><![CDATA[
+Option Filename     Type         Description
+---------------------------------------------------------
+-f                  Input        File name option with a long value
+       path/to/long/trajectory/name
+-f2                 Input        File name option with a long value
+       path/to/long/trajectory
+-lib                Input, Opt., Lib.
+       path/to/long/trajectory/name
+                                 File name option with a long value and type
+-longfileopt        Input, Opt.  File name option with a long name
+       deffile
+-longfileopt2       Input, Opt., Lib.
+       path/to/long/file/name    File name option with multiple long fields
+
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongOptions.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesLongOptions.xml
new file mode 100644 (file)
index 0000000..fe9d7d0
--- /dev/null
@@ -0,0 +1,17 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="HelpText"><![CDATA[
+Option       Type   Value  Description
+---------------------------------------------------
+-[no]longboolean    yes    Boolean option with a long name
+             bool
+-dvec        vector 1.135 2.32 3.2132
+                           Double vector option
+-string      string A very long string value that overflows even the description column Another very long string value that overflows even the description column
+                           String option with very long values (may be less
+                           relevant with selections having their own option
+                           type)
+
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesMultipleSections.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesMultipleSections.xml
new file mode 100644 (file)
index 0000000..d9872f7
--- /dev/null
@@ -0,0 +1,29 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="HelpText"><![CDATA[
+DESCRIPTION
+-----------
+
+Main Title
+
+Description for main section.
+
+Subsection 1 Title
+
+Description for subsection 1.
+
+Subsection 2 Title
+
+Description for subsection 2.
+
+Description for subsection 3.
+
+Option       Type   Value  Description
+---------------------------------------------------
+-sub1        int           Option in subsection 1
+-sub2        int           Option in subsection 2
+-main        int           Option in main section
+
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesOptionTypes.xml
new file mode 100644 (file)
index 0000000..0a7f9a9
--- /dev/null
@@ -0,0 +1,31 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="HelpText"><![CDATA[
+Option Filename     Type         Description
+---------------------------------------------------------
+-f     traj         Input        Input file description
+-lib                Input, Opt., Lib.
+                                 Library file description
+-io                 In/Out, Opt. Input/Output file description
+-o                  Output, Opt. Output file description
+
+Option       Type   Value  Description
+---------------------------------------------------
+-[no]bool    bool   yes    Boolean option
+-[no]hidden  bool   yes    Hidden option
+-int         int    2      Integer option
+-ivec        vector 1 2 3  Integer vector option
+-double      double 2.5    Double option
+-dvec        vector 1.1 2.3 3.2
+                           Double vector option
+-time        double 10     Time option
+-string      string test   String option
+-enum        enum   no     Enum option: no, opt1, or opt2
+
+Selection  Description
+-----------------------------------
+-sel       Selection option
+
+]]></String>
+</ReferenceData>
diff --git a/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesSelectionOptions.xml b/src/gromacs/commandline/tests/refdata/CommandLineHelpWriterTest_HandlesSelectionOptions.xml
new file mode 100644 (file)
index 0000000..425b4b0
--- /dev/null
@@ -0,0 +1,14 @@
+<?xml version="1.0"?>
+<?xml-stylesheet type="text/xsl" href="referencedata.xsl"?>
+<ReferenceData>
+  <String Name="HelpText"><![CDATA[
+Selection  Description
+-----------------------------------
+-refsel    Reference selection option
+    resname SOL
+-sel       Selection option
+    group "Protein" and surface
+    group "Protein" and not surface
+
+]]></String>
+</ReferenceData>
index f4be835492f92f3998ed83359712f27e21e2b442..e65c0be6cbd411dae544c361238dd188c3f377ef 100644 (file)
@@ -116,8 +116,6 @@ class OptionInfo
         int valueCount() const;
         //! Returns the i'th value of the option as a string.
         std::string formatValue(int i) const;
-        //! Returns all the values of the option as a single string.
-        std::string formatValues() const;
 
     protected:
         /*! \cond libapi */
index 69c1b7bb68d55f103f76d0d012820ee335ece279..48167d17a03ec886c5ccc7dc92b5049e266d914c 100644 (file)
@@ -101,21 +101,6 @@ std::string OptionInfo::formatValue(int i) const
     return _option.formatValue(i);
 }
 
-std::string OptionInfo::formatValues() const
-{
-    std::string result;
-    int count = valueCount();
-    for (int i = 0; i < count; ++i)
-    {
-        if (i != 0)
-        {
-            result.append(" ");
-        }
-        result.append(formatValue(i));
-    }
-    return result;
-}
-
 /********************************************************************
  * OptionsIterator
  */