Review Request 117543: Don't leave group chat when window is closed

Dan Vrátil dvratil at redhat.com
Sun Apr 13 16:36:53 UTC 2014



> On April 13, 2014, 6:25 p.m., David Edmundson wrote:
> > app/telepathy-chat-ui.h, line 73
> > <https://git.reviewboard.kde.org/r/117543/diff/3/?file=265230#file265230line73>
> >
> >     I don't see the point of this map.
> >     
> >     if you have a ChatTab you can do tab->channel().
> >     
> >     In the one case where you loop through the values, there's no point it being in a hash. You can just do m_channelaccountMap.value()
> >

> if you have a ChatTab you can do tab->channel().
I can't. In onTabDestroyed() I can use the pointer, because that's handler of QObject::destroyed() signal.

> In the one case where you loop through the values, there's no point it being in a hash. You can just do 
> m_channelaccountMap.value()
I decided to use QHash to improve the lookup time in onGroupChatMessageReceived(), but I can turn it into QMap if you want me to.


> On April 13, 2014, 6:25 p.m., David Edmundson wrote:
> > app/telepathy-chat-ui.cpp, line 91
> > <https://git.reviewboard.kde.org/r/117543/diff/3/?file=265231#file265231line91>
> >
> >     ChatWindow has a getTab(account, contact) method
> >     
> >     Or you can do m_channelAccountMap.contains(channel);
> >     
> >

> ChatWindow has a getTab(account, contact) method
That's useless to me. I'm not interested in whether there's a tab with sch channel, but whether the channel is cached internally.

> Or you can do m_channelAccountMap.contains(channel);
It seems that Tp::ChannelTextPtr are only compared by value of the pointer, so unless I'm guaranteed to always get the same object via handleChannels() representing the same channel as the one I already have cached (think for instance after reconnecting to network), then I can use this, but I think that the iteration check over names is much safer (that's what ChatWindow::getTab() does)


- Dan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/117543/#review55612
-----------------------------------------------------------


On April 13, 2014, 4:53 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/117543/
> -----------------------------------------------------------
> 
> (Updated April 13, 2014, 4:53 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> -------
> 
> When "Don't leave chat room when window is closed" settings is enabled, the channel is not closed when user closes the tab or window, but is maintained by TelepathyChatUi. The channel can be left via Conversation -> Leave room action.
> 
> 
> Diffs
> -----
> 
>   app/chat-window.h 72bbd1d 
>   app/chat-window.cpp a7da574 
>   app/telepathy-chat-ui.h 97bc4b7 
>   app/telepathy-chat-ui.cpp 33150b8 
>   config/behavior-config.h d57fd90 
>   config/behavior-config.cpp eeb3597 
>   config/behavior-config.ui c8e731c 
>   lib/chat-widget.cpp 3682742 
>   lib/notify-filter.h f929ce3 
>   lib/notify-filter.cpp 6807dac 
>   lib/text-chat-config.h e0ba24f 
>   lib/text-chat-config.cpp 57c7c0c 
> 
> Diff: https://git.reviewboard.kde.org/r/117543/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20140413/18d2c5a2/attachment.html>


More information about the KDE-Telepathy mailing list