'Re: Review Request: TaskManager: Store launcher order'

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

List:       kde-panel-devel
Subject:    Re: Review Request: TaskManager: Store launcher order
From:       "Aaron J. Seigo" <aseigo () kde ! org>
Date:       2011-11-01 14:56:39
Message-ID: 20111101145639.19499.38003 () 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/103006/#review7798
-----------------------------------------------------------

usual comment about whitespace correctness; otherwise a few comments inline ... some \
really nice fixes and improvements in this one. a little bit of cleanup and it should \
be ready to go.

libs/taskmanager/groupmanager.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6715>

    instead of having the launchers in two lists, if ordering matters (and i agree it \
should), then perhaps turn the QHash of launchers into a QList<LauncherItem *> and \
keep the ordering there.  
    finding by URL (which happens in two places) then becomes:

    foreach (const LauncherItem *item, launchers) {
        if (item->url() == url) {
             ... found ...
        }
    }

    the # of launchers will remain small so the overhead of this will be negligable \
and it will save both on allocations and having to keep two collections in sync with \
each other.  
    this could even be turned into a small helper method in the private class: int \
indexOfLauncher(const KUrl &url) const;

libs/taskmanager/strategies/alphasortingstrategy.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6722>

    might be more readable as an if/else given the length of the items in the \
ternary...

libs/taskmanager/strategies/alphasortingstrategy.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6723>

    this makes the speed of taskName() even more critical as this will get called \
often

libs/taskmanager/strategies/desktopsortingstrategy.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6726>

    this should be a private member of the class ... as this is a non-exported class, \
binary compatibility doesn't matter.

libs/taskmanager/strategies/programgroupingstrategy.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6729>

    this check looks inccorect: if launchers are integrated or separate, it should \
still be possible to say that a given application should not be groupable with other \
windows from that same application.

libs/taskmanager/taskitem.cpp
<http://git.reviewboard.kde.org/r/103006/#comment6720>

    have you done any time measurements on how long this method takes? it will be \
called potentially quite a few times when a launcher is started ...

- Aaron J. Seigo

On Oct. 31, 2011, 8:42 p.m., Craig Drummond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103006/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2011, 8:42 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> 1. Store the order in which launchers are created
> 2. Add a new config option, separateLaunchers - so that applet can decide if \
> launchers and tasks should be kept separate. 
> (Part of IconTasks' taskmanager changes)
> 
> 
> Diffs
> -----
> 
> libs/taskmanager/abstractgroupingstrategy.cpp 4ed424a 
> libs/taskmanager/abstractsortingstrategy.cpp 390f6f0 
> libs/taskmanager/groupmanager.h acaa142 
> libs/taskmanager/groupmanager.cpp 6e7ffa7 
> libs/taskmanager/launcheritem.cpp 20f0e7c 
> libs/taskmanager/strategies/alphasortingstrategy.cpp 9ec1aca 
> libs/taskmanager/strategies/desktopsortingstrategy.cpp 520fead 
> libs/taskmanager/strategies/manualsortingstrategy.h 113faab 
> libs/taskmanager/strategies/manualsortingstrategy.cpp 4409a6b 
> libs/taskmanager/strategies/programgroupingstrategy.cpp 5c43d03 
> libs/taskmanager/taskgroup.h 53c2871 
> libs/taskmanager/taskgroup.cpp 49140ae 
> libs/taskmanager/taskitem.h 5de8478 
> libs/taskmanager/taskitem.cpp 0a768e5 
> 
> Diff: http://git.reviewboard.kde.org/r/103006/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Craig Drummond
> 
> 

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

 <pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">usual comment about \
whitespace correctness; otherwise a few comments inline ... some really nice fixes \
and improvements in this one. a little bit of cleanup and it should be ready to \
go.</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="http://git.reviewboard.kde.org/r/103006/diff/1/?file=39933#file39933line122" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/groupmanager.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; \
">public:</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">117</font></th>  <td bgcolor="#ffffff" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">QHash</span><span class="o">&lt;</span><span class="n">KUrl</span><span \
class="p">,</span> <span class="n">LauncherItem</span> <span class="o">*&gt;</span> \
<span class="n">launchers</span><span class="p">;</span></pre></td>  <th \
bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">122</font></th>  <td bgcolor="#ffffff" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">QHash</span><span class="o">&lt;</span><span class="n">KUrl</span><span \
class="p">,</span> <span class="n">LauncherItem</span> <span class="o">*&gt;</span> \
<span class="n">launchers</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">123</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">QList</span><span class="o">&lt;</span><span class="n">KUrl</span><span \
class="o">&gt;</span> <span class="n">launcherSort</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;">instead of \
having the launchers in two lists, if ordering matters (and i agree it should), then \
perhaps turn the QHash of launchers into a QList&lt;LauncherItem *&gt; and keep the \
ordering there.

finding by URL (which happens in two places) then becomes:

foreach (const LauncherItem *item, launchers) {
    if (item-&gt;url() == url) {
         ... found ...
    }
}

the # of launchers will remain small so the overhead of this will be negligable and \
it will save both on allocations and having to keep two collections in sync with each \
other.

this could even be turned into a small helper method in the private class: int \
indexOfLauncher(const KUrl &amp;url) const;</pre> </div>
<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/103006/diff/1/?file=39935#file39935line67" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/strategies/alphasortingstrategy.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 \
AlphaSortingStrategy::sortItems(ItemList &amp;items)</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">64</font></th>  <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; ">            <span \
class="n">launcherMap</span><span class="p">.</span><span \
class="n">insertMulti</span><span class="p">(</span><span \
class="n">groupable</span><span class="o">-&gt;</span><span \
class="n">name</span><span class="p">().</span><span class="n">toLower</span><span \
class="p">(),</span> <span class="n">groupable</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">67</font></th>  <td bgcolor="#fdfebc" width="50%"><pre style="font-size: \
8pt; line-height: 140%; margin: 0; ">           <span class="hl"> </span><span \
class="n"><span class="hl">separateLaunchers</span></span><span class="hl"> \
</span><span class="o"><span class="hl">?</span></span> <span \
class="n">launcherMap</span><span class="p">.</span><span \
class="n">insertMulti</span><span class="p">(</span><span \
class="n">groupable</span><span class="o">-&gt;</span><span \
class="n">name</span><span class="p">().</span><span class="n">toLower</span><span \
class="p">(),</span> <span class="n">groupable</span><span class="p">)</span><span \
class="hl"> </span><span class="o"><span class="hl">:</span></span><span class="hl"> \
</span><span class="n"><span class="hl">map</span></span><span class="p"><span \
class="hl">.</span></span><span class="n"><span \
class="hl">insertMulti</span></span><span class="p"><span \
class="hl">(</span></span><span class="n"><span \
class="hl">groupable</span></span><span class="o"><span \
class="hl">-&gt;</span></span><span class="n"><span \
class="hl">name</span></span><span class="p"><span class="hl">().</span></span><span \
class="n"><span class="hl">toLower</span></span><span class="p"><span \
class="hl">(),</span></span><span class="hl"> </span><span class="n"><span \
class="hl">groupable</span></span><span class="p"><span \
class="hl">)</span>;</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;">might be \
more readable as an if/else given the length of the items in the ternary...</pre> \
</div> <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/103006/diff/1/?file=39935#file39935line85" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/strategies/alphasortingstrategy.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 \
AlphaSortingStrategy::sortItems(ItemList &amp;items)</pre></td>

  </tr>
 </tbody>

 <tbody>

  <tr>
    <th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2">82</font></th>  <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span \
class="n">map</span><span class="p">.</span><span class="n">insertMulti</span><span \
class="p">(</span><span class="n">item</span><span class="o">-&gt;</span><span \
class="n">task</span><span class="p"><span class="hl">()</span></span><span \
class="o"><span class="hl">-&gt;</span></span><span class="n"><span \
class="hl">classClass</span></span><span class="p">().</span><span \
class="n">toLower</span><span class="p">(),</span> <span \
class="n">groupable</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">85</font></th>  <td bgcolor="#fdfebc" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span \
class="n">map</span><span class="p">.</span><span class="n">insertMulti</span><span \
class="p">(</span><span class="n">item</span><span class="o">-&gt;</span><span \
class="n">task<span class="hl">Name</span></span><span class="p">().</span><span \
class="n">toLower</span><span class="p">(),</span> <span \
class="n">groupable</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;">this makes \
the speed of taskName() even more critical as this will get called often</pre> </div>
<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/103006/diff/1/?file=39936#file39936line40" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/strategies/desktopsortingstrategy.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="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; \
"></pre></td>  <td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: \
0; ">static bool separateLaunchers=false;</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">39</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="k">static</span> <span class="kt">bool</span> <span \
class="n">separateLaunchers</span><span class="o">=</span><span \
class="kc">false</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;">this should \
be a private member of the class ... as this is a non-exported class, binary \
compatibility doesn&#39;t matter.</pre> </div>
<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/103006/diff/1/?file=39939#file39939line69" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/strategies/programgroupingstrategy.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; \
">ProgramGroupingStrategy::~ProgramGroupingStrategy()</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">69</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">QList</span><span class="o">&lt;</span><span class="n">QAction</span><span \
class="o">*&gt;</span> <span class="n">actionList</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">70</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">71</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">GroupManager</span> <span class="o">*</span><span class="n">gm</span><span \
class="o">=</span><span class="n">qobject_cast</span><span class="o">&lt;</span><span \
class="n">GroupManager</span> <span class="o">*&gt;</span><span \
class="p">(</span><span class="n">AbstractGroupingStrategy</span><span \
class="o">::</span><span class="n">parent</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">72</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">73</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="o">!</span><span \
class="n">gm</span> <span class="o">||</span> <span class="n">gm</span><span \
class="o">-&gt;</span><span class="n">separateLaunchers</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;">this check \
looks inccorect: if launchers are integrated or separate, it should still be possible \
to say that a given application should not be groupable with other windows from that \
same application.</pre> </div>
<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/103006/diff/1/?file=39943#file39943line171" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/taskitem.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; \
">QString TaskItem::name() const</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">171</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">TaskItem</span><span \
class="o">::</span><span class="n">taskName</span><span class="p">()</span> <span \
class="k">const</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">172</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><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">173</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">d</span><span \
class="o">-&gt;</span><span class="n">taskName</span><span class="p">.</span><span \
class="n">isEmpty</span><span class="p">())</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">174</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span \
class="n">KUrl</span> <span class="n">lUrl</span><span class="o">=</span><span \
class="n">launcherUrl</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">175</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">176</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="o">!</span><span \
class="n">lUrl</span><span class="p">.</span><span class="n">isEmpty</span><span \
class="p">()</span> <span class="o">&amp;&amp;</span> <span \
class="n">lUrl</span><span class="p">.</span><span class="n">isLocalFile</span><span \
class="p">()</span> <span class="o">&amp;&amp;</span> <span \
class="n">KDesktopFile</span><span class="o">::</span><span \
class="n">isDesktopFile</span><span class="p">(</span><span \
class="n">lUrl</span><span class="p">.</span><span class="n">toLocalFile</span><span \
class="p">()))</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">177</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            \
<span class="n">KDesktopFile</span> <span class="n">f</span><span \
class="p">(</span><span class="n">lUrl</span><span class="p">.</span><span \
class="n">toLocalFile</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">178</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">179</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">f</span><span \
class="p">.</span><span class="n">tryExec</span><span class="p">())</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">180</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">              \
<span class="n">d</span><span class="o">-&gt;</span><span class="n">taskName</span> \
<span class="o">=</span> <span class="n">f</span><span class="p">.</span><span \
class="n">readName</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">181</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            \
<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">182</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <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">183</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">d</span><span \
class="o">-&gt;</span><span class="n">taskName</span><span class="p">.</span><span \
class="n">isEmpty</span><span class="p">()</span> <span class="o">&amp;&amp;</span> \
<span class="n">d</span><span class="o">-&gt;</span><span class="n">task</span><span \
class="p">)</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">184</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">            \
<span class="n">d</span><span class="o">-&gt;</span><span class="n">taskName</span> \
<span class="o">=</span> <span class="n">d</span><span class="o">-&gt;</span><span \
class="n">task</span><span class="p">.</span><span class="n">data</span><span \
class="p">()</span><span class="o">-&gt;</span><span class="n">classClass</span><span \
class="p">().</span><span class="n">toLower</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">185</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <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">186</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <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">187</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">188</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="k">return</span> <span class="n">d</span><span class="o">-&gt;</span><span \
class="n">taskName</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">189</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><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;">have you \
done any time measurements on how long this method takes? it will be called \
potentially quite a few times when a launcher is started ...</pre> </div>
<br />

<p>- Aaron J.</p>

<br />
<p>On October 31st, 2011, 8:42 p.m., Craig Drummond 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 Craig Drummond.</div>

<p style="color: grey;"><i>Updated Oct. 31, 2011, 8:42 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;">1. Store the order in which launchers are created 2. Add a new config \
option, separateLaunchers - so that applet can decide if launchers and tasks should \
be kept separate.

(Part of IconTasks&#39; taskmanager changes)</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>libs/taskmanager/abstractgroupingstrategy.cpp <span style="color: \
grey">(4ed424a)</span></li>

 <li>libs/taskmanager/abstractsortingstrategy.cpp <span style="color: \
grey">(390f6f0)</span></li>

 <li>libs/taskmanager/groupmanager.h <span style="color: grey">(acaa142)</span></li>

 <li>libs/taskmanager/groupmanager.cpp <span style="color: \
grey">(6e7ffa7)</span></li>

 <li>libs/taskmanager/launcheritem.cpp <span style="color: \
grey">(20f0e7c)</span></li>

 <li>libs/taskmanager/strategies/alphasortingstrategy.cpp <span style="color: \
grey">(9ec1aca)</span></li>

 <li>libs/taskmanager/strategies/desktopsortingstrategy.cpp <span style="color: \
grey">(520fead)</span></li>

 <li>libs/taskmanager/strategies/manualsortingstrategy.h <span style="color: \
grey">(113faab)</span></li>

 <li>libs/taskmanager/strategies/manualsortingstrategy.cpp <span style="color: \
grey">(4409a6b)</span></li>

 <li>libs/taskmanager/strategies/programgroupingstrategy.cpp <span style="color: \
grey">(5c43d03)</span></li>

 <li>libs/taskmanager/taskgroup.h <span style="color: grey">(53c2871)</span></li>

 <li>libs/taskmanager/taskgroup.cpp <span style="color: grey">(49140ae)</span></li>

 <li>libs/taskmanager/taskitem.h <span style="color: grey">(5de8478)</span></li>

 <li>libs/taskmanager/taskitem.cpp <span style="color: grey">(0a768e5)</span></li>

</ul>

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