[Amarok] Replace strptime() with QDateTime. strptime() is P

Bart Cerneels bart.cerneels at kde.org
Tue Nov 10 11:13:48 CET 2009


On Tue, Nov 10, 2009 at 10:45, Bart Cerneels <bart.cerneels at kde.org> wrote:
> Reading KDEPIM code right now to see if/how this was solved.
>
> Otherwise I suggest an ifdef POSIX but the ordering of episodes
> fetched at the same time will be broken because it sorts on pubdate.
>
> Then again, strptime didn't seem to be working correctly either:
> https://bugs.kde.org/show_bug.cgi?id=198443
> Though that might be a database configuration problem.
>
> On Tue, Nov 10, 2009 at 10:30, Mark Kretschmann <kretschmann at kde.org> wrote:
>> On Tue, Nov 10, 2009 at 9:22 AM, Bart Cerneels <bart.cerneels at kde.org> wrote:
>>> On Tue, Nov 10, 2009 at 08:33, Mark Kretschmann <kretschmann at kde.org> wrote:
>>>> commit b7edc70f7d853875b6d37ee6213f302f20c8b023
>>>> Author:     Mark Kretschmann <kretschmann at kde.org>
>>>> AuthorDate: Tue Nov 10 08:26:37 2009 +0100
>>>> Commit:     Mark Kretschmann <kretschmann at kde.org>
>>>> CommitDate: Tue Nov 10 08:26:37 2009 +0100
>>>>
>>>>    Replace strptime() with QDateTime. strptime() is Posix only!
>>>>
>>>>    Stecchino: Please check the format for correctness, I had to translate
>>>>    the different string tokens.
>>>>
>>>> diff --git a/src/podcasts/PodcastReader.cpp b/src/podcasts/PodcastReader.cpp
>>>> index 4130ee0..81f2967 100644
>>>> --- a/src/podcasts/PodcastReader.cpp
>>>> +++ b/src/podcasts/PodcastReader.cpp
>>>> @@ -340,16 +340,14 @@ PodcastReader::read()
>>>>  }
>>>>
>>>>  QDateTime
>>>> -PodcastReader::parsePubDate( const QString &datestring )
>>>> +PodcastReader::parsePubDate( const QString &dateString )
>>>>  {
>>>> -    struct tm tmp;
>>>> +    QDateTime pubDate;
>>>>
>>>> -    if( datestring.contains( ',' ) )
>>>> -        strptime( datestring.toAscii().data(), "%a, %d %b %Y %H:%M:%S %z", &tmp );
>>>> +    if( dateString.contains( ',' ) )
>>>> +        pubDate = QDateTime::fromString( dateString, "dddd d MMMM yyyy h:m:s" );
>>>>     else
>>>> -        strptime( datestring.toAscii().data(), "%d %b %Y %H:%M:%S %z", &tmp );
>>>> -
>>>> -    QDateTime pubDate = QDateTime::fromTime_t( mktime( &tmp ) );
>>>> +        pubDate = QDateTime::fromString( dateString, "d dddd MMMM yyyy h:m:s" );
>>>>
>>>>     return pubDate;
>>>>  }
>>>
>>> No, that is totally wrong. <pubDate> is in RFC822 format:
>>> http://rfc-ref.org/RFC-TEXTS/822/chapter5.html
>>>
>>> And the reason I'm not using QDateTime::fromString() is the timezone
>>> offset. GNU libc handles that very well. I guess we can ignore the
>>> timezone but that would make the pubDate a lot less useful for display
>>> purposes.
>>>
>>> I'm more than happy to accept a merge request implementing pubdate
>>> parsing completely using Qt. Already created a test because I planned
>>> to do it myself. Perhaps during the holiday tomorrow.
>>
>> Ok, the problem is just, we can't release 2.2.1 with the strptime() in
>> it. It won't compile on Windows.
>>
>> --
>> Mark Kretschmann
>> Amarok Developer
>> www.kde.org - amarok.kde.org
>>
>

v2.2.0-695-gf4b6bf5 is using KDateTime::fromString().

Since I have never found this in the past I'm a bit worried this is a
recent addition to kdelibs, so please test using KDE <4.3.

Bart


More information about the Amarok-devel mailing list