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
