It has been a little bit confusing that the prefix dir of the include does
not match the C++ namespace, as elsewhere.
Details
Details
- Reviewers
broulik
Existing runner code (e.g. plasma-workspace/runners) still builds,
with warnings in the log.
Runner code ported to new include path builds, without warnings in the log.
Diff Detail
Diff Detail
- Repository
- R308 KRunner
- Branch
- fixincludenamespacetoplasma
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
Anpther option I considered was to promote instead KRunner:: as C++ namespace, using a namespace alias for the time until KF6.
Unsure where KRunner is heading in the future and how decoupled from Plasma it should be at all (possible reuse in other systems?), I though opted to given variant for now to match the current situation. If only to get talk started :)
Comment Actions
I would actually prefer the class to be KRunner::AbstractRunner but that's obviously not something we could change.
Comment Actions
Okay. I will play with a patch for namespace aliasing then. Most tricky might be to make doxygen deal with it, let's see :)
Comment Actions
Bummer, no straight namespace aliasing possible: "Plasma::" namespace prefixed is used in some signal/slot signatures, and Qt's string-based signal/slot gets in the way here, moc cannot know about any possible aliases, so there is no support in the string normalization.
Besides, doxygen also needed some preprocessor hacks like the following, which at least make the code less pretty:
#ifdef DOXYGEN_SHOULD_SKIP_THIS
namespace KRunner
#else
namespace Plasma {}
namespace KRunner = Plasma;
namespace Plasma
#endif // DOXYGEN_SHOULD_SKIP_THISBadly done tools getting in the way :/
Too bad, I would have preferred to get rid of this namespace mismatch, it makes KRunner look so... not perfect ;) At least I tried, but seems this has to be delayed completly for KF6.