Create Task
Maniphest T15140

Move classes out of Plasma namespace
Open, Needs TriagePublic
Actions

Description

Currently all of the classes are part of the "Plasma" namespace. This is due to it being part of plasma-frameworks during the KDE4 era. However KRunner does not have anything to do with plasma-frameworks, except one deprecated and defunct method.

Moving the classes out of the namespace will cause source code incompatibilities. But by adding using namespace Plasma to our runners, we could already do the necessary changes to our runners now.

One could add such statement to the AbstractRunner header in a negated deprecation guard

+#if !KRUNNER_ENABLE_DEPRECATED_SINCE(5, 91)
using AbstractRunner = Plasma::AbstractRunner;
#endif

This way consumers don't need to add the statement themselves and clean it up after the KF6 port. With the deprecation guard we do not change the behavior of the header unless consumers increase their deprecation version. Based on my experience one rarely uses KRunner and plasma-frameworks in combination, which is why the using namespace statement should not cause issues.

Related Objects
Search...

alex created this task.Dec 29 2021, 7:28 PM
alex updated the task description. (Show Details)Jan 4 2022, 5:27 PM
alex moved this task from Backlog to In Progress on the KF6 board.Jan 10 2022, 7:35 PM
kossebau added subscribers: dfaure, kossebau.Jan 13 2022, 9:01 PM

Please strongly consider adding those generic names into a new namespace KRunner, instead of now introducing those names into the global namespace.

This will result in clashes everywhere where people happened to use QueryMatch, AbstractRunner, RunnerContext in their own namespace (or even as private global ones) and now get the unprefixed unnamespaced aliases as introduced by https://invent.kde.org/frameworks/krunner/-/commit/75fb5dffe9726aab7d1b04031621fa0b311ea017 visible in their compile units by whatever include chains.

@dfaure You might want tp comment on this as authority :)

kossebau added a comment.EditedJan 13 2022, 10:42 PM

In case more authority is needed, such names as introduced by the "using" would be what is called "global namespace pollution" :)

And then such aliases opens traps with Qt's introspection technology, which is string-based in some places, so people might run into issues when using the alias in their new code and wondering why some things do not work, wasting their and the support's time.

The namespace change is really not a big change to do on porting to KF6, should be mainly search&replace. No refactoring needed, no algorithm changes. Done in a few minutes.

That is not worth any troubles and complications now, Remember that the next Plasma LTS will need this code to be maintained for some years to come still. Is there anything bigger gained by using the new names already now? Please take the words from elderly people who have run into this before, the costs are not worth it.

Besides, why not put the new names into the KRunner namespace? (Does not fix the Qt string issues, but should be the KF6 target).

kossebau added a subscriber: broulik.EditedJan 16 2022, 9:51 PM

@alex Got some chance to reflect on this?

At least the namespaceless new alias should be fixed, because that is considered bad practice in general. KRunner might be the expected namespace here. Please handle this soonish, before too many might pick this up from the current development branch.

For the introduction of aliases, I would recommend not to do this due to risk of things not working, as mentioned. But something that can be fixed client side, other than the global namespace pollution (well, it can be worked around, but people will spell curses on the origin :) ).

Inviting @broulik as previous maintainer to also give his insightful 2 cents on the matter :)

alex added a comment.Jan 17 2022, 6:56 PM

https://invent.kde.org/frameworks/krunner/-/merge_requests/88

why not put the new names into the KRunner namespace?

As said on IRC, you have a point. And prefixing the classes with K might not be the ideal solution either.

alex moved this task from In Progress to Backlog on the KF6 board.Mar 15 2022, 6:28 PM
alex moved this task from Backlog to Waiting on KF6 Branching on the KF6 board.Mar 24 2022, 7:38 PM