<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/121287/">https://git.reviewboard.kde.org/r/121287/</a>
     </td>
    </tr>
   </table>
   <br />



 <p>Ship it!</p>



 <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;">Looks good.  I just think that the fact we use toDisplayString() instead of path() should be commented upon somewhere.</p></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="https://git.reviewboard.kde.org/r/121287/diff/1/?file=331035#file331035line245" style="color: black; font-weight: bold; text-decoration: underline;">keduvocdocument/keduvocdocument.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; ">KEduVocDocument::ErrorCode KEduVocDocument::open( const QUrl& url, FileHandlingFlags flags)</pre></td>

  </tr>
 </tbody>



 
 

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">243</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">QString</span> <span class="n">errorMessage</span> <span class="o">=</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"<qt>Cannot open file<br /><b>%1</b></qt>"</span><span class="p">,</span> <span class="n">url</span><span class="p">.</span><span class="n"><span class="hl">path</span></span><span class="p">()</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">245</font></th>
    <td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span class="n">QString</span> <span class="n">errorMessage</span> <span class="o">=</span> <span class="n">i18n</span><span class="p">(</span> <span class="s">"<qt>Cannot open file<br /><b>%1</b></qt>"</span><span class="p">,</span> <span class="n">url</span><span class="p">.</span><span class="n"><span class="hl">toDisplayString</span></span><span class="p">()</span> <span class="p">);</span></pre></td>
  </tr>

 </tbody>

</table>

 <div style="margin-left: 2em;">

  <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 so many instances of this call so it's difficult to know where it's best.  But I do think that we need a comment somewhere to comment on the fact that url.path() doesn't work. And frankly, that feels like a bug in QUrl to me.</p></pre>
 </div>
</div>
<br />



<p>- Inge Wallin</p>


<br />
<p>On November 28th, 2014, 10:59 p.m. UTC, Jeremy Whiting 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 KDE Edu, Andreas Xavier and Inge Wallin.</div>
<div>By Jeremy Whiting.</div>


<p style="color: grey;"><i>Updated Nov. 28, 2014, 10:59 p.m.</i></p>









<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt;">Repository: </b>
libkeduvocdocument
</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;">Before this patch parley on windows has issues saving to any new file, and also saving an existing .kvtml file.
With this patch both issues are fixed.
On windows QUrl .path() returns the path without the drive letter. Changing it to use QUrl .toDisplayString() fixed that here.</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;">Builds and runs and fixes the bug on windows. Will test on linux later on.</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>keduvocdocument/autosave/kautosavefileprivate.cpp <span style="color: grey">(bb2c845)</span></li>

 <li>keduvocdocument/keduvocdocument.cpp <span style="color: grey">(dbaef89)</span></li>

</ul>

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






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








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