Better error messages for SelectionFileOption.

Use the added exception context information to provide the file name in
the error message when there is a problem with a selection provided with
-sf.  Also improve the error messages for cases where the number of
selections in the file does not match what is expected, or where the
assignment of selections to options is ambiguous.

Closes #656.

Change-Id: I6158960a970e6a22fb28f3872538f1de16883887

diff --git a/src/gromacs/selection/selectioncollection.cpp b/src/gromacs/selection/selectioncollection.cpp
index 6e8d228c78d12b18ac182c908f884917b39af48c..19b5727599103d59f79bde2a5c030b9451dc2156 100644 (file)
--- a/src/gromacs/selection/selectioncollection.cpp
+++ b/src/gromacs/selection/selectioncollection.cpp
@@ -499,15 +499,25 @@ SelectionCollection::parseFromStdin(int nr, bool bInteractive)
 SelectionList
 SelectionCollection::parseFromFile(const std::string &filename)
 {
-    yyscan_t scanner;
 
-    File file(filename, "r");
-    // TODO: Exception-safe way of using the lexer.
-    _gmx_sel_init_lexer(&scanner, &impl_->sc_, false, -1,
-                        impl_->bExternalGroupsSet_,
-                        impl_->grps_);
-    _gmx_sel_set_lex_input_file(scanner, file.handle());
-    return runParser(scanner, false, -1);
+    try
+    {
+        yyscan_t scanner;
+        File file(filename, "r");
+        // TODO: Exception-safe way of using the lexer.
+        _gmx_sel_init_lexer(&scanner, &impl_->sc_, false, -1,
+                            impl_->bExternalGroupsSet_,
+                            impl_->grps_);
+        _gmx_sel_set_lex_input_file(scanner, file.handle());
+        return runParser(scanner, false, -1);
+    }
+    catch (GromacsException &ex)
+    {
+        ex.prependContext(formatString(
+                    "Error in parsing selections from file '%s'",
+                    filename.c_str()));
+        throw;
+    }
 }
 
 

diff --git a/src/gromacs/selection/selectionoption.h b/src/gromacs/selection/selectionoption.h
index e970f8916aeb4e299577b2df1388122ffe6487a0..18c1b3e137902f79b6a2ffbbf97125070e050b8f 100644 (file)
--- a/src/gromacs/selection/selectionoption.h
+++ b/src/gromacs/selection/selectionoption.h
@@ -55,6 +55,14 @@ class SelectionOptionStorage;
  *
  * Public methods in this class do not throw.
  *
+ * \todo
+ * Support for specifying that an option accepts, e.g., two to four selections.
+ * Currently, only a fixed count or any number of selections is possible.
+ * \if internal
+ * In addition to allowing this in OptionTemplate, also SelectionOptionManager
+ * needs to be updated.
+ * \endif
+ *
  * \inpublicapi
  * \ingroup module_selection
  */

diff --git a/src/gromacs/selection/selectionoptionmanager.cpp b/src/gromacs/selection/selectionoptionmanager.cpp
index 6463dfe5b1a4ada99c308574bddc3a44eb44a479..df78579a946a3e020d617847b6a5767d3f5add9e 100644 (file)
--- a/src/gromacs/selection/selectionoptionmanager.cpp
+++ b/src/gromacs/selection/selectionoptionmanager.cpp
@@ -177,26 +177,40 @@ void SelectionOptionManager::Impl::placeSelectionsInRequests(
     SelectionList::const_iterator first = selections.begin();
     SelectionList::const_iterator last = first;
     RequestList::const_iterator i;
-    // TODO: Improve error messages.
     for (i = requests_.begin(); i != requests_.end(); ++i)
     {
         const SelectionRequest &request = *i;
         if (request.count() > 0)
         {
-            if (selections.end() - first < request.count())
+            int remaining = selections.end() - first;
+            if (remaining < request.count())
             {
-                GMX_THROW(InvalidInputError("Too few selections provided"));
+                int assigned = first - selections.begin();
+                GMX_THROW(InvalidInputError(formatString(
+                                "Too few selections provided for '%s': "
+                                "Expected %d selections, but only %d left "
+                                "after assigning the first %d to other selections.",
+                                request.name().c_str(), request.count(),
+                                remaining, assigned)));
             }
             last = first + request.count();
         }
         else
         {
-            if (i != requests_.end() - 1)
+            RequestList::const_iterator nextRequest = i;
+            ++nextRequest;
+            if (nextRequest != requests_.end())
             {
-                GMX_THROW(InvalidInputError(
-                            formatString("Request for selection '%s' must "
-                                         "not be followed by others",
-                                         request.name().c_str())));
+                const char *name = request.name().c_str();
+                const char *conflictName = nextRequest->name().c_str();
+                GMX_THROW(InvalidInputError(formatString(
+                                "Ambiguous selections for '%s' and '%s': "
+                                "Any number of selections is acceptable for "
+                                "'%s', but you have requested subsequent "
+                                "selections to be assigned to '%s'. "
+                                "Resolution for such cases is not implemented, "
+                                "and may be impossible.",
+                                name, conflictName, name, conflictName)));
             }
             last = selections.end();
         }
@@ -206,7 +220,14 @@ void SelectionOptionManager::Impl::placeSelectionsInRequests(
     }
     if (last != selections.end())
     {
-        GMX_THROW(InvalidInputError("Too many selections provided"));
+        int count = selections.end() - selections.begin();
+        int remaining = selections.end() - last;
+        int assigned = last - selections.begin();
+        GMX_THROW(InvalidInputError(formatString(
+                        "Too many selections provided: "
+                        "Expected %d selections, but %d provided. "
+                        "Last %d selections could not be assigned to any option.",
+                        assigned, count, remaining)));
     }
 }
 
@@ -298,7 +319,17 @@ void
 SelectionOptionManager::parseRequestedFromFile(const std::string &filename)
 {
     SelectionList selections = impl_->collection_.parseFromFile(filename);
-    impl_->placeSelectionsInRequests(selections);
+    try
+    {
+        impl_->placeSelectionsInRequests(selections);
+    }
+    catch (GromacsException &ex)
+    {
+        ex.prependContext(formatString(
+                    "Error in adding selections from file '%s'",
+                    filename.c_str()));
+        throw;
+    }
 }
 
 void