<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/5443/">http://svn.reviewboard.kde.org/r/5443/</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 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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;">i agree with David Jarvie that removing the information about the file being newer/older/etc from the top label is a step backwards: it removes useful information, instead requiring the user to figure it out on their own using displayed dates. i do agree that it makes sense to move the potentially loooong file names down to the representations themselves, however, under the 'source' and 'destination' labels as that makes it clearer which is which and the informational text at the top easier to parse.

however (looking at the screenshots only, haven't rebuilt with the patch to try it first hand), it now has textual information both above and below the preview icon and the new information above (besides not using KLocale, see inline comments below) is visually formatted differently from the rest of the informational text at the bottom. this really stands out because the text below is formatted so nicely. since the modification time information should already be below the preview icon, i don't see the point of duplicating it above as well. perhaps if it was just the file name it would look fine; i'd leave the metadata to the to the UI that is already providing it below the preview.

(btw, something that would be slick is to bind the value of the vertical scrollbars together, or perhaps even better just provide one vertical scrollbar that controls both file views simultaneously. right now to compare metadata, i have to scroll both bars which is a lot of back-and-forth mousing; i'm not sure there is any real use case for "look at the number of words in one text file, while viewing the content of the other" in this dialog, since it is really all about comparing the two files to decide what to do about overwriting the file.)</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;">"haven't rebuilt with the patch to try it first hand"
Unfortunately, you can't just yet. I had to add a new function, which meant that I had to update the header file as well (renamedialog.h). I'm not sure how I can upload the new header file as a diff to this same review request.

"i'd leave the metadata to the to the UI that is already providing it below the preview."
I did not notice the duplication, and the QLabel's I added originally with this duplication have been removed for the next revision.

"provide one vertical scrollbar that controls both file views simultaneously"
I completely agree, but I'm not sure how to do this. Is it possible to have one container with two images side-by-side within it?</pre>
<br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line242" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">RenameDialog::RenameDialog(QWidget *parent, const QString & _caption,</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">240</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">QLabel</span><span class="o">*</span> <span class="n">titleLabel</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KSqueezedTextLabel</span><span class="p">(</span> <span class="n">sentence1</span><span class="p">,</span> <span class="k">this</span> <span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">235</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">QFont</span> <span class="n">font</span> <span class="o">=</span> <span class="n">titleLabel</span><span class="o">-></span><span class="n">font</span><span class="p">();</span></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">236</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">font</span><span class="p">.</span><span class="n">setBold</span><span class="p">(</span> <span class="kc">true</span> <span class="p">);</span></pre></td>
  </tr>

  <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">237</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">titleLabel</span><span class="o">-></span><span class="n">setFont</span><span class="p">(</span> <span class="n">font</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;">probably should not be bold; bold text tends to have the effect of making everything "heavier" visually without actually improving actual readability when applied to whole sentences like this.</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;">I prefer bold myself so the user can't ignore the message. But I understand what you are getting at. I changed it so you can see how it looks like when I upload a screenshot.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line258" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">RenameDialog::RenameDialog(QWidget *parent, const QString & _caption,</pre></td>

  </tr>
 </tbody>



 
 




 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">247</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">gridLayout</span><span class="o">-></span><span class="n">addWidget</span><span class="p">(</span> <span class="n">srcTitle</span><span class="p">,</span> <span class="mi">1</span><span class="p">,</span> <span class="mi">0</span> <span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">250</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">struct</span> <span class="n">tm</span> <span class="o">*</span><span class="n">srcTimeInfo</span> <span class="o">=</span> <span class="n">localtime</span><span class="p">(</span> <span class="o">&</span><span class="n">mtimeSrc</span> <span class="p">);</span></pre></td>
  </tr>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">248</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">gridLayout</span><span class="o">-></span><span class="n">addWidget</span><span class="p">(</span> <span class="n">srcWidget</span><span class="p">,</span> <span class="mi">2</span><span class="p">,</span> <span class="mi">0</span> <span class="p">);</span></pre></td>
    <th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">251</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">struct</span> <span class="n">tm</span> <span class="o">*</span><span class="n">destTimeInfo</span> <span class="o">=</span> <span class="n">localtime</span><span class="p">(</span> <span class="o">&</span><span class="n">mtimeDest</span> <span class="p">);</span></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">252</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">strftime</span><span class="p">(</span> <span class="n">srcTimeChars</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span> <span class="n">srcTimeChars</span> <span class="p">),</span> <span class="s">"%a %Y-%m-%d %H:%M:%S %Z"</span><span class="p">,</span> <span class="n">srcTimeInfo</span> <span class="p">);</span></pre></td>
  </tr>

  <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">253</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">strftime</span><span class="p">(</span> <span class="n">destTimeChars</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span> <span class="n">destTimeChars</span> <span class="p">),</span> <span class="s">"%a %Y-%m-%d %H:%M:%S %Z"</span><span class="p">,</span> <span class="n">destTimeInfo</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;">this should be done using KLocale, not hand crafted strings.</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;">Good idea, but since I remove the duplication of information, all of this has been deleted and cleaned up. I love cutting code :)</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line271" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">RenameDialog::RenameDialog(QWidget *parent, const QString & _caption,</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">263</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">gridLayout</span><span class="o">-></span><span class="n">addWidget</span><span class="p">(</span> <span class="n">blankLabel</span><span class="p">,</span> <span class="mi">1</span><span class="p">,</span> <span class="mi">0</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;">use spacers instead of empty QLabels?</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;">Whats the name of the class for the spacers? Under what library? I would like to use them as well since QLabels are only workarounds for this kind of situation.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line302" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">RenameDialog::RenameDialog(QWidget *parent, const QString & _caption,</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">294</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">QLabel</span> <span class="o">*</span><span class="n">lb1</span> <span class="o">=</span> <span class="k">new</span> <span class="n">QLabel</span><span class="p">(</span> <span class="s">""</span><span class="p">,</span> <span class="k">this</span> <span class="p">);</span></pre></td>
  </tr>

  <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">295</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">pLayout</span><span class="o">-></span><span class="n">addWidget</span><span class="p">(</span> <span class="n">lb1</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;">why an empty label?</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;">It needs to be a spacer instead to separate the the previews and the rename textbox. This does create blank space, so it not completely necessary. See the *New screenshots to see how it looks.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line305" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">RenameDialog::RenameDialog(QWidget *parent, const QString & _caption,</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">297</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">QString</span> <span class="n">sentence2</span> <span class="o">=</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"Rename:"</span> <span class="p">);</span></pre></td>
  </tr>

  <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">298</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
  </tr>

  <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">299</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">QLabel</span> <span class="o">*</span><span class="n">lb2</span> <span class="o">=</span> <span class="k">new</span> <span class="n">KSqueezedTextLabel</span><span class="p">(</span> <span class="n">sentence2</span><span class="p">,</span> <span class="k">this</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;">why is this squeezed? it's one word and a widget label, it should just be a normal QLabel, and it should be on the same line as the line edit so it is evident what it is referring to.

(a stylistic comment: "sentence2" is a bit of an indirect name for such a thing; why not just put the i18n call right into the consructor: new QLabel(i18n("Rename"), this);?)</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;">I think I have cleaned up most of the simpler situations where code should have been combined. This was missed though. The KSqueezedTextLabel was changed to QLabel as well.</pre>
<br />

<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On October 4th, 2010, 5:17 p.m., <b>Aaron Seigo</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="/r/5443/diff/4/?file=38848#file38848line628" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kio/kio/renamedialog.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 4)

    </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; ">QScrollArea* RenameDialog::createContainerLayout(QWidget* parent, const KFileItem& item, QLabel* preview)</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">619</font></th>
    <td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="k">if</span> <span class="p">((</span><span class="n">text</span><span class="p">.</span><span class="n">compare</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Source"</span><span class="p">)</span> <span class="p">)</span> <span class="o">==</span> <span class="mi">0</span><span class="p">)</span> <span class="o">||</span> <span class="p">(</span><span class="n">text</span><span class="p">.</span><span class="n">compare</span><span class="p">(</span> <span class="n">i18n</span><span class="p">(</span><span class="s">"Destination"</span><span class="p">)</span> <span class="p">)</span> <span class="o">==</span> <span class="mi">0</span><span class="p">))</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;">personally, i'd think it would make more sense just add a boolean to the signature of this method instead of comparing strings, which is easy to break in future.

this, and the rest of the private methods, should be moved to RenameDialogPrivate, as well.</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;">string comparison has been changed to boolean comparison, but I'm not sure how to implement the last suggestion though.</pre>
<br />




<p>- Steven</p>


<br />
<p>On October 3rd, 2010, 1:38 a.m., Steven Sroka wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/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 kdelibs.</div>
<div>By Steven Sroka.</div>


<p style="color: grey;"><i>Updated 2010-10-03 01:38:23</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;">This is my first submission to KDE. If I am missing something, don't hesitate to tell me.

This is a slight GUI change to the rename/overwrite dialog window, just to make it more user friendly.</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;">Correctly Compiled.
"Eye ball'ed" Code.
Ran code with all possible '_mode' as per http://api.kde.org/4.5-api/kdelibs-apidocs/kio/html/namespaceKIO.html#bac5df6792cd3426582dbfd1af706bff
Ran many possible combinations (most if not all) -> move folder to folder, file to file, file to folder, folder to file, and paid attention to creation date.

(I actually found a bug with the preview picture that is shown in a certain scenario - I will create a bug notice for it on bugs.kde.org soon)</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=238942">238942</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/kdelibs/kio/kio/renamedialog.cpp <span style="color: grey">(1181964)</span></li>

</ul>

<p><a href="http://svn.reviewboard.kde.org/r/5443/diff/" style="margin-left: 3em;">View Diff</a></p>



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>

<div>

 <a href="http://svn.reviewboard.kde.org/r/5443/s/524/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/03/Trunk_-_Folder_to_Folder_400x100.png" style="border: 1px black solid;" alt="KDE Trunk - Folder to Folder" /></a>

 <a href="http://svn.reviewboard.kde.org/r/5443/s/525/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/03/New_-_Folder_to_Folder_400x100.png" style="border: 1px black solid;" alt="*New - Folder to Folder" /></a>

 <a href="http://svn.reviewboard.kde.org/r/5443/s/526/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/03/Trunk_-_Folder_to_File_400x100.png" style="border: 1px black solid;" alt="KDE Trunk - Bug" /></a>

 <a href="http://svn.reviewboard.kde.org/r/5443/s/527/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/03/New_-_File_to_File_400x100.png" style="border: 1px black solid;" alt="*New - File to File" /></a>

 <a href="http://svn.reviewboard.kde.org/r/5443/s/528/"><img src="http://svn.reviewboard.kde.org/media/uploaded/images/2010/10/03/KDE4.5__Preview_400x100.png" style="border: 1px black solid;" alt="KDE 4.5 - File to File" /></a>

</div>


  </td>
 </tr>
</table>








  </div>
 </body>
</html>