[Kde-pim] KCal::Calendar sortEvents

[Allen Winter]

On Friday 03 October 2008 03:26:59 Ron Goodheart wrote:
> On Thursday 02 October 2008 05:48:49 Allen Winter wrote:
> > On Wednesday 01 October 2008 00:22:25 ron wrote:
> > > Hi all,
> > > I was working on the issues identified in Bug 170894: 	"month printout is
> > > hard to read"...
> > > One of the problems is that the calendar event sorting 'sortEvents()' is
> > > not returning the event in date & time order -  only date.  While it's
> > > easy enough to fix this in the printing it will certainly pop up in
> > > several places and is better to take care of in one place.
> > >
> > > My thought was to add another eventSortField enum entry called
> > > EventSortStartDateTime.
> > >
> > > Question: is this the right thing to do?
> > >
> > > Using EventSortStartDate compares two KDateTime objects but the resulting
> > > sorted list is only in date & summary order (the first sort done).
> >
> > I think the right thing to do is fix EventSortStartDate to take the date &
> > time into consideration. My bad.  I wrote this stuff.
> >
> > _______________________________________________
> > KDE PIM mailing list kde-pim at kde.org
> > https://mail.kde.org/mailman/listinfo/kde-pim
> > KDE PIM home page at http://pim.kde.org/
> 
> Well, digging a little deeper...
> I think the sort code is actually okay, I think KDateTime is at fault. 
> It comes down to a slight difference in how KDateTime and QDateTime handle 
> date equality if one of the dates is a 'date only' type of date.
> 
> QDateTime considers the 'date only' to be less than a date that has a time.
> KDateTime does not because it only compares the dates (unless the time zones 
> are different then it changes the time if the first date is 'date only').  
> 
> The operator less-than code in KDateTime didn't look right so I ran a small 
> test with the following results:
> 
> dateOnly = "2008-10-02T00:00:00-07:00"
> datetime =  "2008-10-02T00:01:00-07:00"
> dateOnly isDateOnly? =  true
> datetime isDateOnly? =  false
> dateOnly <  datetime =  false
> dateOnly <= datetime =  true   <---
> dateOnly >  datetime =  false
> dateOnly >= datetime =  true   <---
> dateOnly == datetime =  false
> datetime <  dateOnly =  false
> datetime <= dateOnly =  true  <---
> datetime >  dateOnly =  false
> datetime >= dateOnly =  true  <---
> datetime == dateOnly =  false
> 
> This is clearly incorrect!
> 
> With the attached patch (with old code commented) I get:
> dateOnly = "2008-10-03T00:00:00-07:00"
> datetime =  "2008-10-03T00:01:00-07:00"
> dateOnly isDateOnly? =  true
> datetime isDateOnly? =  false
> dateOnly <  datetime =  true    <---
> dateOnly <= datetime =  true  <---
> dateOnly >  datetime =  false
> dateOnly >= datetime =  false
> dateOnly == datetime =  false
> datetime <  dateOnly =  false
> datetime <= dateOnly =  false
> datetime >  dateOnly =  true    <---
> datetime >= dateOnly =  true   <---
> datetime == dateOnly =  false
> 
> I don't understand why the code makes a 'date only' end-of-day unless there 
> was some kind of assumption of start-date and end-date? It does have an 
> interesting side effect, but clearly not desirable. 
> 
> Are there automated tests for KDateTime that I should be checking?
> 
Ron,
There is a unit test for KDateTime in kdelibs/kdecore/tests/kdatetimetest.c

Make sure you run cmake with -DKDE4_BUILD_TESTS=ON
Then you should be able to go in the kdelibs builddir/kdecore/tests
and run ./kdatetimetest

David Jarvie (who I'm sure is reading this) is the KDateTime guru.

-Allen

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