<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://svn.reviewboard.kde.org/r/6580/">http://svn.reviewboard.kde.org/r/6580/</a>
</td>
</tr>
</table>
<br />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks, the patch looks really nice. I'm not really commenting on it yet because I'd like to discuss the general approach first. Using "Ctrl+W" doesn't sound very good (it's usually associated with closing tabs and things like that, plus it is not advertised anywhere), and changing the default behavior of the activated() signal will confuse the users. IMO, you should add a context menu with the options for opening with the default application and choosing the application from a list (and maybe the Preview option too). Besides that, there could be a new setting for choosing the default behavior when an item is selected.</pre>
<br />
<div>
<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="http://svn.reviewboard.kde.org/r/6580/diff/1/?file=45312#file45312line579" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeutils/ark/part/part.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Part::slotPreviewExtracted(KJob *job)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">557</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ArkViewer</span><span class="o">::</span><span class="n">view</span><span class="p">(</span><span class="n">fullName</span><span class="p">,</span> <span class="n">widget</span><span class="p">());</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">574</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ArkViewer</span><span class="o">::</span><span class="n">view</span><span class="p">(</span><span class="n">fullName</span><span class="p">,</span> <span class="n">widget</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You probably need manually remove the file here now that it is not removed in ArkViewer.</pre>
</div>
<br />
<div>
<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="http://svn.reviewboard.kde.org/r/6580/diff/1/?file=45312#file45312line587" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdeutils/ark/part/part.cpp</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">void Part::slotPreviewExtracted(KJob *job)</pre></td>
</tr>
</tbody>
<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">582</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KRun</span><span class="o">::</span><span class="n">displayOpenWithDialog</span><span class="p">(</span><span class="n">lst</span><span class="p">,</span> <span class="n">widget</span><span class="p">());</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">You need to pass true as the third parameter so the file is automatically removed later.</pre>
</div>
<br />
<p>- Raphael</p>
<br />
<p>On March 4th, 2011, 4:07 p.m., peter velthuis wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdeutils.</div>
<div>By peter velthuis.</div>
<p style="color: grey;"><i>Updated March 4, 2011, 4:07 p.m.</i></p>
<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;">-double click/enter: opens with default viewer
-'preview' button: opens the viewer like it does now.
-ctrl+w: 'open with' dialog shows to choose the application to open with. (it's
a KAction, but not added to a context menu or anything (yet)).
other:
QFile::remove calls removed and centralized in the destructor by deleting KTempDirs.
It creates a folder per opened file, same behaviour as winrar. Why?
The problem is that you dont know when an external program closes (started through KRun) so we cant remove the file directly after the external program closes. So with one directory this scenario will happen: open file, (temp dir created and file extracted therre), user closes file, opens again .. and the message "File already exists" appears. Probably this is also the first reason why the current preview dialog was created=> we know when the user closes it.</pre>
</td>
</tr>
</table>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://bugs.kde.org/show_bug.cgi?id=179066">179066</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdeutils/ark/part/arkviewer.cpp <span style="color: grey">(1222951)</span></li>
<li>/trunk/KDE/kdeutils/ark/part/part.h <span style="color: grey">(1222951)</span></li>
<li>/trunk/KDE/kdeutils/ark/part/part.cpp <span style="color: grey">(1222951)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/6580/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>