plasmashell has a different configuration for every lnf theme.
upon switching theme, tear down the whole shell and reload it
with the new configuration, either with the default config of
the lnf package or with the config previously stored.
the config file of the breeze theme still has the old for for
retrocompativility.
extra dependency: https://git.reviewboard.kde.org/r/128597/
Details
- Reviewers
davidedmundson ivan - Group Reviewers
Plasma - Commits
- R871:1e198e2c9d03: prettier javascript code
R871:d2f0ec63c60b: methods to switch lnf
R871:e0370b80c35f: prototype of JS config dump
R120:71eb75da019d: more explanatory comments
R871:0b91161b35da: createActivity(existingName) returns the existing one
R871:41f54319fc4a: Merge branch 'master' into mart/separateLookAndFeelLayout
R871:63bb74a7109c: remove the optional applet id parameter
R871:746884952c76: be silent
R120:d2f0ec63c60b: methods to switch lnf
R871:a697d291f98e: make sure desktop containments exist before script
R871:03d3bddf9f44: internal config migration
R871:669084d7a8fc: generate some comments in js
R871:3bdbdbcbc4ad: Merge branch 'master' into mart/separateLookAndFeelLayout
R871:f86d736def98: add a reloadDefaultLayout method to reset layout
R871:c22f0ce63e64: remove dead code
R871:f58f7f6faf85: support creating applets by id in script engine
R120:41f54319fc4a: Merge branch 'master' into mart/separateLookAndFeelLayout
R871:70c59c003f11: export the layout dump to js
R871:dbead6f45971: delete all graphics objects as well on switching
R871:3476cf0c53c8: Merge branch 'master' into mart/separateLookAndFeelLayout
R871:b03a21d6d6c7: crash--
R871:672c16ad0f8f: prototype for support of per-looknfeel layout
R120:2ddde5efc1e3: remove graphicsobject hack
R871:2ddde5efc1e3: remove graphicsobject hack
R871:57c758ad05dd: comments++
R871:312e61e42c3c: use a single config file again
R871:71eb75da019d: more explanatory comments
R871:2d57dfd63e74: use QRectF
R871:f36d882fea18: possible pass geometry to containment.addWidget()
R871:d628408a59f5: Merge branch 'master' into mart/separateLookAndFeelLayout
R871:83b0e5dfe153: more complete config js dumper
R871:97a6ac9b3ea6: use AppletOrder from the panel config
R871:914b9305e3a6: always use renamed file
R871:368254c79c72: delete views and arrays in the proper order
R871:998c1c940a9b: Merge branch 'master' into mart/separateLookAndFeelLayout
R120:312e61e42c3c: use a single config file again
R871:a42e154c6d9e: monitor for file creation
R120:c22f0ce63e64: remove dead code
R871:51e2072d7ba9: generic kconfiggroup dumper
R871:faba34ffd6bf: rework script generation
R871:8d2526f96f47: complete the dump of the panel
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- mart/separateLookAndFeelLayout
- Lint
No Linters Available - Unit
No Unit Test Coverage
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?
shell/scripting/containment.cpp | ||
---|---|---|
187 | It would be nice if we started putting comments with reasoning what this should do and similar in front of the blocks like these - it will be problematic to understand this code to future contributors | |
shell/shellcorona.cpp | ||
136 | Why is this captured? | |
325–326 | QList<KConfigGroup> groups{rootGroup}; | |
373 | I have a bad feeling about this. When will the generated JS code be used? |
shell/shellcorona.cpp | ||
---|---|---|
469 | how would you notice knf has been changed? only info about it is that key in kdeglobals | |
532 | can we have kconf_update that are guarantee to run before plasmashell is started? was also thinking about just using the same file and have the config to be lost on switch (that's actually what the vdg proposed initially) tough seems really too dangerous to me | |
741 | i'm thinking about adding a |
shell/shellcorona.cpp | ||
---|---|---|
469 | DBus signal from the KCM. It's how we do fonts, styles, colours etc. | |
532 |
Good question. kconf_update also monitors for new scripts at runtime, which is another bug waiting to happen. We might need to have some sort of: if (exists(plasma-shell-appletsrc) { move(plasma-shell-appletsrc , plasma-shell-lnf-appletsrc) } | |
741 | Might be nice, but that doesn't solve the thing that's the problem. You can't just randomly delete things owned by some other object. Knowing which objects owns what is a core part of good design. From what I can see:
delete the applet (which is literally the next line) and you delete this anyway. If this solves a problem, you're solving it in the wrong place. |
- prototype for support of per-looknfeel layout
- monitor for file creation
- delete views and arrays in the proper order
- be silent
- prototype of JS config dump
- more complete config js dumper
- generic kconfiggroup dumper
- complete the dump of the panel
- export the layout dump to js
- createActivity(existingName) returns the existing one
- support creating applets by id in script engine
- Merge branch 'master' into mart/separateLookAndFeelLayout
- prettier javascript code
- crash--
- delete all graphics objects as well on switching
- Merge branch 'master' into mart/separateLookAndFeelLayout
- make sure desktop containments exist before script
- Merge branch 'master' into mart/separateLookAndFeelLayout
- more explanatory comments
- possible pass geometry to containment.addWidget()
- generate some comments in js
- use QRectF
shell/shellcorona.cpp | ||
---|---|---|
469 | plasmshell should have a slot for that invoked by the kcm? |
shell/shellcorona.cpp | ||
---|---|---|
532 | what abouti do the config file move there in the c++ itself? |
- use AppletOrder from the panel config
- always use renamed file
- internal config migration
shell/shellcorona.cpp | ||
---|---|---|
469 | 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 in kdelibs4support I can add a new enum value, but it's still a dependency on kdelibs4support. |
shell/shellcorona.cpp | ||
---|---|---|
319–515 | I'm confused. Should you be getting the current from the current Plasma::Applet* instances or are you loading state from the config file? |
shell/shellcorona.cpp | ||
---|---|---|
319–515 | I'm doing what i can there. 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) |
shell/shellcorona.cpp | ||
---|---|---|
319–515 | If every way is a hack, then maybe this feature shouldn't go in at all. So the root issue is:
Brainstorming, there is an option. 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. | |
319–515 | No!!!! you need to loop through containments() rather than the views Otherwise:
and when you do do the system tray you're going to have a huge problem: | |
319–515 | what about globalConfig()? | |
319–515 | If we go with this patch you should filter out ItemGeometries and AppletOrder here as you're making a special case out of them. Also what's going to happen to activityId |
shell/shellcorona.cpp | ||
---|---|---|
319–515 | yes, it's always from a clean setup.. | |
319–515 | 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... | |
319–515 | yep, same thing for location and form factor i think |
shell/shellcorona.cpp | ||
---|---|---|
319–515 | can't for panels, as their geometry is saved in the view as it should be, because they have a separate geometry setting per screen. |
- prototype for support of per-looknfeel layout
- monitor for file creation
- delete views and arrays in the proper order
- be silent
- prototype of JS config dump
- more complete config js dumper
- generic kconfiggroup dumper
- complete the dump of the panel
- export the layout dump to js
- createActivity(existingName) returns the existing one
- support creating applets by id in script engine
- Merge branch 'master' into mart/separateLookAndFeelLayout
- prettier javascript code
- crash--
- delete all graphics objects as well on switching
- Merge branch 'master' into mart/separateLookAndFeelLayout
- make sure desktop containments exist before script
- Merge branch 'master' into mart/separateLookAndFeelLayout
- more explanatory comments
- possible pass geometry to containment.addWidget()
- generate some comments in js
- use QRectF
- use AppletOrder from the panel config
- always use renamed file
- internal config migration
- remove the optional applet id parameter
- remove graphicsobject hack
- rework script generation
shell/shellcorona.cpp | ||
---|---|---|
550 | I hate this method. |
shell/shellcorona.cpp | ||
---|---|---|
550 | You could do it like in KWin: make the KCM trigger a DBus signal which Plasmashell triggers to. In the end it's going to be the same: one can destroy the current config through dbus, but it's not that exposed. Also I don't think it matters: if a user decides to shoot himself in the foot by manually triggering the DBus signal, let him. |
shell/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. |
shell/shellcorona.cpp | ||
---|---|---|
550 | yeah, agree.... the first is not destructive the second is (just to not have a bool parameter) |
- prototype for support of per-looknfeel layout
- monitor for file creation
- delete views and arrays in the proper order
- be silent
- prototype of JS config dump
- more complete config js dumper
- generic kconfiggroup dumper
- complete the dump of the panel
- export the layout dump to js
- createActivity(existingName) returns the existing one
- support creating applets by id in script engine
- Merge branch 'master' into mart/separateLookAndFeelLayout
- prettier javascript code
- crash--
- delete all graphics objects as well on switching
- Merge branch 'master' into mart/separateLookAndFeelLayout
- make sure desktop containments exist before script
- Merge branch 'master' into mart/separateLookAndFeelLayout
- more explanatory comments
- possible pass geometry to containment.addWidget()
- generate some comments in js
- use QRectF
- use AppletOrder from the panel config
- always use renamed file
- internal config migration
- remove the optional applet id parameter
- remove graphicsobject hack
- rework script generation
- add a reloadDefaultLayout method to reset layout
- methods to switch lnf
- comments++
- Merge branch 'master' into mart/separateLookAndFeelLayout
shell/scripting/containment.cpp | ||
---|---|---|
197 | This seems not to be used |