<table><tr><td style="">mart added a comment.
</td><a style="text-decoration: none; padding: 4px 8px; margin: 0 8px 8px; float: right; color: #464C5C; font-weight: bold; border-radius: 3px; background-color: #F7F7F9; background-image: linear-gradient(to bottom,#fff,#f1f0f1); display: inline-block; border: 1px solid rgba(71,87,120,.2);" href="https://phabricator.kde.org/D3166" rel="noreferrer">View Revision</a></tr></table><br /><div><div><blockquote style="border-left: 3px solid #8C98B8;
          color: #6B748C;
          font-style: italic;
          margin: 4px 0 12px 0;
          padding: 8px 12px;
          background-color: #F8F9FC;">
<div style="font-style: normal;
          padding-bottom: 4px;">In <a href="https://phabricator.kde.org/D3166#58841" style="background-color: #e7e7e7;
          border-color: #e7e7e7;
          border-radius: 3px;
          padding: 0 4px;
          font-weight: bold;
          color: black;text-decoration: none;" rel="noreferrer">D3166#58841</a>, <a href="https://phabricator.kde.org/p/davidedmundson/" style="
              border-color: #f1f7ff;
              color: #19558d;
              background-color: #f1f7ff;
                border: 1px solid transparent;
                border-radius: 3px;
                font-weight: bold;
                padding: 0 4px;" rel="noreferrer">@davidedmundson</a> wrote:</div>
<div style="margin: 0;
          padding: 0;
          border: 0;
          color: rgb(107, 116, 140);"><p>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).</p></div>
</blockquote>

<p>yeah, if no containment wrapper instances exist, this may not get executed.<br />
I think it should probably be attempted when tearing down the whole scripting engine stuff instead.</p>

<p>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</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>

<p>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.<br />
existing scripts in 5.8 has to work, so yeah.<br />
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)</p>

<blockquote style="border-left: 3px solid #a7b5bf; color: #464c5c; font-style: italic; margin: 4px 0 12px 0; padding: 4px 12px; background-color: #f8f9fc;"><p>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.</p></blockquote>

<p>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</p></div></div><br /><div><strong>REPOSITORY</strong><div><div>rPLASMAWORKSPACE Plasma Workspace</div></div></div><br /><div><strong>REVISION DETAIL</strong><div><a href="https://phabricator.kde.org/D3166" rel="noreferrer">https://phabricator.kde.org/D3166</a></div></div><br /><div><strong>EMAIL PREFERENCES</strong><div><a href="https://phabricator.kde.org/settings/panel/emailpreferences/" rel="noreferrer">https://phabricator.kde.org/settings/panel/emailpreferences/</a></div></div><br /><div><strong>To: </strong>mart, Plasma, davidedmundson<br /><strong>Cc: </strong>davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas<br /></div>