search

GothenburgBitFactory / taskwarrior

Taskwarrior - Command line Task Management
https://taskwarrior.org
MIT License
4.44k stars 303 forks source link

'1970-01-01T00:00:00Z' is not a valid date in the 'Y-M-D' format. #3572

Open stefan-sherwood opened 3 months ago

stefan-sherwood commented 3 months ago

To report a bug...

task add description:"Test date" modified:"1970-01-01T00:00:00Z"

If I do the identical command with a slightly different date in the same format, it correctly adds the task with the given time/date,

$ task add description:"Test date" modified:"1970-01-01T00:00:01Z"
Created task 44.

Note that the date format I gave is correct, both according to ISO-8601 rules and according to taskwarrior specification for supported dates.

Task to be added.

Error message: '1970-01-01T00:00:00Z' is not a valid date in the 'Y-M-D' format.

Task was not added.

Compiler Version: 13.2.0 Caps: +stdc +stdc_hosted +LP64 +c8 +i32 +l64 +vp64 +time_t64 Compliance: C++17

Build Features CMake: 3.27.4 libuuid: libuuid + uuid_unparse_lower Build type: release

Configuration File: /home/stefan/.taskrc (found), 1381 bytes, mode 100644 Data: /home/stefan/.task (found), dir, mode 40755 Locking: Enabled GC: Enabled Hooks System: Enabled Location: /home/stefan/.task/hooks (-none-)

Tests Terminal: 350x113 Dups: Scanned 51 tasks for duplicate UUIDs: No duplicates found Broken ref: Scanned 51 tasks for broken references: No broken references found

djmitche commented 3 months ago

I suspect this is because zero is treated as a sentinel value to indicate that the time wasn't converted, and the string ending in :00Z evaluates to zero as a UNIX timestamp.

stefan-sherwood commented 3 months ago

That makes sense. I am trying to run a script from the syncall project that synchronizes Google Keep with taskwarrior. It uses a zero timestamp when there is no modification date, presumably to ensure that all other modification dates compare as being later.

If this is considered functioning as designed, I understand.

As an aside it's a bit strange that it interprets the format was being 'Y-M-D' when it pattern matches an ISO-8601 date/time format but that's just a nitpick about the error message.

djmitche commented 3 months ago

It's trying to interpret that using the dateformat config, but I think that allows some other, fixed formats as well (including all of the relative times like tomorrow or soq and apparently ISO-8601 times)

I think relying on 0 as a sentinel value was based on the idea that nobody was using Taskwarrior in 1970, but the truth is probably lost to time. Presumably we could make 0 valid and return an exception. But I don't know much about the "variant" and "evaluation" stuff in src/columns/ColTypeDate.cpp to know how to do that.

The error message could use some improvement, for sure. Maybe if that just said "Could not parse .. as a date after 1970" it would at least be accurate? WDYT?

stefan-sherwood commented 3 months ago

Looping in the author of syncall @bergercookie to the conversation since this is really about getting the two projects to agree on what makes sense in this context. Having a value that represents "the beginning of time" makes sense to me and having that value be 0 seems to be the right thing to do. Obviously I could change it to pass in 1 but that seems like a hacky workaround.

IMO taskwarrior clients should not be subject to the effects of project-internal sentinel values. I'm not at all familiar with the code but in general if a Variant needs to be able to have a type before its value is set then an additional boolean (e.g. bool _isset) to indicate there's a value is merited. Otherwise, there could be an additional type (e.g. type_none) added that indicates it doesn't have a type.

In terms of the error message, my primary complaint is that it sounds like the reason it's not working is because it's comparing against a different format (YYYY-MM-DD) when ISO-8601 actually is accepted. The comment on the code implies the same thing. The simplest change to change the error message to:

'1970-01-01T00:00:00Z' is not a valid date. See https://taskwarrior.org/docs/dates for more info.

And also update the documentation to indicate what date ranges are accepted since ISO-8601 itself has no such restriction.

As another side note, the syncall project actually uses the now-deprecated truncated ISO-8601 format. And taskwarrior accepts it, even though it's not listed in the taskwarrior documentation as accepted.

djmitche commented 3 months ago

Hopefully someone more familiar with the Variant stuff can fix the internal sentinel values.

But the change to the error message and the two docs fixes sound like great PR(s)!

stefan-sherwood commented 3 months ago

I'm happy to do that but I cannot find where the documentation at https://taskwarrior.org/docs/dates/ is located to edit.

djmitche commented 3 months ago

It's in the https://github.com/GothenburgBitFactory/tw.org repo.

On Wed, Jul 31, 2024 at 7:28 PM stefan-sherwood @.***> wrote:

I'm happy to do that but I cannot find where the documentation at https://taskwarrior.org/docs/dates/ is located to edit.

— Reply to this email directly, view it on GitHub https://github.com/GothenburgBitFactory/taskwarrior/issues/3572#issuecomment-2261653058, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAHAAJ32F3BN25GINN5VKDZPFXJHAVCNFSM6AAAAABLW3MTMCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRRGY2TGMBVHA . You are receiving this because you commented.Message ID: @.***>