Minutes Monday Plasma hangout
David Edmundson
david at davidedmundson.co.uk
Mon Feb 3 17:07:51 UTC 2014
> sebas:
> - sebas/themeswitch branch allows switching themes, almost good to merge:
> - Blur should depend on theme values
> - styled icons don't change on theme changes (notmart will have a look)
>
I did a code review of the branch.
A few nitpicks
Missing & in src/plasma/package.cpp
+ foreach (const QString pack, entries) {
---------------------------
1fedbb587c7a94674484df0fe57d24299d000a06
avoid committing commented out code (the qDebugs)
---------------------------
+ if (!themeName.isEmpty()) {
+ Plasma::Theme *t = new Plasma::Theme(this);
+ t->setThemeName(themeName);
+ }
This API sucks a bit. (not the fault of your branch)
Changing a member of an object, should not affect lots of instances of
that object.
Can I make this a static method in Plasma::Theme instead?
Whilst I'm here:
- setUseGlobalSettings does nothing. It unsets the theme name.. but
then if you call setThemeName you'll then change things globally as
it's still the same d pointer.
- why do we do our own ref counting? QHash<QString,
QSharedPtr<ThemePrivate>> themes; would make all code simpler.
David
More information about the Plasma-devel
mailing list