No need to apologize, that's what code reviews are for :)

Not an ark issue. See https://mail.kde.org/pipermail/kde-buildsystem/2018-March/012418.html

Same, there is only one entry there.

In T9390#155256, @akrutzler wrote:

Apparently, I'm working on exactly this approach for the last week. :) I moved all the MTP handling to a KDED module. The KIO slaves then connects to this KDED module via dbus. It will still take some time to be finished but I get pretty promising results so far.
I will be at the Akademy today (16.08.18) at around 5pm, so if you are around we can discuss more on this topic. Sadly am not able to take my laptop with me to show you how it works so far...

In D13813#310067, @jtamate wrote:

In D13813#310047, @cfeck wrote:

Authored by a Bot?

Somehow my name and email address in .gitconfig got changed. How? No idea.
Should revert and push again with the right name?

In D13813#310047, @cfeck wrote:

Authored by a Bot?

Right. Then let's ship it as is for now, thanks!

I cannot reproduce the linked bug. If open the same file in both kate and kwrite I get only one entry in krunner (despite the file having two different X-KDE-LastOpenedWith values in .local/share/RecentDocuments/).

@jtamate Any updates on this? Can you use the new fastInsert() calls here?

Actually, the list is already const, so this is fine as is.

Looks good otherwise.

Please try to write unit test, if you can (have a look at dolphinmainwindowtest.cpp). That's the only way to make sure we don't break anything :)

Thanks for the patch. Just a couple of small nitpicks, looks good otherwise.

@pino does it look good to you now?

In D14443#299704, @hallas wrote:

Just out of curiosity, what is the motivation for this change? What do we gain by assigning pointers to nullptr? Would it instead make sense to use a smart pointer? Maybe unique_ptr or a Qt version?

But why doesn't the KItemListView have a parent? Who deletes listView then?

QPointer is not a smart pointer and doesn't handle memory. But we don't need it anyway, because we have the Qt::WA_DeleteOnClose attribute set which does the cleanup.

This will crash as soon as you try to edit the Trash place (because m_iconButton in PlacesItemEditDialog::icon() won't be initialized).

@tmarshall Do you have commit access?

+1

In D14199#294796, @hallas wrote:

In D14199#294501, @elvisangelaccio wrote:

The old code was like that for a reason: https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0

Now, the chance this change will lead to a crash is very low, but still I don't see what we gain either.

After reading the blog post and looking at the Qt documentation, I think you are absolutely right about this. The TreeViewContextMenu::open function ends up calling QMenu::exec which runs synchronously meaning that the application can close while processing events, what I do not understand about the original code is what good does the

if (contextMenu.data())

do? The whole point of QPointer is to provide memory management of QObjects, also in the case where the managed object is deleted elsewhere, therefore I can't see that it is needed.

The old code was like that for a reason: https://blogs.kde.org/2009/03/26/how-crash-almost-every-qtkde-application-and-how-fix-it-0

@hallas Thanks. Do you have commit access?

Currently we set the compression options only when adding new files

• Added bug report

Thanks for filing the upstream bug. Looks good to me now.

There is no bugreport, I'll have to file one.

Hmm, so the crash is in ucol_close() (aka in ICU) and we are working around it by forcing the cleanup in the KFileItemModel ctor (= single thread).

I guess this change makes sense (given that we are in the jpeg plugin), but what about QImageReader::decideFormatFromContent() ?

If no one has objections, I'll push it after the next tag.

@jtamate Can you show a complete gdb backtrace + valgrind log of this crash?

Isn't BUILD_TESTING already honored by ecm_add_tests ?

Thanks for the patience :)

I'm ok with the UI changes now.
Just some small nitpicks left and then I think we can ship this.

Impressive, I went from 18 seconds to 4 :O

In D13124#286115, @broulik wrote:

Check out KFileItemActions::addServiceActionsTo