<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/123274/">https://git.reviewboard.kde.org/r/123274/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin: 1em 0 0 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 6th, 2015, 2:29 nachm. UTC, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding: 0 0 0 1em;">
<br style="display: none;" />
<table bgcolor="#f0f0f0" cellpadding="5" cellspacing="5" style="border: 1px solid #c0c0c0; margin-bottom: 10px">
<tr>
<td>
<a href="https://git.reviewboard.kde.org/r/123274/file/2012/" style="color: black; font-weight: bold; font-size: 9pt;">tab_general.png</a>
<p>General Tab</p>
</td>
</tr>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Given this looks like advanced feature, I'd rather we hid it away so it wasn't the first thing that comes up in the config.
So I'm clear, what do you see the use case of this being?</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Well primarily I readded those configuration options because they were there originally, and looking at the bugreports also used. I guess they got lost in the port to kf5/qml. In the config xml they where still there and I guess some people want them back.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Personally I could imagine usecases like custom notifications - e.g. I'll take a short nap of about half an hour - after the timer is done please start playing some music - this could be set as a command I guess.
Other ideas instead of command execution? Could we hook something else into this?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Your argument about the advanced feature is of course correct - any suggestions? Rename the tab from "General" to "Advanced" (?) and move it down below the "Appearance" so the appearance tab is shown when opening the config dialog?</p></pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 6th, 2015, 2:29 nachm. UTC, <b>David Edmundson</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://git.reviewboard.kde.org/r/123274/diff/1/?file=360283#file360283line28" style="color: black; font-weight: bold; text-decoration: underline;">applets/timer/plugin/timer.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">28</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KProcess</span><span class="o">::</span><span class="n">startDetached</span><span class="p">(</span><span class="n">exec</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">QProcess::stateDetached("touch /tmp/asdf");</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">doesn't behave quite as you think it should, the args need to be in a second argument as a stringlist of one per argument.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">You'll need to do some splitting and such in here.</p></pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Oh I thought KProcess was the preferred way..</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Meh.. yeah this will need fixing of course..</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Thanks for the review :)</p></pre>
<br />
<p>- Bernhard</p>
<br />
<p>On April 6th, 2015, 1:11 nachm. UTC, Bernhard Friedreich wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Plasma.</div>
<div>By Bernhard Friedreich.</div>
<p style="color: grey;"><i>Updated April 6, 2015, 1:11 nachm.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
kdeplasma-addons
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Added tab "General" and "Appearance" in the config ui</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">General contains the bits about command execution after the timer has completed
Appearance contains the ability to hide seconds and showing the title</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The configuration option for showing the title is a bit buggy (again)
This bugreport explains the wrong scaling of the title: 304923</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I've converted this plasmoid from being qml only to c++ and qml because it looks like KProcess has no qml interface yet (and I didn't find any place where to start)</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Tested using plasmashell directly and using plasmoidviewer</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>applets/timer/plugin/timer.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/plugin/qmldir <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/plugin/timer.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/plugin/timerplugin.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/plugin/timerplugin.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/CMakeLists.txt <span style="color: grey">(c7cba10a96e6d3f1ba98a3aa755d6d3d8aad0c9f)</span></li>
<li>applets/timer/package/contents/config/config.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/package/contents/ui/configAppearance.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/package/contents/ui/configGeneral.qml <span style="color: grey">(PRE-CREATION)</span></li>
<li>applets/timer/package/contents/ui/timer.qml <span style="color: grey">(f675b93bda59eb161fe10b5f575eaa2082b822e0)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/123274/diff/" style="margin-left: 3em;">View Diff</a></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">File Attachments </h1>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/04/06/d21e3a9e-fef5-4d08-b5b4-1654a4b198b5__tab_general.png">General Tab</a></li>
<li><a href="https://git.reviewboard.kde.org/media/uploaded/files/2015/04/06/bca4649f-4d12-40d5-b801-7a4fc73943ba__tab_appearance.png">Appearance Tab</a></li>
</ul>
</td>
</tr>
</table>
</div>
</body>
</html>