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








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On November 29th, 2012, 1:53 p.m., <b>Aaron J. 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="http://git.reviewboard.kde.org/r/106680/diff/5/?file=93807#file93807line544" style="color: black; font-weight: bold; text-decoration: underline;">plasmate/packagemodel.cpp</a>
    <span style="font-weight: normal;">

     (Diff revision 5)

    </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; ">bool PackageModel::loadPackage()</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">539</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">packageType</span><span class="p">()</span> <span class="o">==</span> <span class="s">"Plasma/Theme"</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;">"Plasma/Theme" should not appear in this file.</pre>
 </blockquote>



 <p>On December 7th, 2012, 5:56 p.m., <b>Giorgos Tsiapaliokas</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;">the "colors" file isn't a requiredDirectory nor a requiredFile, so If I don't add it by hand in the m_topEntries
it won't be visible and also we want the "colors" file in the m_topEntries only when the package is a theme.
No? What am I missing?</pre>
 </blockquote>





 <p>On December 13th, 2012, 4:39 p.m., <b>Aaron J. 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;">perhaps add an exception for top level non-required files, so that everything in the top level, required or not, is shown.</pre>
 </blockquote>





 <p>On December 16th, 2012, 2:28 p.m., <b>Giorgos Tsiapaliokas</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 can't find a way (I don't think that there is one) in order to specify which files are top level ones.

Before the refactor of the packagemodel, the model was doing this:
* find all the files(required or not)
* check if you can give them a name and call them namedFiles(remove the ones which have a name from the hash)
* everything that has been left in the hash add it as a top level item

Now with the refactor of the packagemodel, it does:
* put the required files in a hash and all(including the required ones) the files in another one
** We need the second hash with all the files, because if the project already exists, we want to
add in the ui(with names) all the existing files not just the required ones.
* add all the required files in the ui and give them a name (namedFiles)
* if there are requiredFiles still in the hash add them in the ui.

I see your point that adding a hard coded key can make plasmate fail at some point,
but those keys doesn't change often(actually they don't change at all. No?) so we are fine(not ok).
Even if plasmate fails, in the -worst/and only- case scenario the user won't be able see/modify the "colors" file</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;">Thinking about it some more, all named files should be shown. Everything that is not a directory but a key with a single file associated with it should be shown, otherwise any package structure that has a file in the same way ThemePackage has colors will fail.</pre>
<br />




<p>- Aaron J.</p>


<br />
<p>On December 7th, 2012, 5:58 p.m., Giorgos Tsiapaliokas wrote:</p>






<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://git.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 Plasma.</div>
<div>By Giorgos Tsiapaliokas.</div>


<p style="color: grey;"><i>Updated Dec. 7, 2012, 5:58 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;">create a new theme package->click on the new

a file dialog should appear but instead a simple edit box appears requesting a new filename.

This patch solves the issue</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>plasmate/editors/editpage.h <span style="color: grey">(5cb3ea6)</span></li>

 <li>plasmate/editors/editpage.cpp <span style="color: grey">(7e82ff2)</span></li>

 <li>plasmate/packagemodel.h <span style="color: grey">(efa3001)</span></li>

 <li>plasmate/packagemodel.cpp <span style="color: grey">(9eb0914)</span></li>

</ul>

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




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








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