[Differential] [Commented On] D2345: use a separate configuration per look and feel

ivan added inline comments. INLINE COMMENTS > containment.cpp:197 > +//addWidget("org.kde.plasma.analogclock", {"x": 0, "y": 100, > "width": 300, "height": 400}); > +const QVariantMap geom = > context->argument(1).toVariant().value(); > +geometry = QRectF(con

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > ivan wrote in shellcorona.cpp:550 > I agree with Martin. If the user is crazy enough... > > D-Bus was never really meant to be something that the users should be calling > directly anyhow. yeah, agree so,, I'm thinking about exposing 2 function

[Differential] [Commented On] D2345: use a separate configuration per look and feel

ivan added inline comments. INLINE COMMENTS > mart wrote in shellcorona.cpp:550 > I hate this method. > tough, i need a way from the kcm to reset the current layout to the default. > it's quite dangerous and exporting a dbus method that effectively destroys > the current config is evil... > othe

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > shellcorona.cpp:550 > + > +void ShellCorona::reloadDefaultLayout() > +{ I hate this method. tough, i need a way from the kcm to reset the current layout to the default. it's quite dangerous and exporting a dbus method that effectively destroys the c

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:383 > No > > you need to loop through containments() rather than the views > (same for panel section) > > Otherwise: > > - you're only saving the currently plugged in screens. > - this won't save the conf

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:321 > If we go with this patch > > you should filter out ItemGeometries and AppletOrder here as you're making a > special case out of them. > Otherwise you're saving garbage data in the config which could conf

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:394 > I'm confused. > > Should you be getting the current from the current Plasma::Applet* instances > or are you loading state from the config file? I'm doing what i can there. every way to get items geometr

[Differential] [Commented On] D2345: use a separate configuration per look and feel

davidedmundson added inline comments. INLINE COMMENTS > shellcorona.cpp:394 > > //try to parse the items geometries > KConfigGroup genericConf(&contConfig, QStringLiteral("General")); I'm confused. Should you be getting the current from the current Plasma::Applet* in

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > mart wrote in shellcorona.cpp:469 > plasmshell should have a slot for that invoked by the kcm? i could send the notifychange signal of kglobalsettings, but it's kinda ugly, as it uses an integer as parameter to specify what changed, which is defined

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:511 > > can we have kconf_update that are guarantee to run before plasmashell is > > started? > > Good question. > > kconf_update also monitors for new scripts at runtime, which is another bug > waiting to h

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:469 > DBus signal from the KCM. > > It's how we do fonts, styles, colours etc. plasmshell should have a slot for that invoked by the kcm? REPOSITORY rPLASMAWORKSPACE Plasma Workspace REVISION DETAIL http

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:511 > > can we have kconf_update that are guarantee to run before plasmashell is > > started? > > Good question. > > kconf_update also monitors for new scripts at runtime, which is another bug > waiting to h

[Differential] [Commented On] D2345: use a separate configuration per look and feel

davidedmundson added inline comments. INLINE COMMENTS > mart wrote in shellcorona.cpp:469 > how would you notice knf has been changed? only info about it is that key in > kdeglobals DBus signal from the KCM. It's how we do fonts, styles, colours etc. > mart wrote in shellcorona.cpp:511 > can

[Differential] [Commented On] D2345: use a separate configuration per look and feel

mart added inline comments. INLINE COMMENTS > davidedmundson wrote in shellcorona.cpp:469 > why are we tracking file modifications? > That's not how the rest of the KCMs work. how would you notice knf has been changed? only info about it is that key in kdeglobals > davidedmundson wrote in shel

[Differential] [Commented On] D2345: use a separate configuration per look and feel

ivan added a comment. All in all, seems ok. I think we need to start commenting code like this - what it is doing. I do realize it is popular for the code to be self-documenting, but that has not been working for us that well :) Can you write short instructions on how to test this?