[Kde-pim] Review Request: Remove duplicated code in KOAgendaView::changeIncidenceDisplayAdded()

[Kevin Krammer]

> On 2009-02-08 11:38:11, Kevin Krammer wrote:
> > trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.cpp, line 1324
> > <http://reviewboard.kde.org/r/38/diff/2/?file=357#file357line1324>
> >
> >     since this was an if-then-else construct before, have you checked it wasn't there to have updateEventIndicators() between removeIncidence() and changeIncidenceDisplayAdded() and not after the latter?
> 
>  wrote:
>     yes, it doesn't even work if it's before changeIncidenceDisplayAdded().
>     
>     Try this:
>     1. Open korganizer (unpatched) in week view
>     2. Scroll all up until only 00h-9am (9, 10, 11 will do) is visible.
>     3. Make a new event and set it's time to 22h-23h (so that the event isn't visible).
>     
>     You won't see any green arrows (event indicators) unless you refresh the whole view.
>     
>     I should have posted this little change in a separate review.
>     
>

Ah, ok. I was just wondering whether that change was valid.
Just a code level review, didn't test anything :)


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/38/#review52
-----------------------------------------------------------


On 2009-02-08 11:08:40, Sergio Martins wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/38/
> -----------------------------------------------------------
> 
> (Updated 2009-02-08 11:08:40)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> This patch makes KOAgendaView::changeIncidenceDisplayAdded() share code with KOAgendaView::fillAgenda().
> 
> changeIncidenceDisplayAdded() has some bugs displaying multi-day recurrent events and multi day events whose start isn't visible, these bugs 
> were all fixed in fillAgenda recently.
> 
> Both these functions have the same goal, the difference is that changeIncidenceDisplayAdded only shows one incidence, fillAgenda shows them all, so I created a new private function, displayIncidence(), which is shared by both functions and implements all the recurrence/multi-day logic.
> 
> 
> This addresses bugs 99634 and 108443.
>     https://bugs.kde.org/show_bug.cgi?id=99634
>     https://bugs.kde.org/show_bug.cgi?id=108443
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.h 923377 
>   trunk/KDE/kdepim/korganizer/views/agendaview/koagendaview.cpp 923377 
> 
> Diff: http://reviewboard.kde.org/r/38/diff
> 
> 
> Testing
> -------
> 
> Tested with recurring events, recurring multi-day events, normal events, all day events, some of them without a visible start.
> 
> 
> Thanks,
> 
> Sergio
> 
>

_______________________________________________
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/