Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
stefan-sherwood opened this issue Jul 30, 2024 · 12 comments
Open
Labels
eval:reproduced topic:time Issues relating to time type:bug Something isn't working type:docs Man pages or online docs need updates

Comments

@stefan-sherwood
Copy link

To report a bug...

  • What command(s) did you run?

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.

  • What did you expect to happen?

Task to be added.

  • What actually happened?

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

Task was not added.

  • Paste the output of the task diag command.
task 3.0.2
   Platform: Linux

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
Copy link
Collaborator

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.

@djmitche djmitche added the topic:time Issues relating to time label Jul 30, 2024
@stefan-sherwood
Copy link
Author

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
Copy link
Collaborator

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
Copy link
Author

stefan-sherwood commented Jul 30, 2024

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
Copy link
Collaborator

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
Copy link
Author

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

@djmitche
Copy link
Collaborator

djmitche commented Aug 1, 2024 via email

@tbabej
Copy link
Member

tbabej commented Nov 30, 2024

@stefan-sherwood did we end up changing the docs here?

@tbabej tbabej added type:bug Something isn't working type:docs Man pages or online docs need updates eval:reproduced labels Nov 30, 2024
@tbabej
Copy link
Member

tbabej commented Nov 30, 2024

The problem with using beginning of unix time as an actual value reproduced on 3.1.0.

@stefan-sherwood
Copy link
Author

@tbabej I have not made the error/documentation change because I was hoping for an actual solution (aka supporting ISO-8601 dates as described in the doc). Putting a "btw, dates prior to 1970 are not supported" is a band-aid, not a proper solution.

@tbabej
Copy link
Member

tbabej commented Nov 30, 2024

I thought the issue was about trying to use the 0 (=UTC midnight of 1970-01-01) as a value for the time stamp. Are you implying you have an use case for using dates before 1970?

@stefan-sherwood
Copy link
Author

stefan-sherwood commented Nov 30, 2024

The documentation says that ISO-8601 dates are accepted but only a subset of them is actually accepted. Further, the error message for when you give a valid ISO-8601 date that is not accepted is confusing.

I ran across this problem because a separate software package (syncall) I am trying to use is using t=0. I haven't personally had a direct use for pre-1970 dates but I am using it to keep track of events that happened in the past so if any of those turned out to be pre-1970 then I would run into this problem in that context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
eval:reproduced topic:time Issues relating to time type:bug Something isn't working type:docs Man pages or online docs need updates
Projects
Status: Backlog
Development

No branches or pull requests

3 participants