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

List:       kde-panel-devel
Subject:    Re: Review Request: Improve the QML RunnerModel
From:       "Aleix Pol Gonzalez" <aleixpol () gmail ! com>
Date:       2012-01-29 21:11:16
Message-ID: 20120129211116.32049.86622 () vidsolbach ! de
[Download RAW message or body]

[Attachment #2 (multipart/alternative)]


> On Jan. 27, 2012, 7:21 p.m., Marco Martin wrote:
> > it looks good to me, (and proably you're right 90% of qml models a qabstractlistmodel could \
> > be enough) only thing that i am usire of is the runnerforid function

Just that runnerForIndex thing, please comment on it and I'll submit another patch without the \
trailing whitespaces and "fuuuu".

Thanks!!


> On Jan. 27, 2012, 7:21 p.m., Marco Martin wrote:
> > components/runnermodel/runnermodel.cpp, lines 188-189
> > <http://git.reviewboard.kde.org/r/103806/diff/1/?file=48051#file48051line188>
> > 
> > why a direct access for the runner is needed?

Well, now that you say it it's just used for the section. We can add a new role that does this \
indoors, I can see benefit in accessing the runner in terms of flexibility, though.

If we limit the model user to the roles we might end up making the API not scalable over the \
time.

Since I'm an alien over here, I'll let you decide what's best. :)


- Aleix


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


On Jan. 27, 2012, 6:06 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103806/
> -----------------------------------------------------------
> 
> (Updated Jan. 27, 2012, 6:06 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> Adds some features to the RunnerModel so that it can be used properly in the KRunner QML \
> implementation I've been working on (see the testing section). 
> Also it simplifies the code a bit by moving from QAbstractItemModel -> QAbstractListModel.
> 
> 
> Diffs
> -----
> 
> components/runnermodel/runnermodel.h 899bf1f 
> components/runnermodel/runnermodel.cpp a226f8e 
> 
> Diff: http://git.reviewboard.kde.org/r/103806/diff/diff
> 
> 
> Testing
> -------
> 
> use it in kde:scratch/apol/krunner-qml (proof of concept for a KRunner implemented in \
> QtQuick). 
> here's a video, so that you know what's going on: \
> http://www.proli.net/meu/netrunner/qmlrunner.ogv 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
> 


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





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 27th, 2012, 7:21 p.m., <b>Marco Martin</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;">it looks good to me, (and proably you&#39;re \
right 90% of qml models a qabstractlistmodel could be enough) only thing that i am usire of is \
the runnerforid function</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;">Just that runnerForIndex thing, please \
comment on it and I&#39;ll submit another patch without the trailing whitespaces and \
&quot;fuuuu&quot;.

Thanks!!</pre>
<br />





<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On January 27th, 2012, 7:21 p.m., <b>Marco Martin</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/103806/diff/1/?file=48051#file48051line188" \
style="color: black; font-weight: bold; text-decoration: \
underline;">components/runnermodel/runnermodel.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 \
RunnerModel::matchesChanged(const QList&lt;Plasma::QueryMatch&gt; &amp;matches)</pre></td>

  </tr>
 </tbody>






 
 

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font \
size="2">183</font></th>  <td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; \
line-height: 140%; margin: 0; "></pre></td>  <th bgcolor="#f0f0f0" style="border-left: 1px \
solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">167</font></th>  \
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></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">168</font></th>  \
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"><span class="n">QObject</span><span class="o">*</span> <span \
class="n">RunnerModel</span><span class="o">::</span><span class="n">runnerForId</span><span \
class="p">(</span><span class="k">const</span> <span class="n">QString</span><span \
class="o">&amp;</span> <span class="n">id</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 a direct access for the runner is \
needed?</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;">Well, now that you say it it&#39;s \
just used for the section. We can add a new role that does this indoors, I can see benefit in \
accessing the runner in terms of flexibility, though.

If we limit the model user to the roles we might end up making the API not scalable over the \
time.

Since I&#39;m an alien over here, I&#39;ll let you decide what&#39;s best. :)</pre>
<br />




<p>- Aleix</p>


<br />
<p>On January 27th, 2012, 6:06 p.m., Aleix Pol Gonzalez 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 Aleix Pol Gonzalez.</div>


<p style="color: grey;"><i>Updated Jan. 27, 2012, 6:06 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;">Adds some features to \
the RunnerModel so that it can be used properly in the KRunner QML implementation I&#39;ve been \
working on (see the testing section).

Also it simplifies the code a bit by moving from QAbstractItemModel -&gt; \
QAbstractListModel.</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;">use it in \
kde:scratch/apol/krunner-qml (proof of concept for a KRunner implemented in QtQuick).

here&#39;s a video, so that you know what&#39;s going on: \
http://www.proli.net/meu/netrunner/qmlrunner.ogv</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>components/runnermodel/runnermodel.h <span style="color: grey">(899bf1f)</span></li>

 <li>components/runnermodel/runnermodel.cpp <span style="color: grey">(a226f8e)</span></li>

</ul>

<p><a href="http://git.reviewboard.kde.org/r/103806/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