'Re: Review Request: TaskManager: Prompt user when automatic task to'

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

List:       kde-panel-devel
Subject:    Re: Review Request: TaskManager: Prompt user when automatic task to
From:       "Aaron J. Seigo" <aseigo () kde ! org>
Date:       2011-11-01 15:07:56
Message-ID: 20111101150756.22087.30404 () 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/103007/#review7801
-----------------------------------------------------------

the wholesale copying of KOpenWithDialog is very unfortunate and needs to be avoided \
if at *all* possible. why is it copied instead of used directly?

the really big issue is the exec()'ing of the dialog, however.

it would also be nice to be able to access individual launcher configuration via the \
individual launchers' context menu.

p.s. whitespace :)

libs/taskmanager/kapplicationselectordialog.cpp
<http://git.reviewboard.kde.org/r/103007/#comment6730>

    should be removed.

libs/taskmanager/taskactions.cpp
<http://git.reviewboard.kde.org/r/103007/#comment6731>

    dialogs must not be exec()'d as they *freeze* the entire UI.. in this case the \
whole plasma-desktop shell. i must be handled asynchronously.

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

    in which cases would the mapping not be saved (e.g. saveMapping == false)?

- 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/103007/
> -----------------------------------------------------------
> 
> (Updated Oct. 31, 2011, 8:42 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Description
> -------
> 
> 1. If fail to automatically find launcher, then prompt user to select from \
> installed applications. 2. Add a config page, so that manualy set launchers may be \
> adjusted. 
> (Part of IconTasks' taskmanager changes)
> 
> 
> Diffs
> -----
> 
> libs/taskmanager/CMakeLists.txt 57f5f73 
> libs/taskmanager/groupmanager.h acaa142 
> libs/taskmanager/groupmanager.cpp 6e7ffa7 
> libs/taskmanager/kapplicationselectordialog.h PRE-CREATION 
> libs/taskmanager/kapplicationselectordialog.cpp PRE-CREATION 
> libs/taskmanager/launcherconfig.h PRE-CREATION 
> libs/taskmanager/launcherconfig.cpp PRE-CREATION 
> libs/taskmanager/launcherconfig.ui PRE-CREATION 
> libs/taskmanager/launcherproperties.h PRE-CREATION 
> libs/taskmanager/launcherproperties.cpp PRE-CREATION 
> libs/taskmanager/launcherproperties.ui PRE-CREATION 
> libs/taskmanager/taskactions.cpp 0e6ba8e 
> libs/taskmanager/taskitem.h 5de8478 
> libs/taskmanager/taskitem.cpp 0a768e5 
> 
> Diff: http://git.reviewboard.kde.org/r/103007/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/103007/">http://git.reviewboard.kde.org/r/103007/</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;">the wholesale copying of \
KOpenWithDialog is very unfortunate and needs to be avoided if at *all* possible. why \
is it copied instead of used directly?

the really big issue is the exec()&#39;ing of the dialog, however.

it would also be nice to be able to access individual launcher configuration via the \
individual launchers&#39; context menu.

p.s. whitespace :)</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/103007/diff/1/?file=39948#file39948line484" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/kapplicationselectordialog.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; ">void KApplicationSelectorDialog::slotHighlighted(const QString&amp; entryPath, \
const QString&amp;)</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">484</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">    <span \
class="n">printf</span><span class="p">(</span><span \
class="s">&quot;HIghlgihted</span><span class="se">\n</span><span \
class="s">&quot;</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;">should be \
removed.</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/103007/diff/1/?file=39955#file39955line375" \
style="color: black; font-weight: bold; text-decoration: \
underline;">libs/taskmanager/taskactions.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; \
">ToggleLauncherActionImpl::ToggleLauncherActionImpl(QObject *parent, \
AbstractGroupableItem *item, GroupManager *strategy)</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">375</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">QDialog</span><span \
class="o">::</span><span class="n">Accepted</span><span class="o">==</span><span \
class="n">dlg</span><span class="p">.</span><span class="n">exec</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;">dialogs \
must not be exec()&#39;d as they *freeze* the entire UI.. in this case the whole \
plasma-desktop shell. i must be handled asynchronously.</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/103007/diff/1/?file=39957#file39957line362" \
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; ">void \
TaskItem::addMimeData(QMimeData *mimeData) 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">362</font></th>  <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="kt">void</span> <span class="n">TaskItem</span><span class="o">::</span><span \
class="n">setLauncherUrl</span><span class="p">(</span><span class="k">const</span> \
<span class="n">KUrl</span> <span class="o">&amp;</span><span \
class="n">url</span><span class="p">,</span> <span class="kt">bool</span> <span \
class="n">saveMapping</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;">in which \
cases would the mapping not be saved (e.g. saveMapping == false)?</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. If fail to automatically find launcher, then prompt user to select \
from installed applications. 2. Add a config page, so that manualy set launchers may \
be adjusted.

(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/CMakeLists.txt <span style="color: grey">(57f5f73)</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/kapplicationselectordialog.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/kapplicationselectordialog.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherconfig.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherconfig.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherconfig.ui <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherproperties.h <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherproperties.cpp <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/launcherproperties.ui <span style="color: \
grey">(PRE-CREATION)</span></li>

 <li>libs/taskmanager/taskactions.cpp <span style="color: grey">(0e6ba8e)</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/103007/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] 