From 656e3502916727b1eb6bc1e989dc93964c5d1b05 Mon Sep 17 00:00:00 2001 From: Paul Beckingham Date: Mon, 1 Apr 2013 19:49:27 -0400 Subject: [PATCH] Date Formatting - Some bad inefficiencies in date formatting were noticed, and when addressed, caused a bug to surface. The length of a formatted date can be calculated from the dateformat, but was done incorrectly. Very, very incorrectly. - Added unit tests. - Promoted date column-specific "countdown" size measurements up to the ColDate base class. This neatly falls out from work on #1218. - Noted a potential I18N problem in Date.cpp. --- src/Date.cpp | 61 ++++++++++++++++++++---------------- src/columns/ColDate.cpp | 20 ++++++++++-- src/columns/ColDue.cpp | 43 ------------------------- src/columns/ColDue.h | 2 -- src/columns/ColScheduled.cpp | 43 ------------------------- src/columns/ColScheduled.h | 2 -- src/columns/ColUDA.cpp | 4 +-- test/date.t.cpp | 26 ++++++++++++++- 8 files changed, 79 insertions(+), 122 deletions(-) diff --git a/src/Date.cpp b/src/Date.cpp index 91fdbbf2c..8873ad7c9 100644 --- a/src/Date.cpp +++ b/src/Date.cpp @@ -230,27 +230,27 @@ const std::string Date::toString ( int c = localFormat[i]; switch (c) { - case 'm': sprintf (buffer, "%d", this->month ()); break; - case 'M': sprintf (buffer, "%02d", this->month ()); break; - case 'd': sprintf (buffer, "%d", this->day ()); break; - case 'D': sprintf (buffer, "%02d", this->day ()); break; - case 'y': sprintf (buffer, "%02d", this->year () % 100); break; - case 'Y': sprintf (buffer, "%d", this->year ()); break; - case 'a': sprintf (buffer, "%.3s", Date::dayName (dayOfWeek ()).c_str ()); break; - case 'A': sprintf (buffer, "%s", Date::dayName (dayOfWeek ()).c_str ()); break; - case 'b': sprintf (buffer, "%.3s", Date::monthName (month ()).c_str ()); break; - case 'B': sprintf (buffer, "%.9s", Date::monthName (month ()).c_str ()); break; - case 'v': sprintf (buffer, "%d", Date::weekOfYear (Date::dayOfWeek (context.config.get ("weekstart")))); break; - case 'V': sprintf (buffer, "%02d", Date::weekOfYear (Date::dayOfWeek (context.config.get ("weekstart")))); break; - case 'h': sprintf (buffer, "%d", this->hour ()); break; - case 'H': sprintf (buffer, "%02d", this->hour ()); break; - case 'n': sprintf (buffer, "%d", this->minute ()); break; - case 'N': sprintf (buffer, "%02d", this->minute ()); break; - case 's': sprintf (buffer, "%d", this->second ()); break; - case 'S': sprintf (buffer, "%02d", this->second ()); break; - case 'j': sprintf (buffer, "%d", this->dayOfYear ()); break; - case 'J': sprintf (buffer, "%03d", this->dayOfYear ()); break; - default: sprintf (buffer, "%c", c); break; + case 'm': sprintf (buffer, "%d", this->month ()); break; + case 'M': sprintf (buffer, "%02d", this->month ()); break; + case 'd': sprintf (buffer, "%d", this->day ()); break; + case 'D': sprintf (buffer, "%02d", this->day ()); break; + case 'y': sprintf (buffer, "%02d", this->year () % 100); break; + case 'Y': sprintf (buffer, "%d", this->year ()); break; + case 'a': sprintf (buffer, "%.3s", Date::dayName (dayOfWeek ()).c_str ()); break; + case 'A': sprintf (buffer, "%.10s", Date::dayName (dayOfWeek ()).c_str ()); break; + case 'b': sprintf (buffer, "%.3s", Date::monthName (month ()).c_str ()); break; + case 'B': sprintf (buffer, "%.10s", Date::monthName (month ()).c_str ()); break; + case 'v': sprintf (buffer, "%d", Date::weekOfYear (Date::dayOfWeek (context.config.get ("weekstart")))); break; + case 'V': sprintf (buffer, "%02d", Date::weekOfYear (Date::dayOfWeek (context.config.get ("weekstart")))); break; + case 'h': sprintf (buffer, "%d", this->hour ()); break; + case 'H': sprintf (buffer, "%02d", this->hour ()); break; + case 'n': sprintf (buffer, "%d", this->minute ()); break; + case 'N': sprintf (buffer, "%02d", this->minute ()); break; + case 's': sprintf (buffer, "%d", this->second ()); break; + case 'S': sprintf (buffer, "%02d", this->second ()); break; + case 'j': sprintf (buffer, "%d", this->dayOfYear ()); break; + case 'J': sprintf (buffer, "%03d", this->dayOfYear ()); break; + default: sprintf (buffer, "%c", c); break; } formatted += buffer; @@ -527,16 +527,23 @@ int Date::length (const std::string& format) case 'd': case 'D': case 'y': - case 'A': - case 'b': - case 'B': + case 'v': case 'V': case 'h': case 'H': + case 'n': case 'N': - case 'S': total += 2; break; - case 'a': total += 3; break; - case 'Y': total += 4; break; + case 's': + case 'S': total += 2; break; + case 'b': + case 'j': + case 'J': + case 'a': total += 3; break; + case 'Y': total += 4; break; + case 'A': + case 'B': total += 10; break; + + // TODO This should be a calculated character width, not necessarily 1. default: total += 1; break; } } diff --git a/src/columns/ColDate.cpp b/src/columns/ColDate.cpp index 6d54c5053..1d502b4c6 100644 --- a/src/columns/ColDate.cpp +++ b/src/columns/ColDate.cpp @@ -95,7 +95,13 @@ void ColumnDate::measure (Task& task, unsigned int& minimum, unsigned int& maxim if (format == "") format = context.config.get ("dateformat"); - minimum = maximum = date.toString (format).length (); + minimum = maximum = Date::length (format); + } + else if (_style == "countdown") + { + Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); + Date now; + minimum = maximum = Duration (now - date).format ().length (); } else if (_style == "julian") { @@ -134,7 +140,7 @@ void ColumnDate::render ( // Determine the output date format, which uses a hierarchy of definitions. // rc.report..dateformat // rc.dateformat.report - // rc.dateformat. + // rc.dateformat std::string format = context.config.get ("report." + _report + ".dateformat"); if (format == "") format = context.config.get ("dateformat.report"); @@ -147,6 +153,16 @@ void ColumnDate::render ( Date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)) .toString (format), width))); } + else if (_style == "countdown") + { + Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); + Date now; + + lines.push_back ( + color.colorize ( + rightJustify ( + Duration (now - date).format (), width))); + } else if (_style == "julian") { lines.push_back ( diff --git a/src/columns/ColDue.cpp b/src/columns/ColDue.cpp index 94101f080..d752a206b 100644 --- a/src/columns/ColDue.cpp +++ b/src/columns/ColDue.cpp @@ -73,46 +73,3 @@ void ColumnDue::setStyle (const std::string& value) } //////////////////////////////////////////////////////////////////////////////// -// Set the minimum and maximum widths for the value. -void ColumnDue::measure (Task& task, unsigned int& minimum, unsigned int& maximum) -{ - minimum = maximum = 0; - - if (task.has (_name)) - { - if (_style == "countdown") - { - Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); - Date now; - minimum = maximum = Duration (now - date).format ().length (); - } - else - ColumnDate::measure (task, minimum, maximum); - } -} - -//////////////////////////////////////////////////////////////////////////////// -void ColumnDue::render ( - std::vector & lines, - Task& task, - int width, - Color& color) -{ - if (task.has (_name)) - { - if (_style == "countdown") - { - Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); - Date now; - - lines.push_back ( - color.colorize ( - rightJustify ( - Duration (now - date).format (), width))); - } - else - ColumnDate::render (lines, task, width, color); - } -} - -//////////////////////////////////////////////////////////////////////////////// diff --git a/src/columns/ColDue.h b/src/columns/ColDue.h index e86ef12fb..5a2ee83d0 100644 --- a/src/columns/ColDue.h +++ b/src/columns/ColDue.h @@ -39,8 +39,6 @@ public: bool validate (std::string&); void setStyle (const std::string&); - void measure (Task&, unsigned int&, unsigned int&); - void render (std::vector &, Task&, int, Color&); }; #endif diff --git a/src/columns/ColScheduled.cpp b/src/columns/ColScheduled.cpp index 8ec689802..7d72d38c4 100644 --- a/src/columns/ColScheduled.cpp +++ b/src/columns/ColScheduled.cpp @@ -73,46 +73,3 @@ void ColumnScheduled::setStyle (const std::string& value) } //////////////////////////////////////////////////////////////////////////////// -// Set the minimum and maximum widths for the value. -void ColumnScheduled::measure (Task& task, unsigned int& minimum, unsigned int& maximum) -{ - minimum = maximum = 0; - - if (task.has (_name)) - { - if (_style == "countdown") - { - Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); - Date now; - minimum = maximum = Duration (now - date).format ().length (); - } - else - ColumnDate::measure (task, minimum, maximum); - } -} - -//////////////////////////////////////////////////////////////////////////////// -void ColumnScheduled::render ( - std::vector & lines, - Task& task, - int width, - Color& color) -{ - if (task.has (_name)) - { - if (_style == "countdown") - { - Date date ((time_t) strtol (task.get (_name).c_str (), NULL, 10)); - Date now; - - lines.push_back ( - color.colorize ( - rightJustify ( - Duration (now - date).format (), width))); - } - else - ColumnDate::render (lines, task, width, color); - } -} - -//////////////////////////////////////////////////////////////////////////////// diff --git a/src/columns/ColScheduled.h b/src/columns/ColScheduled.h index 681f85ebe..715dce4d7 100644 --- a/src/columns/ColScheduled.h +++ b/src/columns/ColScheduled.h @@ -39,8 +39,6 @@ public: bool validate (std::string&); void setStyle (const std::string&); - void measure (Task&, unsigned int&, unsigned int&); - void render (std::vector &, Task&, int, Color&); }; #endif diff --git a/src/columns/ColUDA.cpp b/src/columns/ColUDA.cpp index ad95c89b8..275a051fb 100644 --- a/src/columns/ColUDA.cpp +++ b/src/columns/ColUDA.cpp @@ -92,7 +92,7 @@ void ColumnUDA::measure (Task& task, unsigned int& minimum, unsigned int& maximu // Determine the output date format, which uses a hierarchy of definitions. // rc.report..dateformat // rc.dateformat.report - // rc.dateformat. + // rc.dateformat Date date ((time_t) strtol (value.c_str (), NULL, 10)); std::string format = context.config.get ("report." + _report + ".dateformat"); if (format == "") @@ -100,7 +100,7 @@ void ColumnUDA::measure (Task& task, unsigned int& minimum, unsigned int& maximu if (format == "") format = context.config.get ("dateformat"); - minimum = maximum = utf8_width (date.toString (format)); + minimum = maximum = Date::length (format); } else if (_type == "duration") { diff --git a/test/date.t.cpp b/test/date.t.cpp index db75cb3b1..618fa750e 100644 --- a/test/date.t.cpp +++ b/test/date.t.cpp @@ -35,7 +35,7 @@ Context context; //////////////////////////////////////////////////////////////////////////////// int main (int argc, char** argv) { - UnitTest t (184); + UnitTest t (205); try { @@ -407,6 +407,30 @@ int main (int argc, char** argv) Date r29 (3, 13, 2010, 23, 59, 59); r29++; t.is (r29.toString ("YMDHNS"), "20100314235959", "increment across spring DST boundary"); + + // int Date::length (const std::string&); + t.is (Date::length ("m"), 2, "length 'm' --> 2"); + t.is (Date::length ("M"), 2, "length 'M' --> 2"); + t.is (Date::length ("d"), 2, "length 'd' --> 2"); + t.is (Date::length ("D"), 2, "length 'D' --> 2"); + t.is (Date::length ("y"), 2, "length 'y' --> 2"); + t.is (Date::length ("Y"), 4, "length 'Y' --> 4"); + t.is (Date::length ("a"), 3, "length 'a' --> 3"); + t.is (Date::length ("A"), 10, "length 'A' --> 10"); + t.is (Date::length ("b"), 3, "length 'b' --> 3"); + t.is (Date::length ("B"), 10, "length 'B' --> 10"); + t.is (Date::length ("v"), 2, "length 'v' --> 2"); + t.is (Date::length ("V"), 2, "length 'V' --> 2"); + t.is (Date::length ("h"), 2, "length 'h' --> 2"); + t.is (Date::length ("H"), 2, "length 'H' --> 2"); + t.is (Date::length ("n"), 2, "length 'n' --> 2"); + t.is (Date::length ("N"), 2, "length 'N' --> 2"); + t.is (Date::length ("s"), 2, "length 's' --> 2"); + t.is (Date::length ("S"), 2, "length 'S' --> 2"); + t.is (Date::length ("j"), 3, "length 'j' --> 3"); + t.is (Date::length ("J"), 3, "length 'J' --> 3"); + + t.is (Date::length (" "), 1, "length ' ' --> 1"); } catch (const std::string& e)