Bug #433 - Missing punctuation in some command output

- Made punctuation consistent throughout the code and addressed a few
    broken tests.

ChangeLog

@@ -12,6 +12,7 @@
  + Improved man pages (thanks to Andy Lester).
  + Default .taskrc files are now largely empty, and rely almost completed
    on default values.
+ + Fixed bug #433, making task command output more consistent.
 
 ------ old releases ------------------------------
 

src/API.cpp

@@ -612,14 +612,14 @@ bool API::callProgramHook (
 
   // Make call.
   if (lua_pcall (L, 0, 2, 0) != 0)
-    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1);
+    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1) + ".";
 
   // Call successful - get return values.
   if (!lua_isnumber (L, -2))
-    throw std::string ("Error: '") + function + "' did not return a success indicator";
+    throw std::string ("Error: '") + function + "' did not return a success indicator.";
 
   if (!lua_isstring (L, -1) && !lua_isnil (L, -1))
-    throw std::string ("Error: '") + function + "' did not return a message or nil";
+    throw std::string ("Error: '") + function + "' did not return a message or nil.";
 
   int rc              = lua_tointeger (L, -2);
   const char* message = lua_tostring  (L, -1);
@@ -684,17 +684,17 @@ bool API::callTaskHook (
 
   // Make call.
   if (lua_pcall (L, 1, 2, 0) != 0)
-    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1);
+    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1) + ".";
 
   // Hide the task.
   the_task = NULL;
 
   // Call successful - get return values.
   if (!lua_isnumber (L, -2))
-    throw std::string ("Error: '") + function + "' did not return a success indicator";
+    throw std::string ("Error: '") + function + "' did not return a success indicator.";
 
   if (!lua_isstring (L, -1) && !lua_isnil (L, -1))
-    throw std::string ("Error: '") + function + "' did not return a message or nil";
+    throw std::string ("Error: '") + function + "' did not return a message or nil.";
 
   int rc              = lua_tointeger (L, -2);
   const char* message = lua_tostring  (L, -1);
@@ -737,17 +737,17 @@ bool API::callFieldHook (
 
   // Make call.
   if (lua_pcall (L, 2, 3, 0) != 0)
-    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1);
+    throw std::string ("Error calling '") + function + "' - " + lua_tostring (L, -1) + ".";
 
   // Call successful - get return values.
   if (!lua_isstring (L, -3))
-    throw std::string ("Error: '") + function + "' did not return a modified value";
+    throw std::string ("Error: '") + function + "' did not return a modified value.";
 
   if (!lua_isnumber (L, -2))
-    throw std::string ("Error: '") + function + "' did not return a success indicator";
+    throw std::string ("Error: '") + function + "' did not return a success indicator.";
 
   if (!lua_isstring (L, -1) && !lua_isnil (L, -1))
-    throw std::string ("Error: '") + function + "' did not return a message or nil";
+    throw std::string ("Error: '") + function + "' did not return a message or nil.";
 
   const char* new_value = lua_tostring  (L, -3);
   int rc                = lua_tointeger (L, -2);

src/Att.cpp

@@ -256,7 +256,7 @@ bool Att::validNameValue (
     std::string combined;
     join (combined, ", ", matches);
 
-    throw error + combined;
+    throw error + combined + ".";
   }
 
   name = matches[0];
@@ -272,7 +272,7 @@ bool Att::validNameValue (
     autoComplete (mod, candidates, matches);
 
     if (matches.size () == 0)
-      throw std::string ("Unrecognized modifier '") + mod + "'";
+      throw std::string ("Unrecognized modifier '") + mod + "'.";
 
     else if (matches.size () != 1)
     {
@@ -282,7 +282,7 @@ bool Att::validNameValue (
       join (combined, ", ", matches);
       error += combined;
 
-      throw error + combined;
+      throw error + combined + ".";
     }
 
     mod = matches[0];
@@ -467,10 +467,10 @@ void Att::parse (Nibbler& n)
         if (validMod (mod))
           mMod = mod;
         else
-          throw std::string ("The name '") + mod + "' is not a valid modifier"; // TODO i18n
+          throw std::string ("The name '") + mod + "' is not a valid modifier."; // TODO i18n
       }
       else
-        throw std::string ("Missing . or : after modifier"); // TODO i18n
+        throw std::string ("Missing . or : after modifier."); // TODO i18n
     }
 
     if (n.skip (':'))
@@ -484,10 +484,10 @@ void Att::parse (Nibbler& n)
       }
     }
     else
-      throw std::string ("Missing : after attribute name"); // TODO i18n
+      throw std::string ("Missing : after attribute name."); // TODO i18n
   }
   else
-    throw std::string ("Missing : after attribute name"); // TODO i18n
+    throw std::string ("Missing : after attribute name."); // TODO i18n
 
 /* TODO This might be too slow to include.  Test this assumption.
   validNameValue (mName, mMod, mValue);
@@ -719,7 +719,7 @@ std::string Att::composeF4 () const
 void Att::mod (const std::string& input)
 {
   if (input != "" && !validMod (input))
-    throw std::string ("The name '") + input + "' is not a valid modifier"; // TODO i18n
+    throw std::string ("The name '") + input + "' is not a valid modifier."; // TODO i18n
 
   mMod = input;
 }

src/Config.cpp

@@ -457,13 +457,13 @@ void Config::parse (const std::string& input, int nest /* = 1 */)
             if (included.readable ())
               this->load (included, nest + 1);
             else
-              throw std::string ("Could not read include file '") + included.data + "'";
+              throw std::string ("Could not read include file '") + included.data + "'.";
           }
           else
             throw std::string ("Can only include files with absolute paths, not '") + included.data + "'";
         }
         else
-          throw std::string ("Malformed entry '") + line + "'";
+          throw std::string ("Malformed entry '") + line + "'.";
       }
     }
   }
@@ -497,7 +497,7 @@ void Config::createDefaultRC (const std::string& rc, const std::string& data)
 
   // Write out the new file.
   if (! File::write (rc, contents.str ()))
-    throw std::string ("Could not write to '") + rc + "'";
+    throw std::string ("Could not write to '") + rc + "'.";
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/Context.cpp

@@ -321,7 +321,7 @@ void Context::shadow ()
 
     // Optionally display a notification that the shadow file was updated.
     if (config.getBoolean ("shadow.notify"))
-      footnote (std::string ("[Shadow file '") + shadowFile.data + "' updated]");
+      footnote (std::string ("[Shadow file '") + shadowFile.data + "' updated.]");
 
     inShadow = false;
   }
@@ -751,7 +751,7 @@ void Context::parse (
       else
         throw stringtable.get (
           CMD_MISSING,
-          "You must specify a command, or a task ID to modify");
+          "You must specify a command, or a task ID to modify.");
     }
 
     // If the command "task 123" is entered, but with no modifier arguments,

src/Filter.cpp

@@ -132,7 +132,7 @@ void Filter::applySequence (std::vector<Task>& all, Sequence& sequence)
     std::vector <int> right;
     listDiff (filteredSequence, (std::vector <int>&)sequence, left, right);
     if (left.size ())
-      throw std::string ("Sequence filtering error - please report this error to support@taskwarrior.org");
+      throw std::string ("Sequence filtering error - please report this error to support@taskwarrior.org.");
 
     if (right.size ())
     {

src/Hooks.cpp

@@ -303,7 +303,7 @@ void Hooks::initialize ()
             (void) n.skip (',');
           }
           else
-            throw std::string ("Malformed hook definition '") + *it + "'";
+            throw std::string ("Malformed hook definition '") + *it + "'.";
         }
       }
     }
@@ -331,7 +331,7 @@ bool Hooks::trigger (const std::string& event)
           return false;
       }
       else
-        throw std::string ("Unrecognized hook event '") + event + "'";
+        throw std::string ("Unrecognized hook event '") + event + "'.";
     }
   }
 #endif
@@ -358,7 +358,7 @@ bool Hooks::trigger (const std::string& event, std::vector <Task>& tasks)
           return false;
       }
       else
-        throw std::string ("Unrecognized hook event '") + event + "'";
+        throw std::string ("Unrecognized hook event '") + event + "'.";
     }
   }
 #endif
@@ -385,7 +385,7 @@ bool Hooks::trigger (const std::string& event, Task& task)
           return false;
       }
       else
-        throw std::string ("Unrecognized hook event '") + event + "'";
+        throw std::string ("Unrecognized hook event '") + event + "'.";
     }
   }
 #endif
@@ -415,7 +415,7 @@ bool Hooks::trigger (
           return false;
       }
       else
-        throw std::string ("Unrecognized hook event '") + event + "'";
+        throw std::string ("Unrecognized hook event '") + event + "'.";
     }
   }
 #endif

src/Record.cpp

@@ -95,7 +95,7 @@ void Record::parse (const std::string& input)
   {
     if (line.length () == 0)
       throw context.stringtable.get (RECORD_EMPTY,
-                                     "Empty record in input");
+                                     "Empty record in input.");
 
     Nibbler nl (line);
     Att a;
@@ -110,11 +110,11 @@ void Record::parse (const std::string& input)
     nl.getUntilEOS (remainder);
     if (remainder.length ())
       throw context.stringtable.get (RECORD_EXTRA,
-                                     "Unrecognized characters at end of line");
+                                     "Unrecognized characters at end of line.");
   }
   else
     throw context.stringtable.get (RECORD_NOT_FF4,
-                                   "Record not recognized as format 4");
+                                   "Record not recognized as format 4.");
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/Sequence.cpp

@@ -96,7 +96,7 @@ void Sequence::parse (const std::string& input)
     case 1:
       {
         if (! validId (range[0]))
-          throw context.stringtable.get (SEQUENCE_BAD_SEQ, "Invalid ID in sequence");
+          throw context.stringtable.get (SEQUENCE_BAD_SEQ, "Invalid ID in sequence.");
 
         int id = atoi (range[0].c_str ());
         this->push_back (id);
@@ -107,15 +107,15 @@ void Sequence::parse (const std::string& input)
       {
         if (! validId (range[0]) ||
             ! validId (range[1]))
-          throw context.stringtable.get (SEQUENCE_BAD_SEQ, "Invalid ID in range");
+          throw context.stringtable.get (SEQUENCE_BAD_SEQ, "Invalid ID in range.");
 
         int low  = atoi (range[0].c_str ());
         int high = atoi (range[1].c_str ());
         if (low > high)
-          throw context.stringtable.get (SEQUENCE_INVERTED, "Inverted sequence range high-low");
+          throw context.stringtable.get (SEQUENCE_INVERTED, "Inverted sequence range high-low.");
 
         if (high - low >= SEQUENCE_MAX)
-          throw context.stringtable.get (SEQUENCE_RANGE_MAX, "ID Range too large");
+          throw context.stringtable.get (SEQUENCE_RANGE_MAX, "ID Range too large.");
 
         for (int i = low; i <= high; ++i)
           this->push_back (i);

src/Subst.cpp

@@ -105,15 +105,15 @@ void Subst::parse (const std::string& input)
 
     if (mFrom == "")
       throw context.stringtable.get (SUBST_EMPTY,
-                                     "Cannot substitute an empty string");
+                                     "Cannot substitute an empty string.");
 
     if (!n.depleted ())
       throw context.stringtable.get (SUBST_BAD_CHARS,
-                                     "Unrecognized character(s) at end of substitution");
+                                     "Unrecognized character(s) at end of substitution.");
   }
   else
     throw context.stringtable.get (SUBST_MALFORMED,
-                                   "Malformed substitution");
+                                   "Malformed substitution.");
 }
 
 ////////////////////////////////////////////////////////////////////////////////

src/TDB.cpp

@@ -83,7 +83,7 @@ void readTaskmods (std::vector <std::string> &input,
       stream >> ts;
 
       if (stream.fail()) {
-        throw std::string ("Failed to convert \"" + stream.str() + "\" to integer: " + tmod_tmp.getTimeStr());
+        throw std::string ("Failed to convert \"" + stream.str() + "\" to integer: " + tmod_tmp.getTimeStr() + ".");
       }
 
       // 'time' is the first line of a modification
@@ -1452,7 +1452,7 @@ void TDB::merge (const std::string& mergeFile)
   }
   else // nothing to be done
   {
-    std::cout << "nothing to be done" << std::endl;
+    std::cout << "Nothing to be done." << std::endl;
   }
 
   // delete objects

src/Taskmod.cpp

@@ -117,7 +117,7 @@ bool Taskmod::isValid()
 std::string Taskmod::getUuid()
 {
   if (!bAfterSet) {
-    throw std::string("Taskmod::getUuid(): Task object not initialized");
+    throw std::string("Taskmod::getUuid(): Task object not initialized.");
   }
 
   return after.get("uuid");

src/command.cpp

@@ -133,7 +133,7 @@ int handleAdd (std::string &outs)
     context.tdb.add (context.task);
 
 #ifdef FEATURE_NEW_ID
-    out << "Created task " << context.tdb.nextId () << std::endl;
+    out << "Created task " << context.tdb.nextId () << "." << std::endl;
 #endif
 
     context.tdb.commit ();
@@ -715,7 +715,7 @@ int handleShow (std::string &outs)
     std::vector <std::string> unrecognized;
     foreach (i, all)
     {
-      // Disallow partial matches by tacking a leading an trailing space on each
+      // Disallow partial matches by tacking a leading and trailing space on each
       // variable name.
       std::string pattern = " " + *i + " ";
       if (recognized.find (pattern) == std::string::npos)
@@ -786,7 +786,7 @@ int handleShow (std::string &outs)
 
     out << std::endl
         << table.render ()
-        << (table.rowCount () == 0 ? "No matching configuration variables\n" : "")
+        << (table.rowCount () == 0 ? "No matching configuration variables.\n" : "")
         << std::endl;
 
     // Display the unrecognized variables.
@@ -897,7 +897,7 @@ int handleShow (std::string &outs)
 
     if (all.size () == 0)
     {
-      out << "Configuration error: .taskrc contains no entries"
+      out << "Configuration error: .taskrc contains no entries."
           << std::endl;
       rc = 1;
     }
@@ -975,7 +975,7 @@ int handleConfig (std::string &outs)
           {
             std::string::size_type eol = contents.find_first_of ("\r\f\n", pos);
             if (eol == std::string::npos)
-              throw std::string ("Cannot find EOL after entry '") + name + "'";
+              throw std::string ("Cannot find EOL after entry '") + name + "'.";
 
             if (confirm (std::string ("Are you sure you want to change the value of '")
                            + name
@@ -1011,11 +1011,11 @@ int handleConfig (std::string &outs)
           // Remove name
           std::string::size_type pos = contents.find (name + "=");
           if (pos == std::string::npos)
-            throw std::string ("No entry named '") + name + "' found";
+            throw std::string ("No entry named '") + name + "' found.";
 
           std::string::size_type eol = contents.find_first_of ("\r\f\n", pos);
           if (eol == std::string::npos)
-            throw std::string ("Cannot find EOL after entry '") + name + "'";
+            throw std::string ("Cannot find EOL after entry '") + name + "'.";
 
           if (confirm (std::string ("Are you sure you want to remove '") + name + "'?"))
           {
@@ -1112,7 +1112,7 @@ int handleDelete (std::string &outs)
                         << sibling->id
                         << " '"
                         << sibling->get ("description")
-                        << "'"
+                        << "'."
                         << std::endl;
                 }
               }
@@ -1134,7 +1134,7 @@ int handleDelete (std::string &outs)
                   << task->id
                   << " '"
                   << task->get ("description")
-                  << "'"
+                  << "'."
                   << std::endl;
             }
           }
@@ -1154,7 +1154,7 @@ int handleDelete (std::string &outs)
                   << task->id
                   << " '"
                   << task->get ("description")
-                  << "'"
+                  << "'."
                   << std::endl;
           }
         }
@@ -1212,7 +1212,7 @@ int handleStart (std::string &outs)
               << task->id
               << " '"
               << task->get ("description")
-              << "'"
+              << "'."
               << std::endl;
         if (!nagged)
           nagged = nag (*task);
@@ -1270,7 +1270,7 @@ int handleStop (std::string &outs)
               << task->id
               << " '"
               << task->get ("description")
-              << "'"
+              << "'."
               << std::endl;
       }
       else
@@ -1354,7 +1354,7 @@ int handleDone (std::string &outs)
                     << task->id
                     << " '"
                     << task->get ("description")
-                    << "'"
+                    << "'."
                     << std::endl;
 
               ++count;
@@ -1374,7 +1374,7 @@ int handleDone (std::string &outs)
             << task->id
             << " '"
             << task->get ("description")
-            << "' is not pending"
+            << "' is not pending."
             << std::endl;
         rc = 1;
     }
@@ -1389,7 +1389,7 @@ int handleDone (std::string &outs)
           << count
           << " task"
           << (count == 1 ? "" : "s")
-          << " as done"
+          << " as done."
           << std::endl;
 
     outs = out.str ();
@@ -1504,7 +1504,7 @@ int handleModify (std::string &outs)
   context.tdb.unlock ();
 
   if (context.config.getBoolean ("echo.command"))
-    out << "Modified " << count << " task" << (count == 1 ? "" : "s") << std::endl;
+    out << "Modified " << count << " task" << (count == 1 ? "." : "s.") << std::endl;
 
   outs = out.str ();
   return 0;
@@ -1563,6 +1563,7 @@ int handleAppend (std::string &outs)
                     << context.task.get ("description")
                     << "' to task "
                     << other->id
+                    << "."
                     << std::endl;
 
               ++count;
@@ -1576,7 +1577,7 @@ int handleAppend (std::string &outs)
     context.tdb.unlock ();
 
     if (context.config.getBoolean ("echo.command"))
-      out << "Appended " << count << " task" << (count == 1 ? "" : "s") << std::endl;
+      out << "Appended " << count << " task" << (count == 1 ? "." : "s.") << std::endl;
 
     outs = out.str ();
     context.hooks.trigger ("post-append-command");
@@ -1638,6 +1639,7 @@ int handlePrepend (std::string &outs)
                     << context.task.get ("description")
                     << "' to task "
                     << other->id
+                    << "."
                     << std::endl;
 
               ++count;
@@ -1651,7 +1653,7 @@ int handlePrepend (std::string &outs)
     context.tdb.unlock ();
 
     if (context.config.getBoolean ("echo.command"))
-      out << "Prepended " << count << " task" << (count == 1 ? "" : "s") << std::endl;
+      out << "Prepended " << count << " task" << (count == 1 ? "." : "s.") << std::endl;
 
     outs = out.str ();
     context.hooks.trigger ("post-prepend-command");
@@ -1719,20 +1721,20 @@ int handleDuplicate (std::string &outs)
             << task->id
             << " '"
             << task->get ("description")
-            << "'"
+            << "'."
             << std::endl;
       ++count;
     }
 
     if (context.config.getBoolean ("echo.command"))
     {
-      out << "Duplicated " << count << " task" << (count == 1 ? "" : "s") << std::endl;
+      out << "Duplicated " << count << " task" << (count == 1 ? "." : "s.") << std::endl;
 #ifdef FEATURE_NEW_ID
       // All this, just for an id number.
       std::vector <Task> all;
       Filter none;
       context.tdb.loadPending (all, none);
-      out << "Created task " << context.tdb.nextId () << std::endl;
+      out << "Created task " << context.tdb.nextId () << "." << std::endl;
 #endif
     }
 
@@ -1995,7 +1997,7 @@ int handleColor (std::string &outs)
 
         out << std::endl
             << std::endl
-            << "Try running 'task color white on red'"
+            << "Try running 'task color white on red'."
             << std::endl
             << std::endl;
       }
@@ -2058,7 +2060,7 @@ int handleAnnotate (std::string &outs)
                 << task->id
                 << " with '"
                 << context.task.get ("description")
-                << "'"
+                << "'."
                 << std::endl;
         }
       }

src/custom.cpp

@@ -778,7 +778,7 @@ void validReportColumns (const std::vector <std::string>& columns)
   {
     std::string error;
     join (error, ", ", bad);
-    throw std::string ("Unrecognized column name: ") + error;
+    throw std::string ("Unrecognized column name: ") + error + ".";
   }
 }
 
@@ -804,7 +804,7 @@ void validSortColumns (
   {
     std::string error;
     join (error, ", ", bad);
-    throw std::string ("Sort column is not part of the report: ") + error;
+    throw std::string ("Sort column is not part of the report: ") + error + ".";
   }
 }
 

src/edit.cpp

@@ -247,7 +247,7 @@ static void parseTask (Task& task, const std::string& after)
     }
   }
   else
-    throw std::string ("Cannot remove creation date");
+    throw std::string ("Cannot remove creation date.");
 
   // start
   value = findDate (after, "Started:");

src/recur.cpp

@@ -75,7 +75,7 @@ void handleRecurrence ()
                   << t->get ("uuid")
                   << " ("
                   << trim (t->get ("description"))
-                  << ") is past its 'until' date, and has been deleted"
+                  << ") has past its 'until' date, and has been deleted."
                   << std::endl;
 
         // Determine the end date.

src/tests/custom.columns.t

@@ -44,7 +44,7 @@ if (open my $fh, '>', 'custom.rc')
 
 # Generate the usage screen, and locate the custom report on it.
 my $output = qx{../task rc:custom.rc foo 2>&1};
-like ($output, qr/Unrecognized column name: foo\n/, 'custom report spotted invalid column');
+like ($output, qr/Unrecognized column name: foo\.\n/, 'custom report spotted invalid column');
 
 # Cleanup.
 unlink 'pending.data';

src/tests/shadow.t

@@ -43,16 +43,16 @@ if (open my $fh, '>', 'shadow.rc')
 }
 
 my $output = qx{../task rc:shadow.rc add one};
-like ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\]/, 'shadow file updated on add');
+like ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\.\]/, 'shadow file updated on add');
 
 $output = qx{../task rc:shadow.rc list};
-unlike ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\]/, 'shadow file not updated on list');
+unlike ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\.\]/, 'shadow file not updated on list');
 
 $output = qx{../task rc:shadow.rc delete 1};
-like ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\]/, 'shadow file updated on delete');
+like ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\.\]/, 'shadow file updated on delete');
 
 $output = qx{../task rc:shadow.rc list};
-unlike ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\]/, 'shadow file not updated on list');
+unlike ($output, qr/\[Shadow file '\.\/shadow\.txt' updated\.\]/, 'shadow file not updated on list');
 
 # Inspect the shadow file.
 my $file = slurp ('./shadow.txt');

src/util.cpp

@@ -368,14 +368,14 @@ std::string taskDifferences (const Task& before, const Task& after)
   foreach (name, beforeOnly)
     out << "  - "
         << *name
-        << " will be deleted\n";
+        << " will be deleted.\n";
 
   foreach (name, afterOnly)
     out << "  - "
         << *name
         << " will be set to '"
         << renderAttribute (*name, after.get (*name))
-        << "'\n";
+        << "'.\n";
 
   foreach (name, beforeAtts)
     if (*name              != "uuid" &&
@@ -386,11 +386,11 @@ std::string taskDifferences (const Task& before, const Task& after)
           << renderAttribute (*name, before.get (*name))
           << "' to '"
           << renderAttribute (*name, after.get (*name))
-          << "'\n";
+          << "'.\n";
 
   // Shouldn't just say nothing.
   if (out.str ().length () == 0)
-    out << "  - No changes will be made\n";
+    out << "  - No changes will be made.\n";
 
   return out.str ();
 }