if the containment actions plugins have been
configured in the startup script, their config must be
reloaded when the script ends, otherwise during the plasma
session the config will be outdated
BUG:371704
Details
Details
- Reviewers
davidedmundson - Group Reviewers
Plasma
the patch has been tested on new sessions with a customized look and feel package (maui) that changes the visible context menu entries
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- arcpatch-D3166
- Lint
No Linters Available - Unit
No Unit Test Coverage
Comment Actions
If the containment actions plugins have been
configured in the startup script, their config must be
reloaded when the script ends,
That part makes sense....but why is this code in the containment destructor?
Comment Actions
Edit, I think I understand.
So the script is modifying the actions not through some actions part of the API, but through just manually prodding the config.
Manually prodding the config naturally has no effect at runtime changes.
This isn't a very thorough solution.
You're covering containment actions being reconfigured, but not covering them being added or removed - an equally plausible thing to script.
Also this script is making an optimistic assumption that in the script where you might be modding containment configs, you'd create a containment instance. That's not always true (like from the evaluate script DBus calls).
Maybe do this for 5.8, but you really need to expose a new method (or even Actions object) to do this properly - or to delay the loading.
BTW: The script in the bug report has a bug, it's only going to configure the actions on containment 0; even though you'll be reconfiguring the "correct" one.
Comment Actions
yeah, if no containment wrapper instances exist, this may not get executed.
I think it should probably be attempted when tearing down the whole scripting engine stuff instead.
in order to make adding actions (or changing existing ones) working, a loop of Containment::setContainmentActions should be executed looping trought the config that just has been written
Maybe do this for 5.8, but you really need to expose a new method (or even Actions object) to do this properly - or to delay the loading.
I would really like to avoid creating any new api with this qtscript stuff, given the state of qtscript, but I think is feasible to make the current manual config poking fully work.
existing scripts in 5.8 has to work, so yeah.
i don't see possible delaying the loading, as the desktop containment has to already exist by the time the script is executed (see the long story of fixes of scripting during the early 5.7)
BTW: The script in the bug report has a bug, it's only going to configure the actions on containment 0; even though you'll be reconfiguring the "correct" one.
no, that group is not the containment id, it is the containment type, so [ActionPlugins][0] means plugins for desktop containments, [ActionPlugins][1] for panel containments (enum ContainmentType in plasma.h) so that script correctly changes actions for "desktop" containments
Comment Actions
in this version, i'm rebuilding everything on scriptengine teardown, this should make work correctly even if one adds a new plugin, or removes one.
Comment Actions
It's still a bodge round broken user code.
ConfigGroup.write() can't be expected to update anything on the fly, that's not how config works, and it's not how any of the rest of the scripting works. Config API even allows you to open a different file that might not even be part of plasmashell.
Updating just this one weirdly specific action group on script closure makes no design sense.
Comment Actions
it is, but the thing is, that's the way is supported by the api right now, and there are users for it, so it has to work, regardless if it's quite ugly.
i can't really see much other ways, either that or put a watcher on the config update...