<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/120763/">https://git.reviewboard.kde.org/r/120763/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 24th, 2014, 3:28 a.m. IST, <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">Cool, thanks for looking into this! I think that many users will appreciate any improvements in this area.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I'm just wondering, do we really need two different signals which do quite similar things? I guess the 'old' signal would not be used any more, so maybe one could simply change the behavior of currentDirectoryChanged, such that it does what your new signal would do, and not introduce a new signal at all?</p></pre>
</blockquote>
<p>On October 24th, 2014, 5:16 p.m. IST, <b>Arjun AK</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">The new signal is only emitted when the PWD of the shell changes and not when it is changed by the running 'foreground' process. Wouldn't changing the behaviour break other apps?</p></pre>
</blockquote>
<p>On October 24th, 2014, 9:50 p.m. IST, <b>Frank Reininghaus</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">There are no other apps that use this signal. At least http://lxr.kde.org/ident?_i=currentDirectoryChanged&_remember=1 does not show any. Moreover, even if there were other apps, I don't think that there is a common use case where staying informed about temporary directory changes caused by "man ls", "git status", etc., is what the user wants.</p></pre>
</blockquote>
<p>On October 28th, 2014, 8:53 a.m. IST, <b>Kurt Hindenburg</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">If we want this in KDE 4.14.x, we can add signals but not remove them AFAIK.</p></pre>
</blockquote>
<p>On October 28th, 2014, 11:53 a.m. IST, <b>Arjun AK</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">ok, master then?</p></pre>
</blockquote>
<p>On October 28th, 2014, 6:39 p.m. IST, <b>Kurt Hindenburg</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<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;">I wouldn't mind trying to get this in KDE 4.14.x and then we can do add it in master (which is frameworks based).</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">we can add signals but not remove them AFAIK.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">We aren't removing the signal, but just changing the condition under which it gets emitted. Since dolphin is the only KDE application that uses it and since doing so would help fix a bug, can't we push this to 4.14?</p></pre>
<br />
<p>- Arjun</p>
<br />
<p>On October 27th, 2014, 6:49 p.m. IST, Arjun AK 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 Konsole, Emmanuel Pescosta and Frank Reininghaus.</div>
<div>By Arjun AK.</div>
<p style="color: grey;"><i>Updated Oct. 27, 2014, 6:49 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=305085">305085</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
konsole
</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;">Listening to the currentDirectoryChanged() can cause dolphin to act weird when you have programs running on the konsole part. for instance:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">1) <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">$ man ls</code> -> Dolphin temporarily switches to the manpages directory
2) <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">$ man ls</code>. Now quickly press q and CTRL D, you get stuck in the manpages directory.
3) <code style="text-rendering: inherit;color: #4444cc;padding: 0;white-space: normal;margin: 0;line-height: inherit;">(cd /;sleep .5; cd /tmp; sleep .5)</code>
4) When you have scripts running(like kdesrc-build), dolphin keeps on changing the directories</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">By adding a new signal, which is only emitted when the PWD of the shell is changed, these <a href="https://bugs.kde.org/show_bug.cgi?id=305085" style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">bugs</a> can be fixed.</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>src/Session.h <span style="color: grey">(6cd283a)</span></li>
<li>src/Session.cpp <span style="color: grey">(cf70c09)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/120763/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>