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

List:       koffice-devel
Subject:    Re: Updated kpresenter dbus slideshow control patch
From:       James Hogan <james () albanarts ! com>
Date:       2008-10-29 19:48:43
Message-ID: 200810291948.43629.james () albanarts ! com
[Download RAW message or body]

(sorry if you get this email twice, the previous time i sent it with 
uncompressed attachments doesn't seem to have got through)

Hi Thorsten

Thanks for looking over my last patch + your comments.

> Thanks again for the patch. Due to it I found a bug in the presenter view.
> Fredy has just commited the fix so you might need to fix your patch.
great, I updated my patch (i assume it was just r876328 that affects me). I've 
also implemented your other suggestions.

I've attached a new patch (kpresenter_dbus_4.patch) which is based on beta 2 + 
r876328. For your convenience I've also attached the approximate changes 
since the previous patch (kpresenter_dbus_3_4_approx.patch).

Are you happy with the functionality this is implementing (the concerns you 
raised in your other email about dbus signals etc)?

Let me know if you have any other questions.

> +    updateActivePage( m_pages[page] );
>
> you should be using m_pageIndex instead of page. Even if they are the same
> it make it more clear.
(my changes in this area are no longer necessary with r876328 and have been 
removed)

> Is it possible to move that to the initialization list of the KPrView
> constructor?
yes, as long as it is initialised after m_presentationMode as signals from it 
are connected in the adaptor's constructor.

> This should use a static_cast as KPrView needs to be constructed with a
> KPrDocument. So it should be a KPrDocument all the time. Also I think that
> method could be const or? I'm not 100% sure. Maybe you can give it a try.
thank you. after 8 years of c++ coding I never realised that static_cast could 
be used to safely cast up the class heirarchy if the original type was 
definitely known!
/me remembers with dread all the unecessary dynamic_cast's i've ever used.

Cheers
-- 
James Hogan

["kpresenter_dbus_3_4_approx.patch.gz" (application/x-gzip)]
["kpresenter_dbus_4.patch.gz" (application/x-gzip)]

_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel


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