From 5df98cb178558291fbb66db5a474563f48bbacd6 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 27 Dec 2025 00:42:47 +0000 Subject: [PATCH 1/4] Duration: support negative durations by prefixing a '-' before the P in ISO format According to [this MDN document](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Duration) there is an ECMAScript extension to ISO 8601 to allow signed durations by putting a + or - before the ISO duration format. Internally we support signed durations -- we will parse "-60w" or "-60min", which we store in a `time_t` (whose signedness is technically implementation defined, but which is signed on all major compilers). However, when formatting these as ISO 8601 we get get garbed output like PT-1H-56M-9S, which we cannot parse and probably neither can anything else. (Taskwarrior, when asked to assign a negative duration to a duration-typed UDA, will store this garbled output but then reproduce it as PT0S, an unfortunate user experience.) This PR updates `Duration::formatISO` to instead prepend a '-' before negative durations, and updates `Duration::parse_designated` to parse such things. Alternative to #110, which simply blesses the garbled format by extending the parser to support it. --- src/Duration.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/Duration.cpp b/src/Duration.cpp index 55da1dd..66f1fa6 100644 --- a/src/Duration.cpp +++ b/src/Duration.cpp @@ -217,25 +217,28 @@ bool Duration::parse_designated (Pig& pig) { auto checkpoint = pig.cursor (); + // sign = -1 if a '-' is present, else 1 + int sign = !pig.skip ('-') * 2 - 1; + if (pig.skip ('P') && ! pig.eos ()) { long long value; pig.save (); if (pig.getDigits (value) && pig.skip ('Y')) - _year = value; + _year = sign * value; else pig.restore (); pig.save (); if (pig.getDigits (value) && pig.skip ('M')) - _month = value; + _month = sign * value; else pig.restore (); pig.save (); if (pig.getDigits (value) && pig.skip ('D')) - _day = value; + _day = sign * value; else pig.restore (); @@ -244,19 +247,19 @@ bool Duration::parse_designated (Pig& pig) { pig.save (); if (pig.getDigits (value) && pig.skip ('H')) - _hours = value; + _hours = sign * value; else pig.restore (); pig.save (); if (pig.getDigits (value) && pig.skip ('M')) - _minutes = value; + _minutes = sign * value; else pig.restore (); pig.save (); if (pig.getDigits (value) && pig.skip ('S')) - _seconds = value; + _seconds = sign * value; else pig.restore (); } @@ -454,12 +457,18 @@ std::string Duration::formatISO () const if (_period) { time_t t = _period; + + std::stringstream s; + if (t < 0) { + s << '-'; + t *= -1; + } + int seconds = t % 60; t /= 60; int minutes = t % 60; t /= 60; int hours = t % 24; t /= 24; int days = t; - std::stringstream s; s << 'P'; if (days) s << days << 'D'; From b76e66ed0d2b4ca4f95b051adbea16c52de6ba18 Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 27 Dec 2025 00:56:38 +0000 Subject: [PATCH 2/4] Duration: update other format methods to support negative dates This will let us remove some special-case logic from Timewarrior. --- src/Duration.cpp | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/src/Duration.cpp b/src/Duration.cpp index 66f1fa6..f728f7d 100644 --- a/src/Duration.cpp +++ b/src/Duration.cpp @@ -403,12 +403,18 @@ std::string Duration::format () const if (_period) { time_t t = _period; + + std::stringstream s; + if (t < 0) { + s << '-'; + t *= -1; + } + int seconds = t % 60; t /= 60; int minutes = t % 60; t /= 60; int hours = t % 24; t /= 24; int days = t; - std::stringstream s; if (days) s << days << "d "; @@ -432,11 +438,17 @@ std::string Duration::formatHours () const if (_period) { time_t t = _period; + + std::stringstream s; + if (t < 0) { + s << '-'; + t *= -1; + } + int seconds = t % 60; t /= 60; int minutes = t % 60; t /= 60; int hours = t; - std::stringstream s; s << hours << ':' << std::setw (2) << std::setfill ('0') << minutes @@ -501,16 +513,23 @@ std::string Duration::formatISO () const // std::string Duration::formatVague (bool padding) const { + time_t t = _period; float days = (float) _period / 86400.0; std::stringstream formatted; - if (_period >= 86400 * 365) formatted << std::fixed << std::setprecision (1) << (days / 365) << (padding ? "y " : "y"); - else if (_period >= 86400 * 90) formatted << static_cast (days / 30) << (padding ? "mo " : "mo"); - else if (_period >= 86400 * 14) formatted << static_cast (days / 7) << (padding ? "w " : "w"); - else if (_period >= 86400) formatted << static_cast (days) << (padding ? "d " : "d"); - else if (_period >= 3600) formatted << static_cast (_period / 3600) << (padding ? "h " : "h"); - else if (_period >= 60) formatted << static_cast (_period / 60) << "min"; // Longest suffix - no padding - else if (_period >= 1) formatted << static_cast (_period) << (padding ? "s " : "s"); + if (t < 0) { + formatted << '-'; + t *= -1; + days *= -1.0; + } + + if (t >= 86400 * 365) formatted << std::fixed << std::setprecision (1) << (days / 365) << (padding ? "y " : "y"); + else if (t >= 86400 * 90) formatted << static_cast (days / 30) << (padding ? "mo " : "mo"); + else if (t >= 86400 * 14) formatted << static_cast (days / 7) << (padding ? "w " : "w"); + else if (t >= 86400) formatted << static_cast (days) << (padding ? "d " : "d"); + else if (t >= 3600) formatted << static_cast (t / 3600) << (padding ? "h " : "h"); + else if (t >= 60) formatted << static_cast (t / 60) << "min"; // Longest suffix - no padding + else if (t >= 1) formatted << static_cast (t) << (padding ? "s " : "s"); return formatted.str (); } From a6da52e011bbe30acde9839ba62c3ce08efe2314 Mon Sep 17 00:00:00 2001 From: Thomas Lauf Date: Wed, 1 Apr 2026 23:55:21 +0200 Subject: [PATCH 3/4] Add curly braces, apply coding style Signed-off-by: Thomas Lauf --- src/Duration.cpp | 38 ++++++++++++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/src/Duration.cpp b/src/Duration.cpp index f728f7d..eeadd01 100644 --- a/src/Duration.cpp +++ b/src/Duration.cpp @@ -226,49 +226,75 @@ bool Duration::parse_designated (Pig& pig) long long value; pig.save (); if (pig.getDigits (value) && pig.skip ('Y')) + { _year = sign * value; + } else + { pig.restore (); + } pig.save (); if (pig.getDigits (value) && pig.skip ('M')) + { _month = sign * value; + } else + { pig.restore (); + } pig.save (); if (pig.getDigits (value) && pig.skip ('D')) + { _day = sign * value; + } else + { pig.restore (); + } if (pig.skip ('T') && ! pig.eos ()) { pig.save (); if (pig.getDigits (value) && pig.skip ('H')) + { _hours = sign * value; + } else + { pig.restore (); + } pig.save (); if (pig.getDigits (value) && pig.skip ('M')) + { _minutes = sign * value; + } else + { pig.restore (); + } pig.save (); if (pig.getDigits (value) && pig.skip ('S')) + { _seconds = sign * value; + } else + { pig.restore (); + } } auto following = pig.peek (); if (pig.cursor () - checkpoint >= 3 && ! unicodeLatinAlpha (following) && ! unicodeLatinDigit (following)) + { return true; + } } pig.restoreTo (checkpoint); @@ -405,7 +431,8 @@ std::string Duration::format () const time_t t = _period; std::stringstream s; - if (t < 0) { + if (t < 0) + { s << '-'; t *= -1; } @@ -440,7 +467,8 @@ std::string Duration::formatHours () const time_t t = _period; std::stringstream s; - if (t < 0) { + if (t < 0) + { s << '-'; t *= -1; } @@ -471,7 +499,8 @@ std::string Duration::formatISO () const time_t t = _period; std::stringstream s; - if (t < 0) { + if (t < 0) + { s << '-'; t *= -1; } @@ -517,7 +546,8 @@ std::string Duration::formatVague (bool padding) const float days = (float) _period / 86400.0; std::stringstream formatted; - if (t < 0) { + if (t < 0) + { formatted << '-'; t *= -1; days *= -1.0; From 720aa50ef87b759369e3332ae947fca731527832 Mon Sep 17 00:00:00 2001 From: Thomas Lauf Date: Thu, 2 Apr 2026 00:13:02 +0200 Subject: [PATCH 4/4] Add test coverage Signed-off-by: Thomas Lauf --- test/duration.t.cpp | 80 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 79 insertions(+), 1 deletion(-) diff --git a/test/duration.t.cpp b/test/duration.t.cpp index 9ab8954..e42662c 100644 --- a/test/duration.t.cpp +++ b/test/duration.t.cpp @@ -93,7 +93,7 @@ void testParseError ( //////////////////////////////////////////////////////////////////////////////// int main (int, char**) { - UnitTest t (1921); + UnitTest t (2229); // Simple negative tests. testParseError (t, "foo"); @@ -133,6 +133,28 @@ int main (int, char**) // input i Year Mo We Da Ho Mi Se time_t format hours iso vague testParse (t, "0seconds", 8, 0, 0, 0, 0, 0, 0, 0, 0, "0:00:00", "0:00:00", "PT0S", ""); + + // Negative ISO format tests + // input i Year Mo We Da Ho Mi Se time_t format hours iso vague + testParse (t, "-P1Y", 4, -1, 0, 0, 0, 0, 0, 0, -year, "-365d 0:00:00", "-8760:00:00", "-P365D", "-1.0y"); + testParse (t, "-P1M", 4, 0, -1, 0, 0, 0, 0, 0, -month, "-30d 0:00:00", "-720:00:00", "-P30D", "-4w"); + testParse (t, "-P1D", 4, 0, 0, 0, -1, 0, 0, 0, -day, "-1d 0:00:00", "-24:00:00", "-P1D", "-1d"); + testParse (t, "-P1Y1M", 6, -1, -1, 0, 0, 0, 0, 0, -(year + month), "-395d 0:00:00", "-9480:00:00", "-P395D", "-1.1y"); + testParse (t, "-P1Y1D", 6, -1, 0, 0, -1, 0, 0, 0, -(year + day), "-366d 0:00:00", "-8784:00:00", "-P366D", "-1.0y"); + testParse (t, "-P1M1D", 6, 0, -1, 0, -1, 0, 0, 0, -(month + day), "-31d 0:00:00", "-744:00:00", "-P31D", "-4w"); + testParse (t, "-P1Y1M1D", 8, -1, -1, 0, -1, 0, 0, 0, -(year + month + day), "-396d 0:00:00", "-9504:00:00", "-P396D", "-1.1y"); + testParse (t, "-PT1H", 5, 0, 0, 0, 0, -1, 0, 0, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h"); + testParse (t, "-PT1M", 5, 0, 0, 0, 0, 0, -1, 0, -m, "-0:01:00", "-0:01:00", "-PT1M", "-1min"); + testParse (t, "-PT1S", 5, 0, 0, 0, 0, 0, 0, -1, -1, "-0:00:01", "-0:00:01", "-PT1S", "-1s"); + testParse (t, "-PT1H1M", 7, 0, 0, 0, 0, -1, -1, 0, -(h + m), "-1:01:00", "-1:01:00", "-PT1H1M", "-1h"); + testParse (t, "-PT1H1S", 7, 0, 0, 0, 0, -1, 0, -1, -(h + 1), "-1:00:01", "-1:00:01", "-PT1H1S", "-1h"); + testParse (t, "-PT1M1S", 7, 0, 0, 0, 0, 0, -1, -1, -(m + 1), "-0:01:01", "-0:01:01", "-PT1M1S", "-1min"); + testParse (t, "-PT1H1M1S", 9, 0, 0, 0, 0, -1, -1, -1, -(h + m + 1), "-1:01:01", "-1:01:01", "-PT1H1M1S", "-1h"); + testParse (t, "-P1Y1M1DT1H1M1S",15, -1, -1, 0, -1, -1, -1, -1, -(year + month + day + h + m + 1), "-396d 1:01:01", "-9505:01:01", "-P396DT1H1M1S", "-1.1y"); + testParse (t, "-PT24H", 6, 0, 0, 0, 0, -24, 0, 0, -day, "-1d 0:00:00", "-24:00:00", "-P1D", "-1d"); + testParse (t, "-PT40000000S", 12, 0, 0, 0, 0, 0, 0, -40000000, -40000000, "-462d 23:06:40", "-11111:06:40", "-P462DT23H6M40S", "-1.3y"); + testParse (t, "-PT3600S", 8, 0, 0, 0, 0, 0, 0, -3600, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h"); + testParse (t, "-PT60M", 6, 0, 0, 0, 0, 0, -60, 0, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h"); testParse (t, "2 seconds", 9, 0, 0, 0, 0, 0, 0, 0, 2, "0:00:02", "0:00:02", "PT2S", "2s"); testParse (t, "10seconds", 9, 0, 0, 0, 0, 0, 0, 0, 10, "0:00:10", "0:00:10", "PT10S", "10s"); testParse (t, "1.5seconds", 10, 0, 0, 0, 0, 0, 0, 0, 1, "0:00:01", "0:00:01", "PT1S", "1s"); @@ -399,6 +421,62 @@ int main (int, char**) t.diag ("--------------------------------------------"); + // Test negative duration formatting + // Verify that negative durations are formatted with a leading '-' + t.is (Duration ("-1s").format (), "-0:00:01", "format: -1s -> '-0:00:01'"); + t.is (Duration ("-60s").format (), "-0:01:00", "format: -60s -> '-0:01:00'"); + t.is (Duration ("-3600s").format (), "-1:00:00", "format: -3600s -> '-1:00:00'"); + t.is (Duration ("-86400s").format (), "-1d 0:00:00", "format: -86400s -> '-1d 0:00:00'"); + t.is (Duration ("-90061s").format (), "-1d 1:01:01", "format: -90061s -> '-1d 1:01:01'"); + + t.is (Duration ("-1s").formatHours (), "-0:00:01", "formatHours: -1s -> '-0:00:01'"); + t.is (Duration ("-60s").formatHours (), "-0:01:00", "formatHours: -60s -> '-0:01:00'"); + t.is (Duration ("-3600s").formatHours (), "-1:00:00", "formatHours: -3600s -> '-1:00:00'"); + t.is (Duration ("-90061s").formatHours (), "-25:01:01", "formatHours: -90061s -> '-25:01:01'"); + + t.is (Duration ("-1s").formatISO (), "-PT1S", "formatISO: -1s -> '-PT1S'"); + t.is (Duration ("-60s").formatISO (), "-PT1M", "formatISO: -60s -> '-PT1M'"); + t.is (Duration ("-3600s").formatISO (), "-PT1H", "formatISO: -3600s -> '-PT1H'"); + t.is (Duration ("-86400s").formatISO (), "-P1D", "formatISO: -86400s -> '-P1D'"); + t.is (Duration ("-90061s").formatISO (), "-P1DT1H1M1S", "formatISO: -90061s -> '-P1DT1H1M1S'"); + + t.is (Duration ("-1s").formatVague (), "-1s", "formatVague: -1s -> '-1s'"); + t.is (Duration ("-60s").formatVague (), "-1min", "formatVague: -60s -> '-1min'"); + t.is (Duration ("-3600s").formatVague (), "-1h", "formatVague: -3600s -> '-1h'"); + t.is (Duration ("-86400s").formatVague (), "-1d", "formatVague: -86400s -> '-1d'"); + t.is (Duration ("-604800s").formatVague (), "-7d", "formatVague: -604800s -> '-7d'"); + t.is (Duration ("-2592000s").formatVague (), "-4w", "formatVague: -2592000 -> '-4w'"); + t.is (Duration ("-7776000s").formatVague (), "-3mo", "formatVague: -7776000 -> '-3mo'"); + t.is (Duration ("-31536000s").formatVague (), "-1.0y", "formatVague: -31536000 -> '-1.0y'"); + + // Test round-trip: parse negative ISO format, then format back + t.is (Duration ("-PT1S").formatISO (), "-PT1S", "round-trip: -PT1S -> parse -> formatISO -> -PT1S"); + t.is (Duration ("-PT1M").formatISO (), "-PT1M", "round-trip: -PT1M -> parse -> formatISO -> -PT1M"); + t.is (Duration ("-PT1H").formatISO (), "-PT1H", "round-trip: -PT1H -> parse -> formatISO -> -PT1H"); + t.is (Duration ("-P1D").formatISO (), "-P1D", "round-trip: -P1D -> parse -> formatISO -> -P1D"); + t.is (Duration ("-P1DT1H1M1S").formatISO (), "-P1DT1H1M1S", "round-trip: -P1DT1H1M1S -> parse -> formatISO -> -P1DT1H1M1S"); + t.is (Duration ("-PT1H1M1S").formatISO (), "-PT1H1M1S", "round-trip: -PT1H1M1S -> parse -> formatISO -> -PT1H1M1S"); + t.is (Duration ("-P1Y").formatISO (), "-P365D", "round-trip: -P1Y -> parse -> formatISO -> -P365D"); + t.is (Duration ("-P1M").formatISO (), "-P30D", "round-trip: -P1M -> parse -> formatISO -> -P30D"); + + // Test formatVague with padding + t.is (Duration ("-1s").formatVague (true), "-1s ", "formatVague (true): -1s -> '-1s '"); + t.is (Duration ("-3600s").formatVague (true), "-1h ", "formatVague (true): -3600s -> '-1h '"); + t.is (Duration ("-86400s").formatVague (true), "-1d ", "formatVague (true): -86400s -> '-1d '"); + t.is (Duration ("-604800s").formatVague (true), "-7d ", "formatVague (true): -604800s -> '-7d '"); + + // Test that positive durations don't get a leading '-' + t.is (Duration ("1s").format (), "0:00:01", "format: 1s -> '0:00:01' (no leading '-')"); + t.is (Duration ("1s").formatHours (), "0:00:01", "formatHours: 1s -> '0:00:01' (no leading '-')"); + t.is (Duration ("1s").formatISO (), "PT1S", "formatISO: 1s -> 'PT1S' (no leading '-')"); + t.is (Duration ("1s").formatVague (), "1s", "formatVague: 1s -> '1s' (no leading '-')"); + + // Test zero duration + t.is (Duration ("0s").format (), "0:00:00", "format: 0s -> '0:00:00'"); + t.is (Duration ("0s").formatHours (), "0:00:00", "formatHours: 0s -> '0:00:00'"); + t.is (Duration ("0s").formatISO (), "PT0S", "formatISO: 0s -> 'PT0S'"); + t.is (Duration ("0s").formatVague (), "", "formatVague: 0s -> ''"); + return 0; }