Review Request: Handle favorites in JoinChatroomDialog

Martin Klapetek martin.klapetek at gmail.com
Wed Mar 7 10:29:51 UTC 2012



> On March 6, 2012, 5:18 p.m., David Edmundson wrote:
> > rooms-model.cpp, line 178
> > <http://git.reviewboard.kde.org/r/104173/diff/1/?file=52062#file52062line178>
> >
> >     .isValid() should check index < rowCount() so this isn't needed.
> >     
> >
> 
> Dominik Cermak wrote:
>     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.

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 )


- Martin


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


On March 6, 2012, 2:35 p.m., Dominik Cermak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104173/
> -----------------------------------------------------------
> 
> (Updated March 6, 2012, 2:35 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> 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).
> 
> 
> This addresses bug 291711.
>     http://bugs.kde.org/show_bug.cgi?id=291711
> 
> 
> Diffs
> -----
> 
>   dialogs/join-chat-room-dialog.h b5f7c6773f6a73f5653995a559bfab39b085e089 
>   dialogs/join-chat-room-dialog.cpp 06d0a3ae0d9cbfcb2247dd2593f9a6616613db20 
>   dialogs/join-chat-room-dialog.ui bec08058e2ffd56255b0f6c7977980226e973390 
>   rooms-model.h 675ea4cbb02cef5d823b69b6e48b292d3b4b07cf 
>   rooms-model.cpp 8a5cbb0b8fb0eaed8b30d3b516a574d231d7876e 
> 
> Diff: http://git.reviewboard.kde.org/r/104173/diff/
> 
> 
> Testing
> -------
> 
> Adding and removing favorites.
> After closing and reopening contact-list the favorites are still there.
> 
> 
> Screenshots
> -----------
> 
> favorites tab
>   http://git.reviewboard.kde.org/r/104173/s/448/
> query tab
>   http://git.reviewboard.kde.org/r/104173/s/449/
> 
> 
> Thanks,
> 
> Dominik Cermak
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120307/103ceba2/attachment.html>


More information about the KDE-Telepathy mailing list