sourcesimian / uICAL

Lightweight C++ and Python library for reading ICAL calendar format
MIT License
18 stars 9 forks source link

Bad DateTime Format for day events #1

Open aggr0lite opened 1 year ago

aggr0lite commented 1 year ago

I tried to use your library and seem to run in an issue where your code throws an error when dealing with events which last a whole or multiple days. I don't know if it has to do with the setting in google calendar where you can check the "All day" event box: image

The iCalendar content looks different here for the event start date: DTSTART;VALUE=DATE:20230514 --> Throws an error: ValueError: Bad datetime: "20230514"

Common events look like this: DTSTART:20230511T113000Z

Is this format simply not supported and could be covered by Libical for example, which you referred to?

ducky64 commented 1 year ago

I was able to bypass this by allowing DateStamp to parse date-only:

diff --git a/src/datestamp.cpp b/src/datestamp.cpp
index ba8a2e0..28ee50b 100644
--- a/src/datestamp.cpp
+++ b/src/datestamp.cpp
@@ -21,18 +21,22 @@ namespace uICAL {
     DateStamp::DateStamp(const string& datestamp) {
         for (;;) {
             try {
-                if (datestamp.length() != 15) {
+                if (datestamp.length() < 8) {
                     break;
                 }
                 this->year = datestamp.substr(0, 4).as_int();
                 this->month = datestamp.substr(4, 2).as_int();
                 this->day = datestamp.substr(6, 2).as_int();
-                if (datestamp.substr(8, 1) != "T") {
-                    break;
+
+                if (datestamp.length() == 15 && datestamp.substr(8, 1) == "T") {
+                    this->hour = datestamp.substr(9, 2).as_int();
+                    this->minute = datestamp.substr(11, 2).as_int();
+                    this->second = datestamp.substr(13, 2).as_int();
+                } else {
+                    this->hour = 0;
+                    this->minute = 0;
+                    this->second = 0;
                 }
-                this->hour = datestamp.substr(9, 2).as_int();
-                this->minute = datestamp.substr(11, 2).as_int();
-                this->second = datestamp.substr(13, 2).as_int();
                 this->validate();
                 return;
             }
diff --git a/src/datetime.cpp b/src/datetime.cpp
index a0c480d..58e2244 100644
--- a/src/datetime.cpp
+++ b/src/datetime.cpp
@@ -40,16 +40,20 @@ namespace uICAL {
     }

     void DateTime::construct(const string& datetime, const TZMap_ptr& tzmap) {
-        if (datetime.length() < 15)
+        DateStamp ds;
+        if (datetime.length() < 8) {
             throw ValueError(string("Bad datetime: \"") + datetime + "\"");
-
-        DateStamp ds(datetime.substr(0, 15));
-
-        if (datetime.length() > 15) {
-            this->tz = new_ptr<TZ>(datetime.substr(15), tzmap);
-        }
-        else {
-            this->tz = TZ::unaware();
+        } else if (datetime.length() < 15) {  // 7-char date only
+            ds = DateStamp(datetime.substr(0, 8));
+            this->tz = new_ptr<TZ>("Z", tzmap);  // TODO should probably be in localtime
+        } else {  // 15+-char datetime
+            ds = DateStamp(datetime.substr(0, 15));
+            if (datetime.length() > 15) {
+                this->tz = new_ptr<TZ>(datetime.substr(15), tzmap);
+            }
+            else {
+                this->tz = TZ::unaware();
+            }
         }

         this->construct(ds, tz);

This is a pretty hackish solution (since it just places the date as UTC), a better solution should differentiate where dates are allowed (vs where a datetime is required) by the spec, and probably also look at how dates should be handled by the spec. The above just hacks it in GMT, whereas the intuitive thing to do would probably be to make it a whole-day event in local / display time.

sourcesimian commented 6 months ago

Feel free to supply a pull request. Please include new tests to exercise both positive and negative and ensure existing tests pass.