Skip to content

Commit 8e70986

Browse files
apoelstralauft
apoelstra
and
lauft
authored
Duration: support negative durations by prefixing a '-' before the P in ISO format (#111)
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. Signed-off-by: Thomas Lauf <thomas.lauf@tngtech.com> authored-by: <apoelstra@wpsoftware.net> Co-authored-by: Thomas Lauf <thomas.lauf@tngtech.com>
1 parent 71b8ab5 commit 8e70986

2 files changed

Lines changed: 153 additions & 17 deletions

File tree

‎src/Duration.cpp‎

Lines changed: 74 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -217,55 +217,84 @@ bool Duration::parse_designated (Pig& pig)
217217
{
218218
auto checkpoint = pig.cursor ();
219219

220+
// sign = -1 if a '-' is present, else 1
221+
int sign = !pig.skip ('-') * 2 - 1;
222+
220223
if (pig.skip ('P') &&
221224
! pig.eos ())
222225
{
223226
long long value;
224227
pig.save ();
225228
if (pig.getDigits (value) && pig.skip ('Y'))
226-
_year = value;
229+
{
230+
_year = sign * value;
231+
}
227232
else
233+
{
228234
pig.restore ();
235+
}
229236

230237
pig.save ();
231238
if (pig.getDigits (value) && pig.skip ('M'))
232-
_month = value;
239+
{
240+
_month = sign * value;
241+
}
233242
else
243+
{
234244
pig.restore ();
245+
}
235246

236247
pig.save ();
237248
if (pig.getDigits (value) && pig.skip ('D'))
238-
_day = value;
249+
{
250+
_day = sign * value;
251+
}
239252
else
253+
{
240254
pig.restore ();
255+
}
241256

242257
if (pig.skip ('T') &&
243258
! pig.eos ())
244259
{
245260
pig.save ();
246261
if (pig.getDigits (value) && pig.skip ('H'))
247-
_hours = value;
262+
{
263+
_hours = sign * value;
264+
}
248265
else
266+
{
249267
pig.restore ();
268+
}
250269

251270
pig.save ();
252271
if (pig.getDigits (value) && pig.skip ('M'))
253-
_minutes = value;
272+
{
273+
_minutes = sign * value;
274+
}
254275
else
276+
{
255277
pig.restore ();
278+
}
256279

257280
pig.save ();
258281
if (pig.getDigits (value) && pig.skip ('S'))
259-
_seconds = value;
282+
{
283+
_seconds = sign * value;
284+
}
260285
else
286+
{
261287
pig.restore ();
288+
}
262289
}
263290

264291
auto following = pig.peek ();
265292
if (pig.cursor () - checkpoint >= 3 &&
266293
! unicodeLatinAlpha (following) &&
267294
! unicodeLatinDigit (following))
295+
{
268296
return true;
297+
}
269298
}
270299

271300
pig.restoreTo (checkpoint);
@@ -400,12 +429,19 @@ std::string Duration::format () const
400429
if (_period)
401430
{
402431
time_t t = _period;
432+
433+
std::stringstream s;
434+
if (t < 0)
435+
{
436+
s << '-';
437+
t *= -1;
438+
}
439+
403440
int seconds = t % 60; t /= 60;
404441
int minutes = t % 60; t /= 60;
405442
int hours = t % 24; t /= 24;
406443
int days = t;
407444

408-
std::stringstream s;
409445
if (days)
410446
s << days << "d ";
411447

@@ -429,11 +465,18 @@ std::string Duration::formatHours () const
429465
if (_period)
430466
{
431467
time_t t = _period;
468+
469+
std::stringstream s;
470+
if (t < 0)
471+
{
472+
s << '-';
473+
t *= -1;
474+
}
475+
432476
int seconds = t % 60; t /= 60;
433477
int minutes = t % 60; t /= 60;
434478
int hours = t;
435479

436-
std::stringstream s;
437480
s << hours
438481
<< ':'
439482
<< std::setw (2) << std::setfill ('0') << minutes
@@ -454,12 +497,19 @@ std::string Duration::formatISO () const
454497
if (_period)
455498
{
456499
time_t t = _period;
500+
501+
std::stringstream s;
502+
if (t < 0)
503+
{
504+
s << '-';
505+
t *= -1;
506+
}
507+
457508
int seconds = t % 60; t /= 60;
458509
int minutes = t % 60; t /= 60;
459510
int hours = t % 24; t /= 24;
460511
int days = t;
461512

462-
std::stringstream s;
463513
s << 'P';
464514
if (days) s << days << 'D';
465515

@@ -492,16 +542,24 @@ std::string Duration::formatISO () const
492542
//
493543
std::string Duration::formatVague (bool padding) const
494544
{
545+
time_t t = _period;
495546
float days = (float) _period / 86400.0;
496547

497548
std::stringstream formatted;
498-
if (_period >= 86400 * 365) formatted << std::fixed << std::setprecision (1) << (days / 365) << (padding ? "y " : "y");
499-
else if (_period >= 86400 * 90) formatted << static_cast <int> (days / 30) << (padding ? "mo " : "mo");
500-
else if (_period >= 86400 * 14) formatted << static_cast <int> (days / 7) << (padding ? "w " : "w");
501-
else if (_period >= 86400) formatted << static_cast <int> (days) << (padding ? "d " : "d");
502-
else if (_period >= 3600) formatted << static_cast <int> (_period / 3600) << (padding ? "h " : "h");
503-
else if (_period >= 60) formatted << static_cast <int> (_period / 60) << "min"; // Longest suffix - no padding
504-
else if (_period >= 1) formatted << static_cast <int> (_period) << (padding ? "s " : "s");
549+
if (t < 0)
550+
{
551+
formatted << '-';
552+
t *= -1;
553+
days *= -1.0;
554+
}
555+
556+
if (t >= 86400 * 365) formatted << std::fixed << std::setprecision (1) << (days / 365) << (padding ? "y " : "y");
557+
else if (t >= 86400 * 90) formatted << static_cast <int> (days / 30) << (padding ? "mo " : "mo");
558+
else if (t >= 86400 * 14) formatted << static_cast <int> (days / 7) << (padding ? "w " : "w");
559+
else if (t >= 86400) formatted << static_cast <int> (days) << (padding ? "d " : "d");
560+
else if (t >= 3600) formatted << static_cast <int> (t / 3600) << (padding ? "h " : "h");
561+
else if (t >= 60) formatted << static_cast <int> (t / 60) << "min"; // Longest suffix - no padding
562+
else if (t >= 1) formatted << static_cast <int> (t) << (padding ? "s " : "s");
505563

506564
return formatted.str ();
507565
}

‎test/duration.t.cpp‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ void testParseError (
9393
////////////////////////////////////////////////////////////////////////////////
9494
int main (int, char**)
9595
{
96-
UnitTest t (1921);
96+
UnitTest t (2229);
9797

9898
// Simple negative tests.
9999
testParseError (t, "foo");
@@ -133,6 +133,28 @@ int main (int, char**)
133133

134134
// input i Year Mo We Da Ho Mi Se time_t format hours iso vague
135135
testParse (t, "0seconds", 8, 0, 0, 0, 0, 0, 0, 0, 0, "0:00:00", "0:00:00", "PT0S", "");
136+
137+
// Negative ISO format tests
138+
// input i Year Mo We Da Ho Mi Se time_t format hours iso vague
139+
testParse (t, "-P1Y", 4, -1, 0, 0, 0, 0, 0, 0, -year, "-365d 0:00:00", "-8760:00:00", "-P365D", "-1.0y");
140+
testParse (t, "-P1M", 4, 0, -1, 0, 0, 0, 0, 0, -month, "-30d 0:00:00", "-720:00:00", "-P30D", "-4w");
141+
testParse (t, "-P1D", 4, 0, 0, 0, -1, 0, 0, 0, -day, "-1d 0:00:00", "-24:00:00", "-P1D", "-1d");
142+
testParse (t, "-P1Y1M", 6, -1, -1, 0, 0, 0, 0, 0, -(year + month), "-395d 0:00:00", "-9480:00:00", "-P395D", "-1.1y");
143+
testParse (t, "-P1Y1D", 6, -1, 0, 0, -1, 0, 0, 0, -(year + day), "-366d 0:00:00", "-8784:00:00", "-P366D", "-1.0y");
144+
testParse (t, "-P1M1D", 6, 0, -1, 0, -1, 0, 0, 0, -(month + day), "-31d 0:00:00", "-744:00:00", "-P31D", "-4w");
145+
testParse (t, "-P1Y1M1D", 8, -1, -1, 0, -1, 0, 0, 0, -(year + month + day), "-396d 0:00:00", "-9504:00:00", "-P396D", "-1.1y");
146+
testParse (t, "-PT1H", 5, 0, 0, 0, 0, -1, 0, 0, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h");
147+
testParse (t, "-PT1M", 5, 0, 0, 0, 0, 0, -1, 0, -m, "-0:01:00", "-0:01:00", "-PT1M", "-1min");
148+
testParse (t, "-PT1S", 5, 0, 0, 0, 0, 0, 0, -1, -1, "-0:00:01", "-0:00:01", "-PT1S", "-1s");
149+
testParse (t, "-PT1H1M", 7, 0, 0, 0, 0, -1, -1, 0, -(h + m), "-1:01:00", "-1:01:00", "-PT1H1M", "-1h");
150+
testParse (t, "-PT1H1S", 7, 0, 0, 0, 0, -1, 0, -1, -(h + 1), "-1:00:01", "-1:00:01", "-PT1H1S", "-1h");
151+
testParse (t, "-PT1M1S", 7, 0, 0, 0, 0, 0, -1, -1, -(m + 1), "-0:01:01", "-0:01:01", "-PT1M1S", "-1min");
152+
testParse (t, "-PT1H1M1S", 9, 0, 0, 0, 0, -1, -1, -1, -(h + m + 1), "-1:01:01", "-1:01:01", "-PT1H1M1S", "-1h");
153+
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");
154+
testParse (t, "-PT24H", 6, 0, 0, 0, 0, -24, 0, 0, -day, "-1d 0:00:00", "-24:00:00", "-P1D", "-1d");
155+
testParse (t, "-PT40000000S", 12, 0, 0, 0, 0, 0, 0, -40000000, -40000000, "-462d 23:06:40", "-11111:06:40", "-P462DT23H6M40S", "-1.3y");
156+
testParse (t, "-PT3600S", 8, 0, 0, 0, 0, 0, 0, -3600, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h");
157+
testParse (t, "-PT60M", 6, 0, 0, 0, 0, 0, -60, 0, -h, "-1:00:00", "-1:00:00", "-PT1H", "-1h");
136158
testParse (t, "2 seconds", 9, 0, 0, 0, 0, 0, 0, 0, 2, "0:00:02", "0:00:02", "PT2S", "2s");
137159
testParse (t, "10seconds", 9, 0, 0, 0, 0, 0, 0, 0, 10, "0:00:10", "0:00:10", "PT10S", "10s");
138160
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**)
399421

400422
t.diag ("--------------------------------------------");
401423

424+
// Test negative duration formatting
425+
// Verify that negative durations are formatted with a leading '-'
426+
t.is (Duration ("-1s").format (), "-0:00:01", "format: -1s -> '-0:00:01'");
427+
t.is (Duration ("-60s").format (), "-0:01:00", "format: -60s -> '-0:01:00'");
428+
t.is (Duration ("-3600s").format (), "-1:00:00", "format: -3600s -> '-1:00:00'");
429+
t.is (Duration ("-86400s").format (), "-1d 0:00:00", "format: -86400s -> '-1d 0:00:00'");
430+
t.is (Duration ("-90061s").format (), "-1d 1:01:01", "format: -90061s -> '-1d 1:01:01'");
431+
432+
t.is (Duration ("-1s").formatHours (), "-0:00:01", "formatHours: -1s -> '-0:00:01'");
433+
t.is (Duration ("-60s").formatHours (), "-0:01:00", "formatHours: -60s -> '-0:01:00'");
434+
t.is (Duration ("-3600s").formatHours (), "-1:00:00", "formatHours: -3600s -> '-1:00:00'");
435+
t.is (Duration ("-90061s").formatHours (), "-25:01:01", "formatHours: -90061s -> '-25:01:01'");
436+
437+
t.is (Duration ("-1s").formatISO (), "-PT1S", "formatISO: -1s -> '-PT1S'");
438+
t.is (Duration ("-60s").formatISO (), "-PT1M", "formatISO: -60s -> '-PT1M'");
439+
t.is (Duration ("-3600s").formatISO (), "-PT1H", "formatISO: -3600s -> '-PT1H'");
440+
t.is (Duration ("-86400s").formatISO (), "-P1D", "formatISO: -86400s -> '-P1D'");
441+
t.is (Duration ("-90061s").formatISO (), "-P1DT1H1M1S", "formatISO: -90061s -> '-P1DT1H1M1S'");
442+
443+
t.is (Duration ("-1s").formatVague (), "-1s", "formatVague: -1s -> '-1s'");
444+
t.is (Duration ("-60s").formatVague (), "-1min", "formatVague: -60s -> '-1min'");
445+
t.is (Duration ("-3600s").formatVague (), "-1h", "formatVague: -3600s -> '-1h'");
446+
t.is (Duration ("-86400s").formatVague (), "-1d", "formatVague: -86400s -> '-1d'");
447+
t.is (Duration ("-604800s").formatVague (), "-7d", "formatVague: -604800s -> '-7d'");
448+
t.is (Duration ("-2592000s").formatVague (), "-4w", "formatVague: -2592000 -> '-4w'");
449+
t.is (Duration ("-7776000s").formatVague (), "-3mo", "formatVague: -7776000 -> '-3mo'");
450+
t.is (Duration ("-31536000s").formatVague (), "-1.0y", "formatVague: -31536000 -> '-1.0y'");
451+
452+
// Test round-trip: parse negative ISO format, then format back
453+
t.is (Duration ("-PT1S").formatISO (), "-PT1S", "round-trip: -PT1S -> parse -> formatISO -> -PT1S");
454+
t.is (Duration ("-PT1M").formatISO (), "-PT1M", "round-trip: -PT1M -> parse -> formatISO -> -PT1M");
455+
t.is (Duration ("-PT1H").formatISO (), "-PT1H", "round-trip: -PT1H -> parse -> formatISO -> -PT1H");
456+
t.is (Duration ("-P1D").formatISO (), "-P1D", "round-trip: -P1D -> parse -> formatISO -> -P1D");
457+
t.is (Duration ("-P1DT1H1M1S").formatISO (), "-P1DT1H1M1S", "round-trip: -P1DT1H1M1S -> parse -> formatISO -> -P1DT1H1M1S");
458+
t.is (Duration ("-PT1H1M1S").formatISO (), "-PT1H1M1S", "round-trip: -PT1H1M1S -> parse -> formatISO -> -PT1H1M1S");
459+
t.is (Duration ("-P1Y").formatISO (), "-P365D", "round-trip: -P1Y -> parse -> formatISO -> -P365D");
460+
t.is (Duration ("-P1M").formatISO (), "-P30D", "round-trip: -P1M -> parse -> formatISO -> -P30D");
461+
462+
// Test formatVague with padding
463+
t.is (Duration ("-1s").formatVague (true), "-1s ", "formatVague (true): -1s -> '-1s '");
464+
t.is (Duration ("-3600s").formatVague (true), "-1h ", "formatVague (true): -3600s -> '-1h '");
465+
t.is (Duration ("-86400s").formatVague (true), "-1d ", "formatVague (true): -86400s -> '-1d '");
466+
t.is (Duration ("-604800s").formatVague (true), "-7d ", "formatVague (true): -604800s -> '-7d '");
467+
468+
// Test that positive durations don't get a leading '-'
469+
t.is (Duration ("1s").format (), "0:00:01", "format: 1s -> '0:00:01' (no leading '-')");
470+
t.is (Duration ("1s").formatHours (), "0:00:01", "formatHours: 1s -> '0:00:01' (no leading '-')");
471+
t.is (Duration ("1s").formatISO (), "PT1S", "formatISO: 1s -> 'PT1S' (no leading '-')");
472+
t.is (Duration ("1s").formatVague (), "1s", "formatVague: 1s -> '1s' (no leading '-')");
473+
474+
// Test zero duration
475+
t.is (Duration ("0s").format (), "0:00:00", "format: 0s -> '0:00:00'");
476+
t.is (Duration ("0s").formatHours (), "0:00:00", "formatHours: 0s -> '0:00:00'");
477+
t.is (Duration ("0s").formatISO (), "PT0S", "formatISO: 0s -> 'PT0S'");
478+
t.is (Duration ("0s").formatVague (), "", "formatVague: 0s -> ''");
479+
402480
return 0;
403481
}
404482

0 commit comments

Comments
 (0)