On Fri, Aug 7, 2009 at 2:56 AM, Aaron J. Seigo<ase...@kde.org> wrote:
> On Thursday 06 August 2009, Shantanu Tushar Jha wrote:
>> Hi,
>> I had fixed the non-appearance of Open Picture... option in Picture
>> Frame's right-click menu ( r990567and r991008 ). But weirdly,
>> sometimes there are two instances of the Open Picture... option. But,
>> the code does check for it that multiple options aren't added (using
>> the variable m_menuPresent ) -
>>
>> if (hasAuthorization("LaunchApp") && ! (m_menuPresent || m_potd ||
>> (m_currentUrl.path() == "Default" && m_mySlideShow->currentUrl() ==
>> "Default")))
>
> i assume the missing '{' is just an accident :)
>
> anyways, the logic of this if statement is pretty complex. personally, i'd
> write it like this:
>
> const bool invalid = m_currentUrl.path() == "Default" &&
> m_mySlideShow->currentUrl() == "Default";
This was already done in a following commit, I pasted the wrong one.
>
> if (m_potd || invalid) {
> delete m_openPicture;
> m_openPicture = 0;
> } else if (!m_openPicture && hasAuthorization("LaunchApp")) {
> ...
> }
>
> does the same thing, but so much easier to read and understand imho. note that
> this also gets rid of the need for m_menuPresent.
>
Yes, thats really elegant and it works, after some testing, I'll commit it.
>> I tried a lot to figure it out, but the maximum i can guess is that
>> the code is getting run twice simultaneously.
>
> that's not possible. maybe put some debug input in there that shows the values
> of what's being used in the if/else statements.
>
> --
> Aaron J. Seigo
> humru othro a kohnu se
> GPG Fingerprint: 8B8B 2209 0C6F 7C47 B1EA EE75 D6B7 2EB1 A7F1 DB43
>
> KDE core developer sponsored by Qt Software
>
> _______________________________________________
> Plasma-devel mailing list
> Plasma-devel@kde.org
> https://mail.kde.org/mailman/listinfo/plasma-devel
>
>
--
Shantanu Tushar (UTC +0530)
http://www.shantanutushar.com
_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel
