[KPhotoAlbum] Patch for proper "Open With..."

[Andreas Neustifter]

Hi Jan!

Thanks for the comments!

On 18 October 2010 04:32, Jan Kundrát <jkt at gentoo.org> wrote:

> Hi Andreas, thanks for your patch, unfortunately I can't really commit it,
> reasons are below.
>
> +    // check for the special entry for self-defined
>> +    if (name == i18n("Your Command Line")) {
>> +
>> +        static RunDialog* dialog;
>> +        dialog = new RunDialog(MainWindow::Window::theMainWindow(),
>> _list);
>> +        dialog->show();
>> +
>> +        return;
>> +    }
>>
>
> Sorry, but this is just ugly. I haven't checked the code (sorry for that),
> but please do the check based on the QAction* action's value and not the
> actual string data (that might involve keeping track of the pointer value's
> somewhere if the list of QActions is somehow runtime-generated). I realize
> that's from the old code, but I still do not like it at all.
>
> Also note that your code leaks memory, unless there's a proper
> deleteLater() connected to the dialog's slots (and if it was, current code
> would be broken). Current version won't leak, as the constructor is called
> exactly once (the GUI runs in one thread, so that's not an issue here),
> while you call it each time it's needed, not destroying the dialog upon its
> completion. If you go that way, there's also no need to define the local
> variable as static, either.
>

I have reworked the code so that no memory leaks any more by setting the
image list anew each time the dialog is used. I will not clean up current
code, thats for another time.


>
> -       return;  //user clicked the title entry. (i.e: "All Selected
>> Items")
>> +        return;  //user clicked the title entry. (i.e: "All Selected
>> Items")
>>
>
> Please do not mix whitespace cleanup/changes with real changes.
>

Okay.

Andi
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20101019/5a2bcded/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: always_set_new_image_list_in_run_dialog.patch
Type: text/x-patch
Size: 1921 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kphotoalbum/attachments/20101019/5a2bcded/attachment.bin>