From 813d5adbe3d2d43c96e9b54c6a452b77db2d27cd Mon Sep 17 00:00:00 2001 From: Andrew Poelstra Date: Sat, 27 Dec 2025 00:00:33 +0000 Subject: [PATCH] Duration: support negative durations by a '-' prefixing the ISO format Currently in taskwarrior if you attempt to set a negative duration for a UDA, the UDA will appear to be set to PT0S (zero seconds). What is actually happening is a bit weird. Let's consider the example where the user types '-80s'. 1. First, Duration::parse_units will correctly read this as -80 seconds. 2. Then, Duration::formatISO will write this out as PT-1M-20S, which is what gets stored in the database. 3. Later, when the user attempts to read this value, Duration::parse will fail, and the default value of 0 will be output. In my view, we should either reject negative durations when the user first enters them, or this round-trip should succeed. I would kinda like to have negative durations, so this PR makes it succeed, by extending the ISO parser to allow negative entries for times. This approach has the benefit that users who have already accidentally put negative durations into their taskwarrior db will have these entries start working (though this may be perceived as a surprising/breaking change). It has the drawback that we're defining an ad-hoc extension to the ISO format and blessing it in our parser, making it a breaking change to revert this in the future. (Re "ad-hoc" -- I read through ISO 8601 and it does not allow this, or negative durations of any form for that matter. Also, our current code produces this output by using the modulo operator on negative numbers, which used to be implementation defined behavior) (though [this SE post](https://stackoverflow.com/questions/7594508/why-does-the-modulo-operator-result-in-negative-values) claims that C++11 defines it). It also has the drawback that users will see durations like PT-1H-56M-9S in their taskwarrior output, which to my eye looks like a bug, or at least an unintentionally exposed implementation detail. An alternative approach would be to extend the duration format by adding an optional +/- sign before the P. According to [this MDN page](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Temporal/Duration) this is defined by ECMAScript. The benefit here is: we're following a standard (assuming the MDN doc is correct); no existing timewarrior databases are likely to contain durations of this form, reducing the chances of "surprise" behavior changes; the resulting output looks much nicer. It's also easier to implement, though I need to modify both parse() and formatISO(). I have done it on a private branch -- let me know and I will close this PR and open one with this alternative approach. --- src/Duration.cpp | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/src/Duration.cpp b/src/Duration.cpp index 55da1dd..0a6bf23 100644 --- a/src/Duration.cpp +++ b/src/Duration.cpp @@ -221,21 +221,25 @@ bool Duration::parse_designated (Pig& pig) ! pig.eos ()) { long long value; + int sign; pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('Y')) - _year = value; + _year = sign * value; else pig.restore (); pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('M')) - _month = value; + _month = sign * value; else pig.restore (); pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('D')) - _day = value; + _day = sign * value; else pig.restore (); @@ -243,20 +247,23 @@ bool Duration::parse_designated (Pig& pig) ! pig.eos ()) { pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('H')) - _hours = value; + _hours = sign * value; else pig.restore (); pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('M')) - _minutes = value; + _minutes = sign * value; else pig.restore (); pig.save (); + sign = !pig.skip ('-') * 2 - 1; // -1 if a '-' is present, else 1 if (pig.getDigits (value) && pig.skip ('S')) - _seconds = value; + _seconds = sign * value; else pig.restore (); }