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

[mart (Marco Martin)]

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 conflict; one of the new IDs could clash.
> 
> Also what's going to happen to activityId

yep, same thing for location and form factor i think

> davidedmundson wrote in shellcorona.cpp:394
> 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.

yes, it's always from a clean setup..
if you look back in the revision history, creating the applet with  a specified id was exactly what it was doing in the early revisions.
As this would have needed some ugly changes to the desktop containment, after a discussion with Eike we decided to go this route instead, as it doesn't expose directly the config file format in the scripting.

> davidedmundson wrote in shellcorona.cpp:394
> 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.

to me this feature is very desktop specific (so much that i even thought at some point loading it in a plugin only for the desktop shell...
I would consider to just ignore l&f changes on shells different from the desktop one, and even to not to dump config for containments different than the 3 default ones

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/20160809/ce749cbb/attachment.html>