[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-panel-devel
Subject:    Re: Review Request: Plasmate:now StartPage::createNewProject()
From:       "Aaron J. Seigo" <aseigo () kde ! org>
Date:       2011-11-15 7:59:35
Message-ID: 20111115075935.12615.32481 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/102732/#review8202
-----------------------------------------------------------



startpage.cpp
<http://git.reviewboard.kde.org/r/102732/#comment7025>

    won't this always be the case for a Plasmoid (or other Plasma Package)?
    =

    the normal is:
    =

    metadata.desktop
    contents/
    =

    so ... this will go up one directory when an external project  is loade=
d and end up in the wrong place. (e.g. an external project is loaded). that=
 looks incorrect.
    =

    it should only go up one directory in the case of a plasmate created pr=
oject, which looks like this on disk:
    =

    NOTE
    .plasmaprojectrc
    <nameofproject>/
       \__ metadata.desktop
        |__ contents
    =

    i think the real problem here is in trying to autodetect by what is on =
disk what kind of project it is (created by plasmate, or loaded externally)=
. the easiest way around this, really, is to stop trying to do that and hav=
e plasmate simply always point to the directory with the .plasmateprojectrc=
 file.
    =

    of course, in the case where is no projectrc file, then the problem rem=
ains. instead of having these "dir.cd('..')" checks throughout the code, an=
d constantly having bugs in that code, there should be ONE method somewhere=
 that, given a directory, determines where the root of the project should p=
oint to.
    =

    this way we can write it once and get it right for all uses.
    =

    it may even make sense to simply check and see if the project is in apd=
data, and if so then we know it is a plasmate created project with that dir=
 structure .. otherwise it is an imported project and to be treated that wa=
y.


- Aaron J. Seigo


On Nov. 14, 2011, 8:43 p.m., Giorgos Tsiapaliwkas wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/102732/
> -----------------------------------------------------------
> =

> (Updated Nov. 14, 2011, 8:43 p.m.)
> =

> =

> Review request for Plasma.
> =

> =

> Description
> -------
> =

> Hello,
> =

> for the moment the .plasmateprojectrc file exists as a mark for the proje=
cts created by plasmate.
> We want the file to be created in the creation of the project not in the =
load of the project,the patch does that.
> =

> cheers
> =

> =

> Diffs
> -----
> =

>   mainwindow.cpp 6ea7f0c =

>   startpage.h f16af4b =

>   startpage.cpp 75052b1 =

> =

> Diff: http://git.reviewboard.kde.org/r/102732/diff/diff
> =

> =

> Testing
> -------
> =

> workinig
> =

> =

> Thanks,
> =

> Giorgos Tsiapaliwkas
> =

>


[Attachment #5 (text/html)]

<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/102732/">http://git.reviewboard.kde.org/r/102732/</a>
  </td>
    </tr>
   </table>
   <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://git.reviewboard.kde.org/r/102732/diff/2/?file=40941#file40941line587" \
style="color: black; font-weight: bold; text-decoration: \
underline;">startpage.cpp</a>  <span style="font-weight: normal;">

     (Diff revision 2)

    </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 \
StartPage::checkLocalProjectPath(const QString&amp; name)</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">587</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="p">}</span> <span class="k">else</span> <span class="k">if</span> <span \
class="p">(</span><span class="n">QDir</span><span class="p">(</span><span \
class="n">path</span> <span class="o">+</span> <span \
class="s">&quot;/contents&quot;</span><span class="p">).</span><span \
class="n">exists</span><span class="p">())</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;">won&#39;t \
this always be the case for a Plasmoid (or other Plasma Package)?

the normal is:

metadata.desktop
contents/

so ... this will go up one directory when an external project  is loaded and end up \
in the wrong place. (e.g. an external project is loaded). that looks incorrect.

it should only go up one directory in the case of a plasmate created project, which \
looks like this on disk:

NOTE
.plasmaprojectrc
&lt;nameofproject&gt;/
   \__ metadata.desktop
    |__ contents

i think the real problem here is in trying to autodetect by what is on disk what kind \
of project it is (created by plasmate, or loaded externally). the easiest way around \
this, really, is to stop trying to do that and have plasmate simply always point to \
the directory with the .plasmateprojectrc file.

of course, in the case where is no projectrc file, then the problem remains. instead \
of having these &quot;dir.cd(&#39;..&#39;)&quot; checks throughout the code, and \
constantly having bugs in that code, there should be ONE method somewhere that, given \
a directory, determines where the root of the project should point to.

this way we can write it once and get it right for all uses.

it may even make sense to simply check and see if the project is in apddata, and if \
so then we know it is a plasmate created project with that dir structure .. otherwise \
it is an imported project and to be treated that way.</pre> </div>
<br />



<p>- Aaron J.</p>


<br />
<p>On November 14th, 2011, 8:43 p.m., Giorgos Tsiapaliwkas 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 Tsiapaliwkas.</div>


<p style="color: grey;"><i>Updated Nov. 14, 2011, 8:43 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;">Hello,

for the moment the .plasmateprojectrc file exists as a mark for the projects created \
by plasmate. We want the file to be created in the creation of the project not in the \
load of the project,the patch does that.

cheers</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;">workinig</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>mainwindow.cpp <span style="color: grey">(6ea7f0c)</span></li>

 <li>startpage.h <span style="color: grey">(f16af4b)</span></li>

 <li>startpage.cpp <span style="color: grey">(75052b1)</span></li>

</ul>

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




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








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



_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic