From b29f9969e5d38e88df93d2b5ebbfdbae0beafc56 Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Sun, 11 Jul 2010 14:03:15 -0400 Subject: [PATCH] Bug - #423 - The report.foo.filter line in a report definition accepts attributes as filters, but not rc overrides. - Added unit tests. --- ChangeLog | 1 + src/Context.cpp | 102 ++++++++++++++++++++++--------------------- src/Context.h | 1 + src/custom.cpp | 85 ++++++++++-------------------------- src/main.h | 4 -- src/tests/override.t | 64 +++++++++++++++++++++++++++ 6 files changed, 141 insertions(+), 116 deletions(-) create mode 100755 src/tests/override.t diff --git a/ChangeLog b/ChangeLog index 8680ef3e8..e6c633505 100644 --- a/ChangeLog +++ b/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 ------------------------------ diff --git a/src/Context.cpp b/src/Context.cpp index ff4f6dc27..6b797828a 100644 --- a/src/Context.cpp +++ b/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 & input, + std::vector & 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:" || - arg->substr (0, 17) == "rc.data.location=") + else if (arg->substr (0, 16) == "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 - + home - + "\n\n" - + "Would you like a sample " - + rc.data - + " created, so task can proceed?")) - { - config.createDefaultRC (rc, data); - } - else + if (!confirm ("A configuration file could not be found in " // TODO i18n + + home + + "\n\n" + + "Would you like a sample " + + rc.data + + " created, so task can proceed?")) 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 - // 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". + // Apply rc overrides. std::vector filtered; - bool foundTerminator = false; - 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); - } - + applyOverrides (args, filtered); args = filtered; } diff --git a/src/Context.h b/src/Context.h index f4a29e3f2..7b51ae865 100644 --- a/src/Context.h +++ b/src/Context.h @@ -68,6 +68,7 @@ public: std::string canonicalize (const std::string&) const; void disallowModification () const; + void applyOverrides (const std::vector &, std::vector &); private: void loadCorrectConfigFile (); diff --git a/src/custom.cpp b/src/custom.cpp index 90853e00d..929d3b492 100644 --- a/src/custom.cpp +++ b/src/custom.cpp @@ -54,62 +54,6 @@ static std::vector 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 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 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 & 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 columns; - split (columns, columnList, ','); + split (columns, reportColumns, ','); validReportColumns (columns); std::vector 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 sortOrder; - split (sortOrder, sortList, ','); + split (sortOrder, reportSort, ','); validSortColumns (columns, sortOrder); + // Apply rc overrides. std::vector filterArgs; - split (filterArgs, filterList, ' '); + std::vector 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 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); diff --git a/src/main.h b/src/main.h index 8de612b09..bd5b89aab 100644 --- a/src/main.h +++ b/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 &, - std::string&); void validReportColumns (const std::vector &); void validSortColumns (const std::vector &, const std::vector &); void getLimits (const std::string&, int&, int&); diff --git a/src/tests/override.t b/src/tests/override.t new file mode 100755 index 000000000..e60d84da6 --- /dev/null +++ b/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; +