Bug - #423

- The report.foo.filter line in a report definition accepts attributes
  as filters, but not rc overrides.
- Added unit tests.

ChangeLog

@@ -3,6 +3,7 @@
 
 1.9.3 ()
  + Improved man pages (thanks to Andy Lester).
+ + Fixed bug #423, and custom report filters now allow rc overrides.
 
 ------ old releases ------------------------------
 

src/Context.cpp

@@ -252,7 +252,8 @@ int Context::dispatch (std::string &out)
            sequence.size ())                  { rc = handleModify                (out); }
 
   // Commands that display IDs and therefore need TDB::gc first.
-  else if (cmd.validCustom (cmd.command))     { if (!inShadow) tdb.gc (); rc = handleCustomReport  (cmd.command, out); }
+  else if (cmd.validCustom (cmd.command))     { if (!inShadow) tdb.gc ();
+                                                rc = handleCustomReport (cmd.command, out); }
 
   // If the command is not recognized, display usage.
   else                                        { hooks.trigger ("pre-usage-command");
@@ -368,6 +369,44 @@ void Context::disallowModification () const
         + "' command does not allow further modification of a task.";
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// Takes a vector of args (foo, rc.name:value, bar), extracts any rc.name:value
+// args and sets the name/value in context.config, returning only the plain args
+// (foo, bar) as output.
+void Context::applyOverrides (
+  const std::vector <std::string>& input,
+  std::vector <std::string>& output)
+{
+  bool foundTerminator = false;
+  foreach (in, input)
+  {
+    if (*in == "--")
+    {
+      foundTerminator = true;
+      output.push_back (*in);
+    }
+    else if (!foundTerminator && in->substr (0, 3) == "rc.")
+    {
+      std::string name;
+      std::string value;
+      Nibbler n (*in);
+      if (n.getUntil ('.', name)       &&
+          n.skip ('.')                 &&
+          n.getUntilOneOf (":=", name) &&
+          n.skipN (1)                  &&
+          n.getUntilEOS (value))
+      {
+        config.set (name, value);
+        var_overrides += " " + *in;
+        footnote (std::string ("Configuration override ") +  // TODO i18n
+                  in->substr (3));
+      }
+    }
+    else
+      output.push_back (*in);
+  }
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 void Context::loadCorrectConfigFile ()
 {
@@ -418,8 +457,8 @@ void Context::loadCorrectConfigFile ()
   {
     if (*arg == "--")
       break;
-    else if (arg->substr (0, 17) == "rc.data.location:" ||
+    else if (arg->substr (0, 16) == "rc.data.location" &&
-             arg->substr (0, 17) == "rc.data.location=")
+             ((*arg)[16] == ':' || (*arg)[16] == '='))
     {
       data = Directory (arg->substr (17));
       header ("Using alternate data.location " + data.data); // TODO i18n
@@ -430,60 +469,23 @@ void Context::loadCorrectConfigFile ()
   // Do we need to create a default rc?
   if (! rc.exists ())
   {
-    if (confirm ("A configuration file could not be found in " // TODO i18n
+    if (!confirm ("A configuration file could not be found in " // TODO i18n
-               + home
+                + home
-               + "\n\n"
+                + "\n\n"
-               + "Would you like a sample "
+                + "Would you like a sample "
-               + rc.data
+                + rc.data
-               + " created, so task can proceed?"))
+                + " created, so task can proceed?"))
-    {
-      config.createDefaultRC (rc, data);
-    }
-    else
       throw std::string ("Cannot proceed without rc file.");
+
+    config.createDefaultRC (rc, data);
   }
 
   // Create data location, if necessary.
   config.createDefaultData (data);
 
-  // TODO find out why this was done twice - see tw #355
+  // Apply rc overrides.
-  // Load rc file.
-  //config.clear ();       // Dump current values.
-  //config.setDefaults (); // Add in the custom reports.
-  //config.load (rc);      // Load new file.
-
-  // Apply overrides of type: "rc.name:value", or "rc.name=value".
   std::vector <std::string> filtered;
-  bool foundTerminator = false;
+  applyOverrides (args, filtered);
-  foreach (arg, args)
-  {
-    if (*arg == "--")
-    {
-      foundTerminator = true;
-      filtered.push_back (*arg);
-    }
-    else if (!foundTerminator &&
-             arg->substr (0, 3) == "rc.")
-    {
-      std::string name;
-      std::string value;
-      Nibbler n (*arg);
-      if (n.getUntil ('.', name)       &&
-          n.skip ('.')                 &&
-          n.getUntilOneOf (":=", name) &&
-          n.skipN (1)                  &&
-          n.getUntilEOS (value))
-      {
-        config.set (name, value);
-        var_overrides += " " + *arg;
-        footnote (std::string ("Configuration override ") +  // TODO i18n
-                  arg->substr (3));
-      }
-    }
-    else
-      filtered.push_back (*arg);
-  }
-
   args = filtered;
 }
 

src/Context.h

@@ -68,6 +68,7 @@ public:
 
   std::string canonicalize (const std::string&) const;
   void disallowModification () const;
+  void applyOverrides (const std::vector <std::string>&, std::vector <std::string>&);
 
 private:
   void loadCorrectConfigFile ();

src/custom.cpp

@@ -54,62 +54,6 @@ static std::vector <std::string> customReports;
 // This report will eventually become the one report that many others morph into
 // via the .taskrc file.
 int handleCustomReport (const std::string& report, std::string &outs)
-{
-  // Load report configuration.
-  std::string columnList = context.config.get ("report." + report + ".columns");
-  std::string labelList  = context.config.get ("report." + report + ".labels");
-  std::string sortList   = context.config.get ("report." + report + ".sort");
-  std::string filterList = context.config.get ("report." + report + ".filter");
-
-  std::vector <std::string> filterArgs;
-  split (filterArgs, filterList, ' ');
-  {
-    Cmd cmd (report);
-    Task task;
-    Sequence sequence;
-    Subst subst;
-    Filter filter;
-    context.parse (filterArgs, cmd, task, sequence, subst, filter);
-
-    context.sequence.combine (sequence);
-
-    // Allow limit to be overridden by the command line.
-    if (!context.task.has ("limit") && task.has ("limit"))
-      context.task.set ("limit", task.get ("limit"));
-
-    foreach (att, filter)
-      context.filter.push_back (*att);
-  }
-
-  // Get all the tasks.
-  std::vector <Task> tasks;
-  context.tdb.lock (context.config.getBoolean ("locking"));
-  handleRecurrence ();
-  context.tdb.load (tasks, context.filter);
-  context.tdb.commit ();
-  context.tdb.unlock ();
-
-  return runCustomReport (
-    report,
-    columnList,
-    labelList,
-    sortList,
-    filterList,
-    tasks,
-    outs);
-}
-
-////////////////////////////////////////////////////////////////////////////////
-// This report will eventually become the one report that many others morph into
-// via the .taskrc file.
-int runCustomReport (
-  const std::string& report,
-  const std::string& columnList,
-  const std::string& labelList,
-  const std::string& sortList,
-  const std::string& filterList,
-  std::vector <Task>& tasks,
-  std::string &outs)
 {
   int rc = 0;
 
@@ -117,12 +61,17 @@ int runCustomReport (
       context.hooks.trigger (std::string ("pre-") + report + "-command"))
   {
     // Load report configuration.
+    std::string reportColumns = context.config.get ("report." + report + ".columns");
+    std::string reportLabels  = context.config.get ("report." + report + ".labels");
+    std::string reportSort    = context.config.get ("report." + report + ".sort");
+    std::string reportFilter  = context.config.get ("report." + report + ".filter");
+
     std::vector <std::string> columns;
-    split (columns, columnList, ',');
+    split (columns, reportColumns, ',');
     validReportColumns (columns);
 
     std::vector <std::string> labels;
-    split (labels, labelList, ',');
+    split (labels, reportLabels, ',');
 
     if (columns.size () != labels.size () && labels.size () != 0)
       throw std::string ("There are a different number of columns than labels ") +
@@ -134,22 +83,26 @@ int runCustomReport (
         columnLabels[columns[i]] = labels[i];
 
     std::vector <std::string> sortOrder;
-    split (sortOrder, sortList, ',');
+    split (sortOrder, reportSort, ',');
     validSortColumns (columns, sortOrder);
 
+    // Apply rc overrides.
     std::vector <std::string> filterArgs;
-    split (filterArgs, filterList, ' ');
+    std::vector <std::string> filteredArgs;
+    split (filterArgs, reportFilter, ' ');
+    context.applyOverrides (filterArgs, filteredArgs);
+
     {
       Cmd cmd (report);
       Task task;
       Sequence sequence;
       Subst subst;
       Filter filter;
-      context.parse (filterArgs, cmd, task, sequence, subst, filter);
+      context.parse (filteredArgs, cmd, task, sequence, subst, filter);
 
       context.sequence.combine (sequence);
 
-      // Allow limit to be overridden by the command line.
+      // Special case: Allow limit to be overridden by the command line.
       if (!context.task.has ("limit") && task.has ("limit"))
         context.task.set ("limit", task.get ("limit"));
 
@@ -157,6 +110,14 @@ int runCustomReport (
         context.filter.push_back (*att);
     }
 
+    // Get all the tasks.
+    std::vector <Task> tasks;
+    context.tdb.lock (context.config.getBoolean ("locking"));
+    handleRecurrence ();
+    context.tdb.load (tasks, context.filter);
+    context.tdb.commit ();
+    context.tdb.unlock ();
+
     // Filter sequence.
     if (context.sequence.size ())
       context.filter.applySequence (tasks, context.sequence);

src/main.h

@@ -111,10 +111,6 @@ std::string getDueDate (Task&, const std::string&);
 
 // custom.cpp
 int handleCustomReport (const std::string&, std::string &);
-int runCustomReport (const std::string&, const std::string&,
-                     const std::string&, const std::string&,
-                     const std::string&, std::vector <Task>&,
-		     std::string&);
 void validReportColumns (const std::vector <std::string>&);
 void validSortColumns (const std::vector <std::string>&, const std::vector <std::string>&);
 void getLimits (const std::string&, int&, int&);

src/tests/override.t

@@ -0,0 +1,64 @@
+#! /usr/bin/perl
+################################################################################
+## task - a command line task list manager.
+##
+## Copyright 2006 - 2010, Paul Beckingham.
+## All rights reserved.
+##
+## 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.
+##
+## This program 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 General Public License for more
+## details.
+##
+## You should have received a copy of the GNU General Public License along with
+## this program; if not, write to the
+##
+##     Free Software Foundation, Inc.,
+##     51 Franklin Street, Fifth Floor,
+##     Boston, MA
+##     02110-1301
+##     USA
+##
+################################################################################
+
+use strict;
+use warnings;
+use Test::More tests => 5;
+
+# Create the rc file.
+if (open my $fh, '>', 'or.rc')
+{
+  print $fh "data.location=.\n",
+            "annotations=none\n",
+            "report.zzz.columns=id,due,description\n",
+            "report.zzz.labels=ID,Due,Description\n",
+            "report.zzz.sort=due+\n",
+            "report.zzz.filter=status:pending rc.annotations:full\n";
+  close $fh;
+  ok (-r 'or.rc', 'Created or.rc');
+}
+
+# The zzz report is defined with an override in the filter that contradicts
+# the value in the rc.  The filter override should prevail.
+qx{../task rc:or.rc add ONE};
+qx{../task rc:or.rc 1 annotate TWO};
+my $output = qx{../task rc:or.rc zzz};
+like ($output, qr/ONE.+TWO/ms, 'filter override > rc setting');
+
+# Cleanup.
+unlink 'pending.data';
+ok (!-r 'pending.data', 'Removed pending.data');
+
+unlink 'undo.data';
+ok (!-r 'undo.data', 'Removed undo.data');
+
+unlink 'or.rc';
+ok (!-r 'or.rc', 'Removed or.rc');
+
+exit 0;
+