Created attachment 2637 [details]
proposal to fix parseDateTime

Let's fix it properly, including support for the 'Z' format.
Added to R14.0.4 bug list. I will work on this during the weekend or at latest next week.
I changed back priority to normal, since it is not a blocker for the whole TDE project.

One note on the Z format.

20160316T152653Z is valid, so a check behind Z resulting in empty should be valid. The part as I understand is +/- signed time offset

thanks in advance

Created attachment 2638 [details]
proposal to fix parseDateTime v2

After thinking for a while in the past few days, it looks to me that the solution in the function is over-complicated.
I suggest a new solution.
I'll test it next and report back.

Actually I am thinking to use regex to make sure the match is exact.
But if you have other patches fill free to attach them, I will check them all.
Cheers
  Michele

Hi Michele,
the v2 I proposed works fine. I'll attach the patch in couple of minutes.

I do not encourage you to use regex when unnecessary and in this case the approach with removing '-' and ':' is elegant and confirms to ISO standard. 
You have to match 'T' and if you want to add awareness of time zone - match 'Z'.
The ISO is pretty clear what is allowed and what not.

I did not answer that fast, because I was still seeing
TQDate::setYMD: Invalid date 0000-00-00. 

It took me a while (actually I had to write a test program) to understand what was wrong.

It turns out if you feed parseVCard with addresse, where birthday is empty "BDAY:\n", it would feed empty string to parseDateTime, which in my opinion is not correct.

We may need to rework this part as well via another ticket. I am not quite sure how this influences my work here atm.

regards

Looking again into the details of the ISO 8601 spec, I understand much better why they have taken this approach for parsing ... and never finished/implemented it right. 

Perhaps regex is more appropriate - I don't know. If you are not using UTC (Z) and pass some TZ offset it will not work with the old and new code, but there is a TZ key in the vCard format - I'm a bit confused. One should check the vCard specs, but I did not do it so far.

So what means doing this in appropriate way? add an ISO8601 parser/converter? The version we have also does not understand 'W' etc.

If you want the patch, let me know. 

For BDAY, I suggest this (it works for me)

--- orig/tdelibs-trinity-14.0.3/tdeabc/vcardtool.cpp    2013-09-03 21:00:38.000000000 +0200
+++ tdelibs-trinity-14.0.3/tdeabc/vcardtool.cpp 2016-03-19 00:20:47.694643684 +0100
@@ -412,9 +412,11 @@
           addr.setAgent( parseAgent( *lineIt ) );

         // BDAY
-        else if ( identifier == "bday" )
-          addr.setBirthday( parseDateTime( (*lineIt).value().asString() ) );
-
+        else if ( identifier == "bday" ) {
+         TQString s((*lineIt).value().asString());
+         if ( s.length() > 0 )
+             addr.setBirthday( parseDateTime( s ) );
+       }


regards

IMO regexs are perfect for this task, since we have to verify that the string adhears to a specific format.

As for what implement, I would ssay to take example from here
https://www.w3.org/TR/NOTE-datetime
https://en.wikipedia.org/wiki/ISO_8601
If we support all those formats, we should be ok :-)

What do you think?

For the birthday issue, your suggestion looks good at first sight.
I will check better during the week. If confirmed good I will push a commit to your name.

Just for info, I started working on it ;-)

Created attachment 2640 [details]
Patch for VCardTool::parseDateTime()

Hi Deloptes,
please try the attached patch and let me know if it is ok for you. If so, I will push to GIT.
It supports all formats specified in https://www.w3.org/TR/NOTE-datetime and the equivalent ones without '-' and ':' (so basically also 20160323T103401Z is supported)

The patch applies successfully on both R14.1 and R14.0 branches. In the end we may end up pushing it only to R14.1 since it adds functionality and change quite a bit of code. I will discuss with Slavek once you are happy with the patch.

Forgot to mention, the patch does not currently support these formats:
week:                                       2016-W11
week number + day:                2016-W11-6
progressive day from Jan 1st: 	2016-079

Do you think we need these? If so, I can add them, although it only support date in this form and not time.

>It turns out if you feed parseVCard with addresse, where birthday is empty "BDAY:\n", it would feed empty 
>string to parseDateTime, which in my opinion is not correct.
>We may need to rework this part as well via another ticket. I am not quite sure how this influences my work 
>here atm.
I also took a look at this. If the birthday string is empty or invalid, the code will set the birthday field to an empty TQDateTime object, which IMO may be acceptable (i.e. there is no birthday specified).
Did you see any specific problem with that? If not, I propose to not doing any change about it. If yes, please let me know the issue and we will look at fixing it ;-)

Hi Michele,
sorry for late response. My time window gets closed and I can follow up very slowly now.
The patch without a fix for the birthday issue is not good for me. TDE throws an error and the process supervising the execution fails. With the birthday fix it works for me.
The problem is that it only tries to set the date/time to 0. The underlay function TQDate::setYMD if I recall correctly and the other one for HMS refuse to handle the 0 value. And in fact it is true, because as we know 0 date is 1970101...
I gave up waiting for the patch because I know that I have limitted time frame to work on my project and patched myself, created deb etc etc, so its not critical anymore - at least for me.
I don't know if the BDAY is critical for other applications as I said I had to patch it to be able to use it. IMO it is OK to have it, because empty BDAY field is pretty legal.

regards

Ok, understood.
Then I will push the birthday patch to both R14.0.X and R14.1, while the regex patch will be pushed only to R14.1 for the moment. After discussing with Slavek, we may push that to R14.0.x perhaps.

Deloptes,
I will push the patch for the birthday with your name, being your work.
What name and email address should I use?

Regex patch pushed to R14.1.0 in commit f9abaf5

Regex patch also pushed to R14.0.x in commit eacbecd

(In reply to Michele Calgaro from comment #18)
> Regex patch also pushed to R14.0.x in commit eacbecd

It is great! Before that due CardDAV addressbook I had in .xsession-errors flock of such reports:

TQTime::setHMS Invalid time 48:00:00.000

After the patch are now these reports away!
Guys, thank you for your good work!

Hi,
great to hear that from Slavek. Unfortunately the spare time, I have I use to chase the challenges on my goal - the sync.

Deloptes,
maybe you missed comment 16.

I would like to push the patch for the birthday with your name, being your work. What name and email address should I use?

Hi Michele,
sorry I missed the most and now found time to catch up on that. Sorry, but I get involved in new project and at home had to babysit for 1 week, which rendered the free time to just 1h/day.

Here my answers
Comment 12:
  week:                         2016-W11
  week number + day:            2016-W11-6
  progressive day from Jan 1st: 2016-079

I do not think this is needed for the card tool. I mentioned this because you used the word "properly", which means implement ISO-8601.

Comment 16:
You can use the yahoo or alternatively Name(E. Kotsev), Mail(office@fincom.at)

big thank you for pushing the changes

Sorry for the late action, I have been busy....

Birthday patch pushed in commit d10e1df (R14.1.x) and fef74a0 (R14.0.x).
Thanks deloptes!!