That is excellent! But in the implementation, mind the "multiple kxmlgui-clients" use case, like konqueror or kdevelop (or kate I guess). It can make things tricky...
(you drop between two clients, how do you know which one should memorize the action into its kxmlgui file?)

Why is this not in the same commit as the related unittest, as is common practice?

The description says this depends on D8773 which isn't approved.

Why does KRun duplicate all of the (new) code from DesktopExecParser, when DesktopExecParser is actually a helper class for KRun?
I would expect it to have solved all this already, unless I'm missing something about the various code paths.
The code you changed in krun.cpp was supposed to simply resolve desktop:/foo to file:///bleh but everything else about %u/%f is done by DesktopExecParser.

OK, please give me a day or two to review this, it shouldn't go in before Saturday anyway so we have one month of testing in git before release

Right, we are going too far. What we want to deprecate is desktop files pointing to c++ plugins.
And therefore the class that loads such plugins.

No reconstructing-call-history-via-a-boolean-member, I like this! :-)

So after a double-click, activated() is emitted, the boolean is set, and much later some Key_Enter event that should have been emitted, gets eaten?

I'm not smarter than you, but I consider this code hacky :-)

About your very first point: for information, lxr.kde.org also relies on kdesrc-build to check out all the relevant sources. This has been working rather well for many years, I don't see that as "unwanted coupling of tools".
But OK, the rest of your email makes that point moot anyway :-)

This patch itself is still correct though, of course.

Urgh. bool members as used here to share state between methods, with the assumption that they're always called together and in a certain order, are very brittle.

Wow I had never seen KIconUtils. This is very nice stuff (because very independent from anything else).

So many hardcoded numbers! Much better indeed.

add missing "get" (this bug was in kmimetypetrader.h already, blatant proof of copy/pasting... ;-) )

https://lxr.kde.org/ident?_i=writeServiceTypeProfile&_remember=1 -> completely unused

This makes perfect sense to me, thanks for the detailed analysis and the fix.

Then write your own, it's easy:

OK then we could deprecate KDE::icon(1 arg), and keep KDE::icon(2 args)

In my opinion it could be done, yes. The common plasma use case (linux distro desktop) installs everything and the specialized setups probably don't need the KCM, or can install it. On the other hand there's a ton of stuff we can do before branching for KF6, so we could also wait until after the branching for the move. We can't really change KIO's dependencies before that anyway.

I like the idea of moving the KCMs to kio-extras. They are indeed extra functionality.

A unittest would be very welcome.

My head hurts a bit but I think I understand this now ;)

Thanks for the fix, sorry for not noticing.

I think this needs more work, we lost some tolerance here, AFAICS.

It would help if I wasn't a complete idiot :)

Restarting kdeinit is only needed when making changes that affect kioslaves.
Here it's just about a widget, all you need is to start the application from a terminal.

@ngraham AFAIK gnome has a trick where a fuse mount is created, its path is passed to the application being started, and the application, if it supports gvfs, re-translates that into a URL and uses that instead if it makes more sense. This way "dumb" apps get a local file (with all the limitations of doing synchronous I/O over the network) and network-transparent applications use URLs.
On the other hand, the KDE logic is "if the app takes %f and not %u in the Exec line, it doesn't support remote URLs, so we need to download the file first" (that's done by kioexec). If you see a "download first" check if kioexec is running. But if it's the app doing it, then I have no idea.

I approve, given that I wrote the same in D25643 :-)

I started a kdesrc-build with the method removed, and found that KFileDialog::toolBar (in kdelibs4support) uses it, so it needs a matching #if in case EXCLUDE_DEPRECATED_BEFORE_AND_AT.

Note that calligra/filters/karbon/pdf/PdfImport.cpp is broken by this as well.

Now that I see "The signals will still be emitted of course" I'm no longer sure.
Trapping sounds like the event is completely eaten and the signal isn't emitted.

We must keep compatibility, yes. That's the promise of KDE Frameworks. There *is* more KF5-based code outside lxr.kde.org.

This is also a good candidate for upstreaming the feature into Qt... as QLineEdit::setTrapReturnKey for instance. Maybe we should try that first...

This could be done externally by an event filter.

Hmmpf, indeed. I forgot about the objection I had here :)