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








<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
 <p style="margin-top: 0;">On March 6th, 2012, 5:18 p.m., <b>David Edmundson</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/104173/diff/1/?file=52062#file52062line178" style="color: black; font-weight: bold; text-decoration: underline;">rooms-model.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; ">QVariant FavoriteRoomsModel::data(const QModelIndex &index, int role) 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">178</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">index</span><span class="p">.</span><span class="n">row</span><span class="p">()</span> <span class="o">>=</span> <span class="n">m_favoriteRoomsList</span><span class="p">.</span><span class="n">size</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;">.isValid() should check index < rowCount() so this isn't needed.

</pre>
 </blockquote>



 <p>On March 7th, 2012, 9:28 a.m., <b>Dominik Cermak</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;">Are you sure about that?
"A valid index belongs to a model, and has non-negative row and column numbers." (http://qt-project.org/doc/qt-4.8/qmodelindex.html#isValid)

As far as I understand this statement it isn't checked, but I you are sure it is I'm happy to remove it.</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;">You are correct, it does not check < rowCount();

The function actually looks like this:

  inline bool isValid() const { return (r >= 0) && (c >= 0) && (m != 0); }

so it does not even check if the data exists.

(for reference -- http://qt.gitorious.org/qt/qt/blobs/4.7/src/corelib/kernel/qabstractitemmodel.h#line77 )</pre>
<br />




<p>- Martin</p>


<br />
<p>On March 6th, 2012, 2:35 p.m., Dominik Cermak 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 Telepathy.</div>
<div>By Dominik Cermak.</div>


<p style="color: grey;"><i>Updated March 6, 2012, 2:35 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;">Now you can maintain a list of your favorite chatrooms.
These are saved in the general ktelepathyrc to use them for autoconnection (not available yet, will work on it).

To not clutter the dialog too much I introduced tabs.

I'm wondering whether it would make sense to merge those two models (RoomsModel and FavoriteRoomsModel) into one (with FavoriteRoomsModel as the base for the new one).</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;">Adding and removing favorites.
After closing and reopening contact-list the favorites are still there.</pre>
  </td>
 </tr>
</table>



<div style="margin-top: 1.5em;">
 <b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>


 <a href="http://bugs.kde.org/show_bug.cgi?id=291711">291711</a>


</div>


<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">

 <li>dialogs/join-chat-room-dialog.h <span style="color: grey">(b5f7c6773f6a73f5653995a559bfab39b085e089)</span></li>

 <li>dialogs/join-chat-room-dialog.cpp <span style="color: grey">(06d0a3ae0d9cbfcb2247dd2593f9a6616613db20)</span></li>

 <li>dialogs/join-chat-room-dialog.ui <span style="color: grey">(bec08058e2ffd56255b0f6c7977980226e973390)</span></li>

 <li>rooms-model.h <span style="color: grey">(675ea4cbb02cef5d823b69b6e48b292d3b4b07cf)</span></li>

 <li>rooms-model.cpp <span style="color: grey">(8a5cbb0b8fb0eaed8b30d3b516a574d231d7876e)</span></li>

</ul>

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



<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Screenshots </h1>

<div>

 <a href="http://git.reviewboard.kde.org/r/104173/s/448/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/03/06/joinchatroom_favorites_400x100.png" style="border: 1px black solid;" alt="favorites tab" /></a>

 <a href="http://git.reviewboard.kde.org/r/104173/s/449/"><img src="http://git.reviewboard.kde.org/media/uploaded/images/2012/03/06/joinchatroom_query_400x100.png" style="border: 1px black solid;" alt="query tab" /></a>

</div>


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








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