From 38ffa390eadbacc355eb7d0d6bca7aff1a5a1f5e Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Fri, 26 Nov 2010 14:00:49 -0500 Subject: [PATCH] Bug #542 - Fixed bug #542, which sorted the countdown columns incorrectly (thanks to Michelle Crane). - Duration could not parse all varieties of it's own output, which is why the sorting was broken, in particular negative durations and real numbers in durations (such as (1.2y'). - Sorting in Table::sort_compare for Table::periodAscending and Table::periodDescending was broken. - Missing unit tests for countdown_compact. - Imported Nibbler::getNumber from the Attitash project. - Additional Duration unit tests. - Additional Nibbler unit tests. - Additional countdown.t unit tests. --- ChangeLog | 2 + src/Duration.cpp | 106 ++++++++++++------------ src/Nibbler.cpp | 72 +++++++++++++++++ src/Nibbler.h | 1 + src/Table.cpp | 8 +- src/tests/countdown.t | 168 ++++++++++++++++++++++++++++++--------- src/tests/duration.t.cpp | 43 ++++++++-- src/tests/nibbler.t.cpp | 13 ++- 8 files changed, 311 insertions(+), 102 deletions(-) diff --git a/ChangeLog b/ChangeLog index a84d17105..a868efc7e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,8 @@ + Fixed bug #538, where some color legend items were not readable. + Fixed bug #539, where the man page task-color(5) contained a line that began with a ' and was not displayed. + + Fixed bug #542, which sorted the countdown columns incorrectly (thanks to + Michelle Crane). + Fixed bug #555, which caused a segfault when logging a task with a project (thanks to Itay Perl). diff --git a/src/Duration.cpp b/src/Duration.cpp index 1e5f2aa4b..0e74b8dc9 100644 --- a/src/Duration.cpp +++ b/src/Duration.cpp @@ -1,7 +1,7 @@ //////////////////////////////////////////////////////////////////////////////// // taskwarrior - a command line task list manager. // -// Copyright 2006 - 2010, Paul Beckingham. +// Copyright 2006 - 2010, Paul Beckingham, Federico Hernandez. // All rights reserved. // // This program is free software; you can redistribute it and/or modify it under @@ -242,13 +242,13 @@ bool Duration::valid (const std::string& input) const std::string lower_input = lowerCase (input); // Assume the ordinal is 1, but look for an integer, just in case. - int value = 1; + double value = 1; Nibbler n (lower_input); n.skipAll (' '); - n.getInt (value); + n.getNumber (value); n.skipAll (' '); - if (value < 0) + if (value < 0.0) value = -value; std::string units; @@ -272,14 +272,14 @@ void Duration::parse (const std::string& input) std::string lower_input = lowerCase (input); // Assume the ordinal is 1, but look for an integer, just in case. - int value = 1; + double value = 1; Nibbler n (lower_input); n.skipAll (' '); - n.getInt (value); + n.getNumber (value); n.skipAll (' '); - if (value < 0) + if (value < 0.0) { mNegative = true; value = -value; @@ -301,62 +301,62 @@ void Duration::parse (const std::string& input) { std::string match = matches[0]; - if (match == "biannual") mSecs = value * 86400 * 730; - else if (match == "biyearly") mSecs = value * 86400 * 730; + if (match == "biannual") mSecs = (int) (value * 86400 * 730); + else if (match == "biyearly") mSecs = (int) (value * 86400 * 730); - else if (match == "yearly") mSecs = value * 86400 * 365; - else if (match == "annual") mSecs = value * 86400 * 365; - else if (match == "years") mSecs = value * 86400 * 365; - else if (match == "yrs") mSecs = value * 86400 * 365; - else if (match == "y") mSecs = value * 86400 * 365; - else if (match == "yearly") mSecs = value * 86400 * 365; - else if (match == "annual") mSecs = value * 86400 * 365; + else if (match == "yearly") mSecs = (int) (value * 86400 * 365); + else if (match == "annual") mSecs = (int) (value * 86400 * 365); + else if (match == "years") mSecs = (int) (value * 86400 * 365); + else if (match == "yrs") mSecs = (int) (value * 86400 * 365); + else if (match == "y") mSecs = (int) (value * 86400 * 365); + else if (match == "yearly") mSecs = (int) (value * 86400 * 365); + else if (match == "annual") mSecs = (int) (value * 86400 * 365); - else if (match == "semiannual") mSecs = value * 86400 * 183; + else if (match == "semiannual") mSecs = (int) (value * 86400 * 183); - else if (match == "bimonthly") mSecs = value * 86400 * 61; - else if (match == "quarterly") mSecs = value * 86400 * 91; - else if (match == "quarters") mSecs = value * 86400 * 91; - else if (match == "qrtrs") mSecs = value * 86400 * 91; - else if (match == "qtrs") mSecs = value * 86400 * 91; - else if (match == "q") mSecs = value * 86400 * 91; + else if (match == "bimonthly") mSecs = (int) (value * 86400 * 61); + else if (match == "quarterly") mSecs = (int) (value * 86400 * 91); + else if (match == "quarters") mSecs = (int) (value * 86400 * 91); + else if (match == "qrtrs") mSecs = (int) (value * 86400 * 91); + else if (match == "qtrs") mSecs = (int) (value * 86400 * 91); + else if (match == "q") mSecs = (int) (value * 86400 * 91); - else if (match == "monthly") mSecs = value * 86400 * 30; - else if (match == "month") mSecs = value * 86400 * 30; - else if (match == "months") mSecs = value * 86400 * 30; - else if (match == "mnths") mSecs = value * 86400 * 30; - else if (match == "mos") mSecs = value * 86400 * 30; - else if (match == "mo") mSecs = value * 86400 * 30; - else if (match == "mths") mSecs = value * 86400 * 30; + else if (match == "monthly") mSecs = (int) (value * 86400 * 30); + else if (match == "month") mSecs = (int) (value * 86400 * 30); + else if (match == "months") mSecs = (int) (value * 86400 * 30); + else if (match == "mnths") mSecs = (int) (value * 86400 * 30); + else if (match == "mos") mSecs = (int) (value * 86400 * 30); + else if (match == "mo") mSecs = (int) (value * 86400 * 30); + else if (match == "mths") mSecs = (int) (value * 86400 * 30); - else if (match == "biweekly") mSecs = value * 86400 * 14; - else if (match == "fortnight") mSecs = value * 86400 * 14; + else if (match == "biweekly") mSecs = (int) (value * 86400 * 14); + else if (match == "fortnight") mSecs = (int) (value * 86400 * 14); - else if (match == "weekly") mSecs = value * 86400 * 7; - else if (match == "sennight") mSecs = value * 86400 * 7; - else if (match == "weeks") mSecs = value * 86400 * 7; - else if (match == "wks") mSecs = value * 86400 * 7; - else if (match == "w") mSecs = value * 86400 * 7; + else if (match == "weekly") mSecs = (int) (value * 86400 * 7); + else if (match == "sennight") mSecs = (int) (value * 86400 * 7); + else if (match == "weeks") mSecs = (int) (value * 86400 * 7); + else if (match == "wks") mSecs = (int) (value * 86400 * 7); + else if (match == "w") mSecs = (int) (value * 86400 * 7); - else if (match == "daily") mSecs = value * 86400 * 1; - else if (match == "day") mSecs = value * 86400 * 1; - else if (match == "weekdays") mSecs = value * 86400 * 1; - else if (match == "days") mSecs = value * 86400 * 1; - else if (match == "d") mSecs = value * 86400 * 1; + else if (match == "daily") mSecs = (int) (value * 86400 * 1); + else if (match == "day") mSecs = (int) (value * 86400 * 1); + else if (match == "weekdays") mSecs = (int) (value * 86400 * 1); + else if (match == "days") mSecs = (int) (value * 86400 * 1); + else if (match == "d") mSecs = (int) (value * 86400 * 1); - else if (match == "hours") mSecs = value * 3600; - else if (match == "hrs") mSecs = value * 3600; - else if (match == "h") mSecs = value * 3600; + else if (match == "hours") mSecs = (int) (value * 3600); + else if (match == "hrs") mSecs = (int) (value * 3600); + else if (match == "h") mSecs = (int) (value * 3600); - else if (match == "minutes") mSecs = value * 60; - else if (match == "mins") mSecs = value * 60; - else if (match == "min") mSecs = value * 60; - else if (match == "m") mSecs = value * 60; + else if (match == "minutes") mSecs = (int) (value * 60); + else if (match == "mins") mSecs = (int) (value * 60); + else if (match == "min") mSecs = (int) (value * 60); + else if (match == "m") mSecs = (int) (value * 60); - else if (match == "seconds") mSecs = value; - else if (match == "secs") mSecs = value; - else if (match == "sec") mSecs = value; - else if (match == "s") mSecs = value; + else if (match == "seconds") mSecs = (int) value; + else if (match == "secs") mSecs = (int) value; + else if (match == "sec") mSecs = (int) value; + else if (match == "s") mSecs = (int) value; else if (match == "-") mSecs = 0; } diff --git a/src/Nibbler.cpp b/src/Nibbler.cpp index 286d3f591..4514e59d0 100644 --- a/src/Nibbler.cpp +++ b/src/Nibbler.cpp @@ -323,6 +323,78 @@ bool Nibbler::getUnsignedInt (int& result) return false; } +//////////////////////////////////////////////////////////////////////////////// +// number: +// int frac? exp? +// +// int: +// -? digit+ +// +// frac: +// . digit+ +// +// exp: +// e digit+ +// +// e: +// e|E (+|-)? +// +bool Nibbler::getNumber (double& result) +{ + std::string::size_type i = mCursor; + + // [+-]? + if (i < mLength && mInput[i] == '-') + ++i; + + // digit+ + if (i < mLength && isdigit (mInput[i])) + { + ++i; + + while (i < mLength && isdigit (mInput[i])) + ++i; + + // ( . digit+ )? + if (i < mLength && mInput[i] == '.') + { + ++i; + + while (i < mLength && isdigit (mInput[i])) + ++i; + } + + // ( [eE] [+-]? digit+ )? + if (i < mLength && (mInput[i] == 'e' || mInput[i] == 'E')) + { + ++i; + + if (i < mLength && (mInput[i] == '+' || mInput[i] == '-')) + ++i; + + if (i < mLength && isdigit (mInput[i])) + { + ++i; + + while (i < mLength && isdigit (mInput[i])) + ++i; + + result = atof (mInput.substr (mCursor, i - mCursor).c_str ()); + mCursor = i; + return true; + } + + return false; + } + + result = atof (mInput.substr (mCursor, i - mCursor).c_str ()); + mCursor = i; + return true; + } + + return false; +} + //////////////////////////////////////////////////////////////////////////////// bool Nibbler::getLiteral (const std::string& literal) { diff --git a/src/Nibbler.h b/src/Nibbler.h index 0b5651f11..7c69348ef 100644 --- a/src/Nibbler.h +++ b/src/Nibbler.h @@ -50,6 +50,7 @@ public: bool getQuoted (char, std::string&, bool unescape = true, bool quote = false); bool getInt (int&); bool getUnsignedInt (int&); + bool getNumber (double&); bool getLiteral (const std::string&); bool getRx (const std::string&, std::string&); diff --git a/src/Table.cpp b/src/Table.cpp index e5ea2c070..aae0d0d6c 100644 --- a/src/Table.cpp +++ b/src/Table.cpp @@ -763,8 +763,8 @@ bool sort_compare (int left, int right) return true; else if ((std::string)*cell_left != "" && (std::string)*cell_right == "") return false; - else if (Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right)) - return true; + else + return Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right) ? true : false; break; case Table::descendingPeriod: @@ -772,8 +772,8 @@ bool sort_compare (int left, int right) return false; else if ((std::string)*cell_left == "" && (std::string)*cell_right != "") return true; - else if (Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right)) - return false; + else + return Duration ((std::string)*cell_left) < Duration ((std::string)*cell_right) ? false : true; break; } } diff --git a/src/tests/countdown.t b/src/tests/countdown.t index afffc2582..a98d79600 100755 --- a/src/tests/countdown.t +++ b/src/tests/countdown.t @@ -2,7 +2,7 @@ ################################################################################ ## taskwarrior - a command line task list manager. ## -## Copyright 2006 - 2010, Paul Beckingham. +## Copyright 2006 - 2010, Paul Beckingham, Federico Hernandez. ## All rights reserved. ## ## This program is free software; you can redistribute it and/or modify it under @@ -28,57 +28,151 @@ use strict; use warnings; -use Test::More tests => 12; +use Test::More tests => 85; # Create the rc file. if (open my $fh, '>', 'countdown.rc') { print $fh "data.location=.\n"; - print $fh "report.countdown.description=Countdown report\n"; - print $fh "report.countdown.columns=id,countdown,description\n"; - print $fh "report.countdown.labels=ID,Countdown,Description\n"; - print $fh "report.countdown.sort=countdown-"; + print $fh "report.up.description=countdown+ report\n"; + print $fh "report.up.columns=id,countdown,description\n"; + print $fh "report.up.labels=ID,Countdown,Description\n"; + print $fh "report.up.filter=status:pending\n"; + print $fh "report.up.sort=countdown+\n"; + + print $fh "report.down.description=countdown- report\n"; + print $fh "report.down.columns=id,countdown,description\n"; + print $fh "report.down.labels=ID,Countdown,Description\n"; + print $fh "report.down.filter=status:pending\n"; + print $fh "report.down.sort=countdown-\n"; + + print $fh "report.upc.description=countdown_compact+ report\n"; + print $fh "report.upc.columns=id,countdown_compact,description\n"; + print $fh "report.upc.labels=ID,Countdown,Description\n"; + print $fh "report.upc.filter=status:pending\n"; + print $fh "report.upc.sort=countdown_compact+\n"; + + print $fh "report.downc.description=countdown_compact- report\n"; + print $fh "report.downc.columns=id,countdown_compact,description\n"; + print $fh "report.downc.labels=ID,Countdown,Description\n"; + print $fh "report.downc.filter=status:pending\n"; + print $fh "report.downc.sort=countdown_compact-\n"; close $fh; ok (-r 'countdown.rc', 'Created countdown.rc'); } # Create a variety of pending tasks with increasingly higher due dates -# and ensure sort order. +# to ensure proper sort order. +qx{../task rc:countdown.rc add one due:-1.2y}; +qx{../task rc:countdown.rc add two due:-9mo}; +qx{../task rc:countdown.rc add three due:-2mo}; +qx{../task rc:countdown.rc add four due:-3wk}; +qx{../task rc:countdown.rc add five due:-7d}; +qx{../task rc:countdown.rc add six due:-2d}; +qx{../task rc:countdown.rc add seven due:-1d}; +qx{../task rc:countdown.rc add eight due:-12h}; +qx{../task rc:countdown.rc add nine due:-6h}; +qx{../task rc:countdown.rc add ten due:-1h}; +qx{../task rc:countdown.rc add eleven due:-30s}; +qx{../task rc:countdown.rc add twelve due:1h}; +qx{../task rc:countdown.rc add thirteen due:6h}; +qx{../task rc:countdown.rc add fourteen due:12h}; +qx{../task rc:countdown.rc add fifteen due:1d}; +qx{../task rc:countdown.rc add sixteen due:2d}; +qx{../task rc:countdown.rc add seventeen due:7d}; +qx{../task rc:countdown.rc add eighteen due:3wk}; +qx{../task rc:countdown.rc add nineteen due:2mo}; +qx{../task rc:countdown.rc add twenty due:9mo}; +qx{../task rc:countdown.rc add twentyone due:1.2y}; -# The -1 guarantees that no duration will be rendered as '-'. -my $now = time () - 1; +my $output = qx{../task rc:countdown.rc up}; +like ($output, qr/\bone\b.+\btwo\b/ms, 'up: one < two'); +like ($output, qr/\btwo\b.+\bthree/ms, 'up: two < three'); +like ($output, qr/\bthree\b.+\bfour/ms, 'up: three < four'); +like ($output, qr/\bfour\b.+\bfive/ms, 'up: four < five'); +like ($output, qr/\bfive\b.+\bsix/ms, 'up: five < six'); +like ($output, qr/\bsix\b.+\bseven/ms, 'up: six < seven'); +like ($output, qr/\bseven\b.+\beight/ms, 'up: seven < eight'); +like ($output, qr/\beight\b.+\bnine/ms, 'up: eight < nine'); +like ($output, qr/\bnine\b.+\bten/ms, 'up: nine < ten'); +like ($output, qr/\bten\b.+\beleven/ms, 'up: ten < eleven'); +like ($output, qr/\beleven\b.+\btwelve/ms, 'up: eleven < twelve'); +like ($output, qr/\btwelve\b.+\bthirteen/ms, 'up: twelve < thirteen'); +like ($output, qr/\bthirteen\b.+\bfourteen/ms, 'up: thirteen < fourteen'); +like ($output, qr/\bfourteen\b.+\bfifteen/ms, 'up: fourteen < fifteen'); +like ($output, qr/\bfifteen\b.+\bsixteen/ms, 'up: fifteen < sixteen'); +like ($output, qr/\bsixteen\b.+\bseventeen/ms, 'up: sixteen < seventeen'); +like ($output, qr/\bseventeen\b.+\beighteen/ms, 'up: seventeen < eighteen'); +like ($output, qr/\beighteen\b.+\bnineteen/ms, 'up: eighteen < nineteen'); +like ($output, qr/\bnineteen\b.+\btwenty/ms, 'up: nineteen < twenty'); +like ($output, qr/\btwenty\b.+\btwentyone/ms, 'up: twenty < twentyone'); -my $nowminusone = $now - 3600; -my $nowplusone = $now + 3600; -my $nowplusseven = $now + 7 * 3600; -my $nowplustwelve = $now + 12 * 3600; -my $nowplusthirtysix = $now + 36 * 3600; -my $nowplusseventytwo = $now + 72 * 3600; +$output = qx{../task rc:countdown.rc down}; +like ($output, qr/\btwentyone\b.+\btwenty/ms, 'down: twentyone < twenty'); +like ($output, qr/\btwenty\b.+\bnineteen/ms, 'down: twenty < nineteen'); +like ($output, qr/\bnineteen\b.+\beighteen/ms, 'down: nineteen < eighteen'); +like ($output, qr/\beighteen\b.+\bseventeen/ms, 'down: eighteen < seventeen'); +like ($output, qr/\bseventeen\b.+\bsixteen/ms, 'down: seventeen < sixteen'); +like ($output, qr/\bsixteen\b.+\bfifteen/ms, 'down: sixteen < fifteen'); +like ($output, qr/\bfifteen\b.+\bfourteen/ms, 'down: fifteen < fourteen'); +like ($output, qr/\bfourteen\b.+\bthirteen/ms, 'down: fourteen < thirteen'); +like ($output, qr/\bthirteen\b.+\btwelve/ms, 'down: thirteen < twelve'); +like ($output, qr/\btwelve\b.+\beleven/ms, 'down: twelve < eleven'); +like ($output, qr/\beleven\b.+\bten/ms, 'down: eleven < ten'); +like ($output, qr/\bten\b.+\bnine/ms, 'down: ten < nine'); +like ($output, qr/\bnine\b.+\beight/ms, 'down: nine < eight'); +like ($output, qr/\beight\b.+\bseven/ms, 'down: eight < seven'); +like ($output, qr/\bseven\b.+\bsix/ms, 'down: seven < six'); +like ($output, qr/\bsix\b.+\bfive/ms, 'down: six < five'); +like ($output, qr/\bfive\b.+\bfour/ms, 'down: five < four'); +like ($output, qr/\bfour\b.+\bthree/ms, 'down: four < three'); +like ($output, qr/\bthree\b.+\btwo/ms, 'down: three < two'); +like ($output, qr/\btwo\b.+\bone\b/ms, 'down: two < one'); -if (open my $fh, '>', 'pending.data') -{ - print $fh < left = Duration ("2secs"); right = Duration ("1sec"); t.ok (left > right, "2sec > 1secs"); left = Duration ("-1sec"); right = Duration ("-2secs"); t.ok (left > right, "-1secs > -2sec"); @@ -695,6 +722,8 @@ int main (int argc, char** argv) left = Duration ("1mo"); right = Duration ("1w"); t.ok (left > right, "1mo > 1w"); left = Duration ("1q"); right = Duration ("1mo"); t.ok (left > right, "1q > 1mo"); left = Duration ("1y"); right = Duration ("1q"); t.ok (left > right, "1y > 1q"); + + left = Duration ("-3s"); right = Duration ("-6s"); t.ok (left > right, "duration -3s > -6s"); } catch (const std::string& e) { t.diag (e); } diff --git a/src/tests/nibbler.t.cpp b/src/tests/nibbler.t.cpp index 26b83da5e..37443c3de 100644 --- a/src/tests/nibbler.t.cpp +++ b/src/tests/nibbler.t.cpp @@ -33,13 +33,14 @@ Context context; //////////////////////////////////////////////////////////////////////////////// int main (int argc, char** argv) { - UnitTest t (149); + UnitTest t (155); try { Nibbler n; std::string s; int i; + double d; // Make sure the nibbler behaves itself with trivial input. t.diag ("Test all nibbler calls given empty input"); @@ -228,6 +229,16 @@ int main (int argc, char** argv) t.is (i, 4, " '4' : getUnsignedInt () -> '4'"); t.ok (n.depleted (), " '' : depleted () -> true"); + // bool getNumber (double&); + t.diag ("Nibbler::getNumber"); + n = Nibbler ("-1.234 2.3e4"); + t.ok (n.getNumber (d), "'-1.234 2.3e4' : getNumber () -> true"); + t.is (d, -1.234, "'-1.234 2.3e4' : getNumber () -> '-1.234'"); + t.ok (n.skip (' '), " ' 2.3e4' : skip (' ') -> true"); + t.ok (n.getNumber (d), " '2.3e4' : getNumber () -> true"); + t.is (d, 2.3e4, " '2.3e4' : getNumber () -> '2.3e4'"); + t.ok (n.depleted (), " '' : depleted () -> true"); + // bool getLiteral (const std::string&); t.diag ("Nibbler::getLiteral"); n = Nibbler ("foobar");