[Differential] [Requested Changes To] D2345: use a separate configuration per look and feel

[davidedmundson (David Edmundson)]

davidedmundson requested changes to this revision.
davidedmundson added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> shellcorona.cpp:321
>  
>  QString dumpconfigGroupJS(const KConfigGroup &rootGroup, const QString &prefix)
>  {

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 conflict; one of the new IDs could clash.

Also what's going to happen to activityId

> shellcorona.cpp:383
>      int i = 0;
>      foreach (DesktopView *view, m_desktopViewforId.values()) {
>          Plasma::Containment *cont = view->containment();

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 configuration for any applets in a system tray.

and when you do do the system tray you're going to have a huge problem:
system tray writes out SystrayContainmentId... you aren't going to know what that is.

> mart wrote in shellcorona.cpp:394
> I'm doing what i can there.
> every way to get items geometries is actually an hack, i think reading the config file is a bit more reliable, even if assumes how the config file is.
> otherwise it could get to the applet graphics object then map its geometry with  containment graphics object geometry (however implementation detail, it would introduce an error as the geometry of the margins of the framesvg applet background wouldn't be included then)
> 
> i can go for either of the approaches, none of them is perfect, but don't have strong preference there (while I do for the panel, as i think is the only way to ensure proper applets order)

If every way is a hack, then maybe this feature shouldn't go in at all.

So the root issue is:

- for saving/restory applet geometry is handled by the containment which is in completely arbitrary as it's done by that containment plugin.
- they use the ID of the applet for an index
- ID won't be the same

Brainstorming, there is an option.
*if* we assume dump and resume is always going to be from a clean setup we could just expose setting initial id to applet scripting. It's already in Plasma::Containment. it would fix all the problems without any hacks.

-----

I can imagine this patch will destroy PMC if someone switched LNF twice as you're hardcoding stuff in plasma-workspace based on behaviour of plasma-desktop.

> shellcorona.cpp:467
>          //enumerate config keys for containment
>          KConfigGroup contConfig = cont->config();
>          script += "    //Panel containment configuration\n";

what about globalConfig()?

REPOSITORY
  rPLASMAWORKSPACE Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D2345

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mart, davidedmundson, #plasma
Cc: davidedmundson, ivan, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160808/d99131e4/attachment-0001.html>