[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-pim
Subject:    Re: [Kde-pim] [PATCH] Recurrences
From:       Reinhold Kainhofer <reinhold () kainhofer ! com>
Date:       2005-07-03 13:45:41
Message-ID: 200507031545.44833.reinhold () kainhofer ! com
[Download RAW message or body]

[Attachment #2 (multipart/signed)]


Am Samstag, 2. Juli 2005 10:38 schrieb David Jarvie:
> On Friday 01 Jul 2005 00:39, Reinhold Kainhofer wrote:
> > Hi guys,
> > Here is the patch for libkcal, which implements recurrences much better:
> > http://www.fam.tuwien.ac.at/~reinhold/KOrganizer/RecurrenceRule/
> >
> > I've also created the patches for the affected apps (KAlarm, Kitchensync,
> > KMobile, KOrganizer, KPilot and KResources). I haven't been able to test
> > them all, but at least they compile ;-)
>
> I won't be able to look at this properly until tomorrow, but here are a few
> random comments in the meantime:
>
> Recurrence::endDate() - this returns a date-time value, so its name isn't
> very intuitive, suggesting as it does that it returns a QDate.

Everything in recurrences works with QDateTime, and I didn't want to use 
"DateTime" all over the place, when the only place that really only uses a 
QDate is the method to get all recurrences times for a given day, and one 
convenience method.


> There are some new QPtrList members to hold RecurrenceRules. Given that
> QValueList is more compatible with Qt4, wouldn't it be better to use
> QValueList<RecurrenceRule*> to ease the pending transition?

That would mean that I also implement auto-deletion for them...

> It looks as if the compat code is being removed. If so, I don't think that
> it should be, since there must be people still using fairly old versions of
> KDE PIM who will benefit from this code when they eventually upgrade.

No, the code is not removed. Rather, it's now done in the proper libkcal way 
by a class CompatPre31 derived from the Compat class (after the loading is 
finished and before the class is used). From what I saw in the old compat 
code, which was spread out all over the recurrence class, the only problems 
were:
-) floating events were one day too short (i.e. an all day event on July 3 
needs to go from July3 0:00 until July 4 0:00, while pre-3.1 it had July3 
0:00 until July 3 0:00).
-) rDuration was set to the number of time periods to occur, not the number of 
actual occurences (with a special error in the weekly case).
-) dates of the year were stored as year day with a special case for leap 
years.


On a side note: I see that kalarm does its own compat functionality, and the 
libkcal CompatFactory doesn't check for kalarm's prodid. So all errors by 
wrong implementations in libkcal will not be caught by the libkcal compat 
code. Proably we shall switch our prodids to something of the form
PRODID:-//KDE//NONSGML KOrganizer 3.5 pre (libkcal 3.5 pre)//EN
instead of our current
PRODID:-//K Desktop Environment//NONSGML KOrganizer 3.5 pre//EN

The advantage would be that libkcal could detect the libkcal version and 
correct problems with previous libkcal versions, while we can also correct 
wrong usage of libkcal in applications.


> KAlarm's coding convention is to have no spaces inside round brackets.

Okay. I'm just so used to the libkcal and korganizer style that this happens 
automatically. Will change.

> The phrase "infinite recursion" occurs a number of times in Doxygen
> comments. It should read "infinite recurrence" (recursion is a different
> thing).

Oops, typos... Corrected locally. (Knot in my brain, which was then copied a 
few times).

> The spelling "occurence" appears a number of times in Doxygen comments. It
> should be "occurrences".

Typos, too... Corrected locally.


Reinhold
-- 
------------------------------------------------------------------
Reinhold Kainhofer, Vienna, Austria
email: reinhold@kainhofer.com, http://reinhold.kainhofer.com/
 * Financial and Actuarial Mathematics, TU Wien, http://www.fam.tuwien.ac.at
 * K Desktop Environment, http://www.kde.org/, KOrganizer / KPilot maintainer

[Attachment #5 (application/pgp-signature)]

_______________________________________________
kde-pim mailing list
kde-pim@kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
kde-pim home page at http://pim.kde.org/

[prev in list] [next in list] [prev in thread] [next in thread]