Re: Review Request 130221: Add an option to stay online when there are other active presence controls to the contact list offline on close.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130221/#review103590 --- File Attachment: Config screenshot - Config screenshot <https://git.reviewboard.kde.org//r/130221/#fcomment616> This is very weird wording. - Aleix Pol Gonzalez On ago. 10, 2017, 5:24 p.m., James Smith wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130221/ > --- > > (Updated ago. 10, 2017, 5:24 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-kded-module > > > Description > --- > > Add an option to stay online when there are other active presence controls to > the contact list offline on close. > > > Diffs > - > > config/telepathy-kded-config.cpp 88220645c2e119dc7879cdae065cbbf06aa13329 > config/telepathy-kded-config.ui 2b80dfa34381af2e9206384a31c025f9b914854a > > Diff: https://git.reviewboard.kde.org/r/130221/diff/ > > > Testing > --- > > Compile, run. > > > File Attachments > > > Config screenshot > > https://git.reviewboard.kde.org/media/uploaded/files/2017/08/10/dee5ab39-88b4-46b7-8b8a-5114161547f1__Screenshot_20170809_211431.png > > > Thanks, > > James Smith > >
Re: Review Request 130044: Require Qt 5.7.0
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130044/#review102938 --- Ship it! Ship It! - Aleix Pol Gonzalez On mar. 23, 2017, 8:53 p.m., Niels Ole Salscheider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130044/ > --- > > (Updated mar. 23, 2017, 8:53 p.m.) > > > Review request for Telepathy, David Edmundson and Martin Klapetek. > > > Repository: ktp-text-ui > > > Description > --- > > This is necessary after 06f9110dd1f4fdc8552d48ed3d10277d4e44a152. > > Please review this before beta tagging! > > > Diffs > - > > CMakeLists.txt 403e805 > > Diff: https://git.reviewboard.kde.org/r/130044/diff/ > > > Testing > --- > > Still builds. > > > Thanks, > > Niels Ole Salscheider > >
Re: Review Request 127005: Port to QWebEngine
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127005/#review102519 --- +1 LGTM. Yes, QtWebEngine is an acceptable dependency. - Aleix Pol Gonzalez On Feb. 11, 2017, 7:32 p.m., Niels Ole Salscheider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127005/ > --- > > (Updated Feb. 11, 2017, 7:32 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > --- > > This ports the message viewer from QWebKit to QWebEngine. > > > Diffs > - > > CMakeLists.txt 86aa80a > adiumxtra-protocol-handler/CMakeLists.txt f78a62f > app/CMakeLists.txt 9a90cec > config/appearance/CMakeLists.txt dfb5d04 > config/appearance/appearance-config-tab.cpp f2f298b > lib/CMakeLists.txt 5294521 > lib/adium-theme-view.h 5a0c2e6 > lib/adium-theme-view.cpp d1c93f4 > lib/chat-search-bar.h c8c5118 > lib/chat-search-bar.cpp 484975a > lib/chat-widget.h 588407c > lib/chat-widget.cpp fdc9c1e > logviewer/CMakeLists.txt c36157c > logviewer/log-viewer.cpp b1dad26 > logviewer/message-view.h ec592c7 > logviewer/message-view.cpp 1b3bbe4 > > Diff: https://git.reviewboard.kde.org/r/127005/diff/ > > > Testing > --- > > - Builds > - Chats with the text UI work > - Links work > - The log viewer works > > > Thanks, > > Niels Ole Salscheider > >
Re: Review Request 129311: [Quick Chat] Trim text that is copied into clipboard
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129311/#review100495 --- Ship it! Ship It! - Aleix Pol Gonzalez On Nov. 2, 2016, 11 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129311/ > --- > > (Updated Nov. 2, 2016, 11 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > Especially when there's spaces at the end you can't really tell them which > makes copying passwords annoying. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 5ce61e2 > > Diff: https://git.reviewboard.kde.org/r/129311/diff/ > > > Testing > --- > > Sent some text with spaces around, they were trimmed. Spaces within text are > left alone. > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 129311: [Quick Chat] Trim text that is copied into clipboard
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129311/#review100494 --- Hm? We're talking about copying passwords on a chat? - Aleix Pol Gonzalez On Nov. 2, 2016, 11 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129311/ > --- > > (Updated Nov. 2, 2016, 11 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > Especially when there's spaces at the end you can't really tell them which > makes copying passwords annoying. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 5ce61e2 > > Diff: https://git.reviewboard.kde.org/r/129311/diff/ > > > Testing > --- > > Sent some text with spaces around, they were trimmed. Spaces within text are > left alone. > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 129069: [Quick Chat] Remove unused popupSide property
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129069/#review99657 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 29, 2016, 10:20 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129069/ > --- > > (Updated Sept. 29, 2016, 10:20 a.m.) > > > Review request for Telepathy and Aleix Pol Gonzalez. > > > Repository: ktp-desktop-applets > > > Description > --- > > This property is unused and PlasmaCore.Dialog location and visualParent > already take care of this. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ConversationDelegate.qml a089fbc > chat/org.kde.ktp-chat/contents/ui/FullChatList.qml 258c1cf > > Diff: https://git.reviewboard.kde.org/r/129069/diff/ > > > Testing > --- > > Popup location still sensible in all four panel locations > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 129052: [Chat List] Activate chat also on Enter press
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129052/#review99590 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 27, 2016, 2:52 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129052/ > --- > > (Updated Sept. 27, 2016, 2:52 p.m.) > > > Review request for Telepathy and Aleix Pol Gonzalez. > > > Repository: ktp-desktop-applets > > > Description > --- > > I usually just reach over with my thumb from my mouse. > > > Diffs > - > > contactlist/org.kde.ktp-contactlist/contents/ui/ContactList.qml fd2aca4 > > Diff: https://git.reviewboard.kde.org/r/129052/diff/ > > > Testing > --- > > Much more comfortable :) > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 129016: [ktp-auth-handler] Fixed uninitialized variable usage
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129016/#review99519 --- Ship it! :D - Aleix Pol Gonzalez On Sept. 25, 2016, 7:33 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129016/ > --- > > (Updated Sept. 25, 2016, 7:33 p.m.) > > > Review request for Telepathy and Aleix Pol Gonzalez. > > > Repository: ktp-auth-handler > > > Description > --- > > If there is no account (condition at 175 is false), then ```SignOn::Identity > *identity``` will be used without initialization at line 210. > > Bug reported by the clang static analyzer. > > Thanks to Aleix Pol Gonzalez for the idea to run the analyzer. :) > > > Diffs > - > > x-telepathy-password-auth-operation.cpp be0ed99 > > Diff: https://git.reviewboard.kde.org/r/129016/diff/ > > > Testing > --- > > Compiles > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128979/#review99400 --- Ship it! The current implementation was wird. :/ `if (!something == -1)` Wrong at so many levels... O.o - Aleix Pol Gonzalez On Sept. 21, 2016, 4:06 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128979/ > --- > > (Updated Sept. 21, 2016, 4:06 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Fixed incorrect check of adaptee method existance > > See also: https://git.reviewboard.kde.org/r/128844 > > I have no idea how I missed this at the first time. > > > Diffs > - > > otr-proxy/KTpProxy/svc-channel-proxy.cpp db5d474 > > Diff: https://git.reviewboard.kde.org/r/128979/diff/ > > > Testing > --- > > Compiles with less warnings > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128979/#review99376 --- otr-proxy/KTpProxy/svc-channel-proxy.cpp (line 63) <https://git.reviewboard.kde.org/r/128979/#comment66894> Wait, this should be >=0 - Aleix Pol Gonzalez On Sept. 21, 2016, 4:06 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128979/ > --- > > (Updated Sept. 21, 2016, 4:06 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Fixed incorrect check of adaptee method existance > > See also: https://git.reviewboard.kde.org/r/128844 > > I have no idea how I missed this at the first time. > > > Diffs > - > > otr-proxy/KTpProxy/svc-channel-proxy.cpp db5d474 > > Diff: https://git.reviewboard.kde.org/r/128979/diff/ > > > Testing > --- > > Compiles with less warnings > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)
> On Sept. 21, 2016, 5:31 p.m., Aleix Pol Gonzalez wrote: > > Ship It! Withdrawn - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128979/#review99375 --- On Sept. 21, 2016, 4:06 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128979/ > --- > > (Updated Sept. 21, 2016, 4:06 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Fixed incorrect check of adaptee method existance > > See also: https://git.reviewboard.kde.org/r/128844 > > I have no idea how I missed this at the first time. > > > Diffs > - > > otr-proxy/KTpProxy/svc-channel-proxy.cpp db5d474 > > Diff: https://git.reviewboard.kde.org/r/128979/diff/ > > > Testing > --- > > Compiles with less warnings > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128979/#review99375 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 21, 2016, 4:06 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128979/ > --- > > (Updated Sept. 21, 2016, 4:06 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Fixed incorrect check of adaptee method existance > > See also: https://git.reviewboard.kde.org/r/128844 > > I have no idea how I missed this at the first time. > > > Diffs > - > > otr-proxy/KTpProxy/svc-channel-proxy.cpp db5d474 > > Diff: https://git.reviewboard.kde.org/r/128979/diff/ > > > Testing > --- > > Compiles with less warnings > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128976: [Contact List Applet] Drop custom compact representation
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128976/#review99359 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 21, 2016, 3:12 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128976/ > --- > > (Updated Sept. 21, 2016, 3:12 p.m.) > > > Review request for Telepathy, Aleix Pol Gonzalez and David Edmundson. > > > Repository: ktp-desktop-applets > > > Description > --- > > All it did was show an icon with a MouseArea, we can just set the > plasmoid.icon instead > > > Diffs > - > > contactlist/org.kde.ktp-contactlist/contents/ui/Presence.qml 575d97f > contactlist/org.kde.ktp-contactlist/contents/ui/main.qml 43b8c8e > > Diff: https://git.reviewboard.kde.org/r/128976/diff/ > > > Testing > --- > > I now get proper hover effect on the tray icon and it still works because > DefaultCompactRepresentation is also just an IconItem > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 128961: [KTp/Declarative/MessagesModel] Fixed allocation of modelIndex variable in onMessageSent()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128961/#review99331 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 20, 2016, 3:34 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128961/ > --- > > (Updated Sept. 20, 2016, 3:34 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > We actually story the index, so we should not just reference it. > > > Diffs > - > > KTp/Declarative/messages-model.cpp 6823574 > > Diff: https://git.reviewboard.kde.org/r/128961/diff/ > > > Testing > --- > > It compiles. > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128960: [KTp/Declarative/MessagesModel] Use message token to prevent message duplication on scrollback received
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128960/#review99330 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 20, 2016, 3:27 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128960/ > --- > > (Updated Sept. 20, 2016, 3:27 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Sent and received messages can be received again as scrollback and before > this patch it leads to messages duplication. > E.g.: server sent last 20 messages on connection to a group chat, the local > user has a network issue and thus got disconnected and connected back with > messaging window kept open. > > We already have a table for sent messages and this patch: > 1) adds insertion of received messages into the same table, > 2) uses the table to ignore already sent/received messages. > > In future we can also use the table to implement message correction (see > XEP-0308, Message_Header_Key "supersedes" and so on). > > > Diffs > - > > KTp/Declarative/messages-model.cpp 6823574 > > Diff: https://git.reviewboard.kde.org/r/128960/diff/ > > > Testing > --- > > Sent and received messages are not duplicated on scrollback received. > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128960: [KTp/Declarative/MessagesModel] Use message token to prevent message duplication on scrollback received
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128960/#review99328 --- Fix it, then Ship it! I was not aware of this QPersistentModel hash, should probably look into it in the future. KTp/Declarative/messages-model.cpp (line 245) <https://git.reviewboard.kde.org/r/128960/#comment66877> this shouldn't be a reference, as you're actually storing it. - Aleix Pol Gonzalez On Sept. 20, 2016, 3:09 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128960/ > --- > > (Updated Sept. 20, 2016, 3:09 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Sent and received messages can be received again as scrollback and before > this patch it leads to messages duplication. > E.g.: server sent last 20 messages on connection to a group chat, the local > user has a network issue and thus got disconnected and connected back with > messaging window kept open. > > We already have a table for sent messages and this patch: > 1) adds insertion of received messages into the same table, > 2) uses the table to ignore already sent/received messages. > > In future we can also use the table to implement message correction (see > XEP-0308, Message_Header_Key "supersedes" and so on). > > > Diffs > - > > KTp/Declarative/messages-model.cpp 6823574 > > Diff: https://git.reviewboard.kde.org/r/128960/diff/ > > > Testing > --- > > Sent and received messages are not duplicated on scrollback received. > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128954: [KTp/Declarative/MessagesModel] Implemented message sort by sent timestamp
> On Sept. 20, 2016, 12:06 p.m., Aleix Pol Gonzalez wrote:
> > KTp/Declarative/messages-model.cpp, line 222
> > <https://git.reviewboard.kde.org/r/128954/diff/1/?file=477161#file477161line222>
> >
> > Use iterators?
>
> Alexandr Akulich wrote:
> How to get an index from iterator?
>
> Aleix Pol Gonzalez wrote:
> You can count in parallel without calling `QList::operator[]` on the
> index.
>
> Alexandr Akulich wrote:
> Actually I call ```const T (int i) const;```. And what is the reason
> to use iterators, if you still suggest to use a counter? Do you actually
> think that
> ```
> int i = d->messages.count() - 1;
> for (auto it = d->messages.constEnd(); it !=
> d->messages.constBegin(); --it, --i) {
> if (sentTimestamp > it->message.time()) {
> newMessageIndex = i;
> break;
> }
> // Or do you suggest to place --i; here?
> }
> ```
> is more readable, than plain
> ```
> for (int i = d->messages.count() - 1; i >= 0; --i) {
> if (sentTimestamp > d->messages.at(i).message.time()) {
> newMessageIndex = i;
> break;
> }
> }
> ```
> ?
>
> IMO it is a common practice to use iterators if you don't need index (and
> just use the index otherwise). It is a bit more optimal and I don't made an
> error in the condition.
>
> Aleix Pol Gonzalez wrote:
> You can use reverse iterators:
> http://doc.qt.io/qt-5/qvector.html#crbegin
>
> Alexandr Akulich wrote:
> I know about the reverse methods and it is not any better without some
> reverse adaptor in QList.
>
> int i = d->messages.count() - 1;
> for (auto it = d->messages.crbegin(); it != d->messages.crend();
> ++it, --i) {
> if (sentTimestamp > it->message.time()) {
> newMessageIndex = i;
> break;
> }
> // Or do you suggest to place --i; here?
> }
>
> Why it is better? It reads worse and it looks like "iterators for the
> iterators" without a real benefits. It does not save us from an error,
> because we need the (reverse) counter. Do we have a coding convention for
> this case?
>
> Aleix Pol Gonzalez wrote:
> I'd say in general the coding convention is to use iterators whenever
> possible, since we can ensure the access to the random object will be
> optimal. If you really feel like this is worse, feel free to use something
> else. I didn't come here to play sargeant.
>
> Alexandr Akulich wrote:
> I'm sorry if I sounded rude or unfriendly.
>
> Iterators can indeed save us from the assert in QList::at(), but I think
> that in this particular case the code will be more complex, harder to
> understand and thus error-prone: we use reverse iterators, which we have to
> increase and at the same time decrease the counter. If there would be no
> beginInsertRows() call, I would be all for use iterators.
>
> In this case we can follow the KISS rule without performance penalty.
>
> By the way, I can not find anything related to iterators usage in "Qt
> Coding Style", "Qt Coding Conventions" or "Frameworks Coding Style" (which
> seems to be a successor of Kdelibs Coding Style). The only rule is "Don't mix
> const and non-const iterators." Probably we need to update the convention(s)
> with C++11 things.
>
> I will remove the "obvious" comment and push this change, if you agree. I
> would like to upload one may be "more discussable" change, which depends on
> this one.
>
> And anyway, thanks for the feedback!
Ok, do that.
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128954/#review99301
---
On Sept. 20, 2016, 11:51 a.m., Alexandr Akulich wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128954/
> ---
>
> (Updated Sept. 20, 2016, 11:51 a.m.)
>
>
> Review request for Telepathy.
>
>
> Repository: ktp-common-internals
>
>
> Description
> ---
>
> Implemented message sort by sent timestamp (if available).
>
> This fixes order of scrollback messages.
>
>
> Diffs
> -
>
> KTp/Declarative/messages-model.cpp dc1088c
>
> Diff: https://git.reviewboard.kde.org/r/128954/diff/
>
>
> Testing
> ---
>
> Works, fixes the issue.
>
>
> Thanks,
>
> Alexandr Akulich
>
>
Re: Review Request 128954: [KTp/Declarative/MessagesModel] Implemented message sort by sent timestamp
> On Sept. 20, 2016, 12:06 p.m., Aleix Pol Gonzalez wrote:
> > KTp/Declarative/messages-model.cpp, line 222
> > <https://git.reviewboard.kde.org/r/128954/diff/1/?file=477161#file477161line222>
> >
> > Use iterators?
>
> Alexandr Akulich wrote:
> How to get an index from iterator?
>
> Aleix Pol Gonzalez wrote:
> You can count in parallel without calling `QList::operator[]` on the
> index.
>
> Alexandr Akulich wrote:
> Actually I call ```const T (int i) const;```. And what is the reason
> to use iterators, if you still suggest to use a counter? Do you actually
> think that
> ```
> int i = d->messages.count() - 1;
> for (auto it = d->messages.constEnd(); it !=
> d->messages.constBegin(); --it, --i) {
> if (sentTimestamp > it->message.time()) {
> newMessageIndex = i;
> break;
> }
> // Or do you suggest to place --i; here?
> }
> ```
> is more readable, than plain
> ```
> for (int i = d->messages.count() - 1; i >= 0; --i) {
> if (sentTimestamp > d->messages.at(i).message.time()) {
> newMessageIndex = i;
> break;
> }
> }
> ```
> ?
>
> IMO it is a common practice to use iterators if you don't need index (and
> just use the index otherwise). It is a bit more optimal and I don't made an
> error in the condition.
>
> Aleix Pol Gonzalez wrote:
> You can use reverse iterators:
> http://doc.qt.io/qt-5/qvector.html#crbegin
>
> Alexandr Akulich wrote:
> I know about the reverse methods and it is not any better without some
> reverse adaptor in QList.
>
> int i = d->messages.count() - 1;
> for (auto it = d->messages.crbegin(); it != d->messages.crend();
> ++it, --i) {
> if (sentTimestamp > it->message.time()) {
> newMessageIndex = i;
> break;
> }
> // Or do you suggest to place --i; here?
> }
>
> Why it is better? It reads worse and it looks like "iterators for the
> iterators" without a real benefits. It does not save us from an error,
> because we need the (reverse) counter. Do we have a coding convention for
> this case?
I'd say in general the coding convention is to use iterators whenever possible,
since we can ensure the access to the random object will be optimal. If you
really feel like this is worse, feel free to use something else. I didn't come
here to play sargeant.
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128954/#review99301
---
On Sept. 20, 2016, 11:51 a.m., Alexandr Akulich wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128954/
> ---
>
> (Updated Sept. 20, 2016, 11:51 a.m.)
>
>
> Review request for Telepathy.
>
>
> Repository: ktp-common-internals
>
>
> Description
> ---
>
> Implemented message sort by sent timestamp (if available).
>
> This fixes order of scrollback messages.
>
>
> Diffs
> -
>
> KTp/Declarative/messages-model.cpp dc1088c
>
> Diff: https://git.reviewboard.kde.org/r/128954/diff/
>
>
> Testing
> ---
>
> Works, fixes the issue.
>
>
> Thanks,
>
> Alexandr Akulich
>
>
Re: Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128847/#review99309 --- I'd say that if these changes fit you, they should go in. I honestly never used ktp-debugger for more than 1 minute straight. - Aleix Pol Gonzalez On Sept. 6, 2016, 4:54 p.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128847/ > --- > > (Updated Sept. 6, 2016, 4:54 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > The main goal of this change is to split logic and UI parts > > This is the first step in direction to debugger, which: > 1) works with any Telepathy process with DebugInterface support; > 2) detects new processess "on fly"; > 3) has no hardcoded services; > 4) shows one process just once, independently of number of dbus services, > registered by the process. > > The change also opens a way to a QML-based UI at some point in future. > > Questionable thing is the "TelepathyProcess" class name. > TelepathyService does not fit, because: > 1) Single process can expose a number of services (e.g. MissionControl), > 2) The debug interface is applicable to any telepathy application, including > clients, so word "Service" (which is not associated with clients) would > mislead. > > > I uploaded a draft of "second step" to my scratch repo: > https://quickgit.kde.org/?p=scratch%2Fakulichalexandr%2Fktp-common-internals.git=commitdiff=7e07b65f330d85527c9a6b014154527f7e3e7c01=db202a7143be88db37e056913a88992fe7ce507d > > I will make a ReviewRequest with the second part on this (split) commit > landed. > > > Diffs > - > > tools/debugger/CMakeLists.txt e35de89 > tools/debugger/debug-message-view.h ae745db > tools/debugger/debug-message-view.cpp ea09d79 > tools/debugger/main-window.cpp 490f803 > tools/debugger/telepathy-process.h PRE-CREATION > tools/debugger/telepathy-process.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128847/diff/ > > > Testing > --- > > Works as previously. > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128954: [KTp/Declarative/MessagesModel] Implemented message sort by sent timestamp
> On Sept. 20, 2016, 12:06 p.m., Aleix Pol Gonzalez wrote:
> > KTp/Declarative/messages-model.cpp, line 222
> > <https://git.reviewboard.kde.org/r/128954/diff/1/?file=477161#file477161line222>
> >
> > Use iterators?
>
> Alexandr Akulich wrote:
> How to get an index from iterator?
>
> Aleix Pol Gonzalez wrote:
> You can count in parallel without calling `QList::operator[]` on the
> index.
>
> Alexandr Akulich wrote:
> Actually I call ```const T (int i) const;```. And what is the reason
> to use iterators, if you still suggest to use a counter? Do you actually
> think that
> ```
> int i = d->messages.count() - 1;
> for (auto it = d->messages.constEnd(); it !=
> d->messages.constBegin(); --it, --i) {
> if (sentTimestamp > it->message.time()) {
> newMessageIndex = i;
> break;
> }
> // Or do you suggest to place --i; here?
> }
> ```
> is more readable, than plain
> ```
> for (int i = d->messages.count() - 1; i >= 0; --i) {
> if (sentTimestamp > d->messages.at(i).message.time()) {
> newMessageIndex = i;
> break;
> }
> }
> ```
> ?
>
> IMO it is a common practice to use iterators if you don't need index (and
> just use the index otherwise). It is a bit more optimal and I don't made an
> error in the condition.
You can use reverse iterators:
http://doc.qt.io/qt-5/qvector.html#crbegin
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128954/#review99301
---
On Sept. 20, 2016, 11:51 a.m., Alexandr Akulich wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128954/
> ---
>
> (Updated Sept. 20, 2016, 11:51 a.m.)
>
>
> Review request for Telepathy.
>
>
> Repository: ktp-common-internals
>
>
> Description
> ---
>
> Implemented message sort by sent timestamp (if available).
>
> This fixes order of scrollback messages.
>
>
> Diffs
> -
>
> KTp/Declarative/messages-model.cpp dc1088c
>
> Diff: https://git.reviewboard.kde.org/r/128954/diff/
>
>
> Testing
> ---
>
> Works, fixes the issue.
>
>
> Thanks,
>
> Alexandr Akulich
>
>
Re: Review Request 128867: [KTp/Message] Direction of received message now depends on sender
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128867/#review99307 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 9, 2016, 12:18 a.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128867/ > --- > > (Updated Sept. 9, 2016, 12:18 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > If the sender is selfContact, then the direction should be LocalToRemote even > for ReceivedMessages. > > With this fix Scrollback feature in ktp-text-ui starts to partially work. > > Remaining issues (with scrollback): > - If ktp-proxy is on, then sender of received message forced to the channel > target, so all received messages would be "incoming"; > - Messages in TextUi appears in order of receiving. This means that if > scrollback received from the newer to older messages, then the older message > would be showed at the end (like if it was been just sent). > > > Diffs > - > > KTp/message.cpp 566241e > > Diff: https://git.reviewboard.kde.org/r/128867/diff/ > > > Testing > --- > > Compiles, runs. Fixes scrollback for 40% (30% and 30% are broken ktp-proxy > and buggy ktp-text-ui, which shows messages in straight instead of > chronological message order). > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128954: [KTp/Declarative/MessagesModel] Implemented message sort by sent timestamp
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128954/#review99301 --- KTp/Declarative/messages-model.cpp (line 221) <https://git.reviewboard.kde.org/r/128954/#comment66862> I'd remove the comment, I'd say it's obvious that this needs to be iterated backwards. KTp/Declarative/messages-model.cpp (line 222) <https://git.reviewboard.kde.org/r/128954/#comment66861> Use iterators? - Aleix Pol Gonzalez On Sept. 20, 2016, 11:51 a.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128954/ > --- > > (Updated Sept. 20, 2016, 11:51 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > Implemented message sort by sent timestamp (if available). > > This fixes order of scrollback messages. > > > Diffs > - > > KTp/Declarative/messages-model.cpp dc1088c > > Diff: https://git.reviewboard.kde.org/r/128954/diff/ > > > Testing > --- > > Works, fixes the issue. > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128844: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128844/#review98928 --- We shouldn't have generated code in git... Pragmatic +1 - Aleix Pol Gonzalez On Sept. 6, 2016, 11:37 a.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128844/ > --- > > (Updated Sept. 6, 2016, 11:37 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-common-internals > > > Description > --- > > ProxyServiceAdaptor seems to be generated by outdated qt-svc-gen.py from > telepathy-qt. > > > Diffs > - > > otr-proxy/KTpProxy/svc-proxy-service.cpp 53db454 > > Diff: https://git.reviewboard.kde.org/r/128844/diff/ > > > Testing > --- > > Compiles without warnings > > > Thanks, > > Alexandr Akulich > >
Re: Review Request 128561: port salut plugin to kf5
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128561/#review98107 --- Fix it, then Ship it! The patch looks good, I'm unsure why this is not working, TBH. I'd say merge it in and then we can look into figuring out why isn't signond finding the provider. data/kaccounts/ktp-salut.provider.in (line 5) <https://git.reviewboard.kde.org/r/128561/#comment66075> Remove trailing spaces - Aleix Pol Gonzalez On July 31, 2016, 1:28 a.m., Björn Bidar wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128561/ > --- > > (Updated July 31, 2016, 1:28 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-accounts-kcm > > > Description > --- > > port salut plugin to kf5 > > > Diffs > - > > data/kaccounts/ktp-salut-im.service.in PRE-CREATION > data/kaccounts/ktp-salut.provider.in PRE-CREATION > plugins/CMakeLists.txt db9bfc458cf1bb9e06d0402d6a03b6c320fbcc3e > plugins/salut/CMakeLists.txt 32a428e8237ee15053255a7f51bf7059abe4c33e > plugins/salut/salut-account-ui-plugin.cpp > 79c969a86c9fc27b873fcc04eefb9842ec260918 > plugins/salut/salut-advanced-options-widget.ui > 9b05f768be1d855221b4941e99b6fbcd0cc4dcae > plugins/salut/salut-main-options-widget.ui > 75d74e3497dc03f65675de1bbbacdc041447536f > > Diff: https://git.reviewboard.kde.org/r/128561/diff/ > > > Testing > --- > > I ported the plugin to mimic the changes made to the other plugins, however I > get the message that the handler for this protocoll isn't installed althrough > it is. > Need some help on this. > > > Thanks, > > Björn Bidar > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128525: [Quick Chat] Fix looping over dropped URLs
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128525/#review97849 --- Ship it! Ship It! - Aleix Pol Gonzalez On July 26, 2016, 10:49 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128525/ > --- > > (Updated July 26, 2016, 10:49 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > The for..in loop didn't properly iterate the array of URLs (JavaScript...), > replacing it by a traditional for loop fixes file transfers. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ConversationDelegateButton.qml 79b4f4a > > Diff: https://git.reviewboard.kde.org/r/128525/diff/ > > > Testing > --- > > Together with Review 128524 this fixes dropping files onto chat icons in my > panel to send files. Tested with both a single and multiple files. > > > Thanks, > > Kai Uwe Broulik > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128138: [Quick Chat] Allow cycling through chats with Ctrl+(Shift)+Tab
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128138/#review97529 --- Ship it! Ship It! - Aleix Pol Gonzalez On June 9, 2016, 3:13 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128138/ > --- > > (Updated June 9, 2016, 3:13 p.m.) > > > Review request for Telepathy and KDE Usability. > > > Repository: ktp-desktop-applets > > > Description > --- > > This makes switching between conversations possible without using the mouse > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 1485c4c > chat/org.kde.ktp-chat/contents/ui/ConversationDelegate.qml 701f721 > > Diff: https://git.reviewboard.kde.org/r/128138/diff/ > > > Testing > --- > > Pressed Ctrl+Tab multiple times -> got to the next conversation until the > last one where it wrap around and select the first one again > Ctrl+Shift+Tab works the same way, just in the other direction. > > > Thanks, > > Kai Uwe Broulik > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128460/#review97439 --- I really like the idea! filters/geopoint/geopoint-filter.cpp (line 34) <https://git.reviewboard.kde.org/r/128460/#comment65742> Can we maybe use something that is more free software friendly than Google Maps? - Aleix Pol Gonzalez On July 15, 2016, 11:18 a.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128460/ > --- > > (Updated July 15, 2016, 11:18 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > --- > > Added a geopoint filter which adds an iframe with a map for each > geo:<double,double> in message. > > I tried to use openstreetmap, but had no success. I would like to add an > option to select osm or google maps, but I have no time to do it now. > > > Diffs > - > > filters/CMakeLists.txt 8118b13 > filters/geopoint/CMakeLists.txt PRE-CREATION > filters/geopoint/geopoint-filter.h PRE-CREATION > filters/geopoint/geopoint-filter.cpp PRE-CREATION > filters/geopoint/ktptextui_message_filter_geopoint.desktop.cmake > PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128460/diff/ > > > Testing > --- > > It works for me. > > > Thanks, > > Alexandr Akulich > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128459: ktp-text-ui: Cleanup headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128459/#review97438 --- Ship it! Ship It! - Aleix Pol Gonzalez On July 15, 2016, 10:51 a.m., Alexandr Akulich wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128459/ > --- > > (Updated July 15, 2016, 10:51 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > --- > > Removed unneeded includes > > > Diffs > - > > app/chat-tab.h 92bf8fe > app/chat-tab.cpp bf18945 > app/chat-window.h 0d1e0bb > app/chat-window.cpp babcb57 > > Diff: https://git.reviewboard.kde.org/r/128459/diff/ > > > Testing > --- > > It compiles. > > > Thanks, > > Alexandr Akulich > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128138: [Quick Chat] Allow cycling through chats with Ctrl+(Shift)+Tab
> On June 9, 2016, 4:06 p.m., Aleix Pol Gonzalez wrote:
> > you want to switch to conversations without them having to be active.
> >
> > how are users going to learn about the shortcut?
>
> Kai Uwe Broulik wrote:
> Ktp-text-ui also has tabs which you can switch by Ctrl+Tab.
>
> Thomas Pfeiffer wrote:
> Actually I think this has become enough of a learned behavior from tabbed
> interfaces by now that people might at least try it if they want to switch.
> It's also how you switch between conversations in Ktp-Text-UI, after all.
Would it make sense to get these into `Actions {}` and show them in the
plasmoid context menu? There we'd see the shortcut.
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128138/#review96321
---
On June 9, 2016, 3:13 p.m., Kai Uwe Broulik wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128138/
> ---
>
> (Updated June 9, 2016, 3:13 p.m.)
>
>
> Review request for Telepathy and KDE Usability.
>
>
> Repository: ktp-desktop-applets
>
>
> Description
> ---
>
> This makes switching between conversations possible without using the mouse
>
>
> Diffs
> -
>
> chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 1485c4c
> chat/org.kde.ktp-chat/contents/ui/ConversationDelegate.qml 701f721
>
> Diff: https://git.reviewboard.kde.org/r/128138/diff/
>
>
> Testing
> ---
>
> Pressed Ctrl+Tab multiple times -> got to the next conversation until the
> last one where it wrap around and select the first one again
> Ctrl+Shift+Tab works the same way, just in the other direction.
>
>
> Thanks,
>
> Kai Uwe Broulik
>
>
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128138: [Quick Chat] Allow cycling through chats with Ctrl+(Shift)+Tab
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128138/#review96321 --- you want to switch to conversations without them having to be active. how are users going to learn about the shortcut? - Aleix Pol Gonzalez On June 9, 2016, 3:13 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128138/ > --- > > (Updated June 9, 2016, 3:13 p.m.) > > > Review request for Telepathy and KDE Usability. > > > Repository: ktp-desktop-applets > > > Description > --- > > This makes switching between conversations possible without using the mouse > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 1485c4c > chat/org.kde.ktp-chat/contents/ui/ConversationDelegate.qml 701f721 > > Diff: https://git.reviewboard.kde.org/r/128138/diff/ > > > Testing > --- > > Pressed Ctrl+Tab multiple times -> got to the next conversation until the > last one where it wrap around and select the first one again > Ctrl+Shift+Tab works the same way, just in the other direction. > > > Thanks, > > Kai Uwe Broulik > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128137: [Quick Chat] Handle when ConversationsModel activeChatIndex changes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128137/#review96320 --- Ship it! Right, I misread the description, I guess it's correct then. - Aleix Pol Gonzalez On June 9, 2016, 2:33 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128137/ > --- > > (Updated June 9, 2016, 2:33 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > When a conversation with a new contact is opened, the chat expands > automatically. However, if a conversation is already open, and you select the > contact in the contact list, nothing happens. This patch fixes this. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/FullChatList.qml bd28eaf > > Diff: https://git.reviewboard.kde.org/r/128137/diff/ > > > Testing > --- > > Start plasmashell, then: > * Chose a contact from my chat list plasmoid -> got a popup > * Chose the same contact again -> previously nothing would happen, now I get > a poup again > * Had the contact write something to me -> popup did not open but got an > "unread" badge as it should be > > > Thanks, > > Kai Uwe Broulik > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 128137: [Quick Chat] Handle when ConversationsModel activeChatIndex changes
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128137/#review96317 --- You don't really want your focus to change as soon as a new conversation starts. It should stay notified and the user can consider it as soon as she wants. - Aleix Pol Gonzalez On June 9, 2016, 2:33 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128137/ > --- > > (Updated June 9, 2016, 2:33 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > When a conversation with a new contact is opened, the chat expands > automatically. However, if a conversation is already open, and you select the > contact in the contact list, nothing happens. This patch fixes this. > > > Diffs > - > > chat/org.kde.ktp-chat/contents/ui/FullChatList.qml bd28eaf > > Diff: https://git.reviewboard.kde.org/r/128137/diff/ > > > Testing > --- > > Start plasmashell, then: > * Chose a contact from my chat list plasmoid -> got a popup > * Chose the same contact again -> previously nothing would happen, now I get > a poup again > * Had the contact write something to me -> popup did not open but got an > "unread" badge as it should be > > > Thanks, > > Kai Uwe Broulik > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 127771: KTp qml applet tooltip improvements.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127771/#review94937 --- Ship it! Ship It! - Aleix Pol Gonzalez On April 27, 2016, 10:20 p.m., James Smith wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127771/ > --- > > (Updated April 27, 2016, 10:20 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-desktop-applets > > > Description > --- > > 1)Show the presence message in the tooltip if there is one. > 2)Indicate if there are no accounts online in the tooltip. > 3)Fix the tooltip showing the wrong state, e.g. Connecting... when connected, > changing presence to available... when available. > > > Diffs > - > > contactlist/org.kde.ktp-contactlist/contents/ui/main.qml > 43b8c8ebeef8c018450719fba3ac06e42434e326 > > Diff: https://git.reviewboard.kde.org/r/127771/diff/ > > > Testing > --- > > Compile, run. > > > Thanks, > > James Smith > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 127007: logviewer: Fix signals of search field
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127007/#review92136 --- logviewer/log-viewer.cpp (line 118) <https://git.reviewboard.kde.org/r/127007/#comment62853> const& doesn't belong within SLOT() and SIGNAL() - Aleix Pol Gonzalez On Feb. 7, 2016, 9:26 p.m., Niels Ole Salscheider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127007/ > --- > > (Updated Feb. 7, 2016, 9:26 p.m.) > > > Review request for Telepathy. > > > Repository: ktp-text-ui > > > Description > --- > > This fixes the search in the log viewer. > > > Diffs > - > > logviewer/log-viewer.h e6b6d3dbab67ceb3286bd402121e1b0504e8a0af > logviewer/log-viewer.cpp 1ae0f7b87b0d8fee0587215fec38fac2af047b9d > > Diff: https://git.reviewboard.kde.org/r/127007/diff/ > > > Testing > --- > > - Builds > - Searching works again > > > Thanks, > > Niels Ole Salscheider > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 126353: Allow passing relative paths to the services cmake function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126353/ --- (Updated Dec. 21, 2015, 1:20 p.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Changes --- Submitted with commit f68079c11b352d3ac91277c987ac62e965b4a4ab by Aleix Pol to branch master. Repository: kaccounts-integration Description --- Otherwise we used to get an odd and silent warning. Also be more verbose when the command fails. Diffs - src/lib/KAccountsMacros.cmake dfcb636 Diff: https://git.reviewboard.kde.org/r/126353/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 126353: Allow passing relative paths to the services cmake function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126353/ --- (Updated Dec. 15, 2015, 3:29 p.m.) Review request for KDEPIM and Telepathy. Changes --- Adressed mck's issues. Repository: kaccounts-integration Description --- Otherwise we used to get an odd and silent warning. Also be more verbose when the command fails. Diffs (updated) - src/lib/KAccountsMacros.cmake dfcb636 Diff: https://git.reviewboard.kde.org/r/126353/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 126353: Allow passing relative paths to the services cmake function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126353/ --- Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- Otherwise we used to get an odd and silent warning. Also be more verbose when the command fails. Diffs - src/lib/KAccountsMacros.cmake dfcb636 Diff: https://git.reviewboard.kde.org/r/126353/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 125457: Add missing include
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125457/#review86165 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 30, 2015, 8:29 a.m., Niels Ole Salscheider wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125457/ > --- > > (Updated Sept. 30, 2015, 8:29 a.m.) > > > Review request for Telepathy. > > > Repository: ktp-call-ui > > > Description > --- > > Add missing include > > > Diffs > - > > libktpcall/private/phonon-integration.cpp 9db6a71 > > Diff: https://git.reviewboard.kde.org/r/125457/diff/ > > > Testing > --- > > It does not build (on current Exherbo with Qt 5.5) without the include, but > it does with. > > > Thanks, > > Niels Ole Salscheider > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 125008: Reenable tts-filter and make it use QtSpeech instead of dying KSpeech api.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125008/#review84689 --- Ship it! Ship It! - Aleix Pol Gonzalez On Sept. 1, 2015, 3:46 a.m., Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125008/ > --- > > (Updated Sept. 1, 2015, 3:46 a.m.) > > > Review request for Telepathy and David Edmundson. > > > Repository: ktp-text-ui > > > Description > --- > > Also made it build by adding the #include "tts-filter.moc", I guess it's been > disabled for a little while. > > > Diffs > - > > CMakeLists.txt e88f6faabfddf9480a080cae8bf23c34994b92d7 > filters/CMakeLists.txt 52b3a790c632b2d53b670aa8d7c4d396cf89f475 > filters/texttospeech/CMakeLists.txt > f3aaff84cafc321ae8c0a633c2c7bdfea8a4bdd8 > filters/texttospeech/tts-filter.cpp > bb9c7a5cf0f9174f6041c64f2a7d84ecfcb3f9e7 > > Diff: https://git.reviewboard.kde.org/r/125008/diff/ > > > Testing > --- > > It builds. I didn't go to the point of setting up a telepathy account to test > this with just yet, but will if the code looks ok. > > > Thanks, > > Jeremy Whiting > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 125008: Reenable tts-filter and make it use QtSpeech instead of dying KSpeech api.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125008/#review84677 --- CMakeLists.txt (line 22) <https://git.reviewboard.kde.org/r/125008/#comment58593> Maybe better to have this in filters/texttospeech/CMakeLists.txt. Looks good! - Aleix Pol Gonzalez On Aug. 31, 2015, 11:56 p.m., Jeremy Whiting wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125008/ > --- > > (Updated Aug. 31, 2015, 11:56 p.m.) > > > Review request for Telepathy and David Edmundson. > > > Repository: ktp-text-ui > > > Description > --- > > Also made it build by adding the #include "tts-filter.moc", I guess it's been > disabled for a little while. > > > Diffs > - > > CMakeLists.txt e88f6faabfddf9480a080cae8bf23c34994b92d7 > filters/CMakeLists.txt 52b3a790c632b2d53b670aa8d7c4d396cf89f475 > filters/texttospeech/CMakeLists.txt > f3aaff84cafc321ae8c0a633c2c7bdfea8a4bdd8 > filters/texttospeech/tts-filter.cpp > bb9c7a5cf0f9174f6041c64f2a7d84ecfcb3f9e7 > > Diff: https://git.reviewboard.kde.org/r/125008/diff/ > > > Testing > --- > > It builds. I didn't go to the point of setting up a telepathy account to test > this with just yet, but will if the code looks ok. > > > Thanks, > > Jeremy Whiting > > ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 114574: KeepAwake plugin for KTp
On July 11, 2015, 11:55 p.m., Aleix Pol Gonzalez wrote: Controlling the system's powermanagement profile with KTp state doesn't sound very logical to me. I could agree that being able to specify you're in front of the computer could be cool, but this doesn't seem to be the way. :/ James Smith wrote: Mostly useful is that it doesn't shut down when the IM presence specifically requires an operational KTp,the IM presence for the most part being conscienciously set anyway. This closely matches the behaviour of watching a movie or listening to a music track which is also a consciencious action, and also inhibits power suspension. I didn't say it's not useful, I said it's not obvious. It's not the kind of behaviour to expect. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114574/#review82367 --- On July 11, 2015, 10:45 p.m., James Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114574/ --- (Updated July 11, 2015, 10:45 p.m.) Review request for Telepathy. Repository: ktp-kded-module Description --- KeepAwake plugin inhibits suspend for certain IM states. Also adds infrastructure for presence-responsive plugins. Diffs - keepawake.cpp PRE-CREATION status-handler.cpp 4b9c25a2ccba451f6e608bb704626e33149108cc telepathy-module.cpp 3c34b6e5e0364334c962b4df0dffc70cffed91bc config/telepathy-kded-config.cpp a4b890ce05c35f58c8a446b8fc8151727f174355 config/telepathy-kded-config.ui 93c06dc74b4dcb37e0473d0debfb5e738a24afa9 keepawake.h PRE-CREATION CMakeLists.txt a5317b480f2013a1c227c1c7f2da85cad13a64b3 config/telepathy-kded-config.h 0400626f205468b1ac9c6964d96a9644ceec32c3 Diff: https://git.reviewboard.kde.org/r/114574/diff/ Testing --- Compile, runtime. Thanks, James Smith ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123938: Don't add send delivery messages in the messages vector
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123938/ --- (Updated July 20, 2015, 1:28 p.m.) Status -- This change has been discarded. Review request for Telepathy. Repository: ktp-common-internals Description --- Currently, when using Telegram, we keep getting empty lines, this skips them. OTOH, it _really_ looks like a bug in the CM, as these messages should go through the received messages rather than the sent, nevertheless I'm unsure if the patch should still go in. Diffs - KTp/Declarative/messages-model.cpp dc1088c Diff: https://git.reviewboard.kde.org/r/123938/diff/ Testing --- The chat plasmoid works fine. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124368: Add KPeople.PersonsSortFilterProxyModel into the qml plugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124368/#review82554 --- Ship it! Ship It! - Aleix Pol Gonzalez On July 15, 2015, 10:14 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124368/ --- (Updated July 15, 2015, 10:14 p.m.) Review request for Plasma and Telepathy. Repository: kpeople Description --- So that it can be used from qml Diffs - src/declarative/peopleqmlplugin.cpp fa84764 Diff: https://git.reviewboard.kde.org/r/124368/diff/ Testing --- Works fine from qml Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 114574: KeepAwake plugin for KTp
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114574/#review82367 --- Controlling the system's powermanagement profile with KTp state doesn't sound very logical to me. I could agree that being able to specify you're in front of the computer could be cool, but this doesn't seem to be the way. :/ - Aleix Pol Gonzalez On July 11, 2015, 10:45 p.m., James Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/114574/ --- (Updated July 11, 2015, 10:45 p.m.) Review request for Telepathy. Repository: ktp-kded-module Description --- KeepAwake plugin inhibits suspend for certain IM states. Also adds infrastructure for presence-responsive plugins. Diffs - keepawake.cpp PRE-CREATION status-handler.cpp 4b9c25a2ccba451f6e608bb704626e33149108cc telepathy-module.cpp 3c34b6e5e0364334c962b4df0dffc70cffed91bc config/telepathy-kded-config.cpp a4b890ce05c35f58c8a446b8fc8151727f174355 config/telepathy-kded-config.ui 93c06dc74b4dcb37e0473d0debfb5e738a24afa9 keepawake.h PRE-CREATION CMakeLists.txt a5317b480f2013a1c227c1c7f2da85cad13a64b3 config/telepathy-kded-config.h 0400626f205468b1ac9c6964d96a9644ceec32c3 Diff: https://git.reviewboard.kde.org/r/114574/diff/ Testing --- Compile, runtime. Thanks, James Smith ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124310: Improve quick chat widget
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/#review82287 --- chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml (line 91) https://git.reviewboard.kde.org/r/124310/#comment56664 I would rather not. We used to have this scrollbar. It doesn't add anything to the plate, it takes space and it did't work all that well. chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml (line 142) https://git.reviewboard.kde.org/r/124310/#comment56665 don't remove followConversation, we need it to go false only when the user moves the flickable. Sending a message or receiving an image can do that as well. - Aleix Pol Gonzalez On July 9, 2015, 8:03 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/ --- (Updated July 9, 2015, 8:03 p.m.) Review request for Plasma and Telepathy. Repository: ktp-desktop-applets Description --- This improves the chat widget by: - Not hardcoding the hader size - Using a heading for the name so it stands out a bit more (since it's often not obvious at a glance who you're writing to) - Giving it a proper scroll bar - Making the auto scrolling a bit more robust - Cleaing up the code a bit Diffs - chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 4e802c7 Diff: https://git.reviewboard.kde.org/r/124310/diff/ Testing --- Been using it for a while, works nicely and looks better Thanks, Kai Uwe Broulik ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124310: Improve quick chat widget
On July 9, 2015, 8:14 p.m., Aleix Pol Gonzalez wrote: chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml, line 153 https://git.reviewboard.kde.org/r/124310/diff/1/?file=383909#file383909line153 don't remove followConversation, we need it to go false only when the user moves the flickable. Sending a message or receiving an image can do that as well. Kai Uwe Broulik wrote: Hm, indeed. However, neither with, nor without it works properly. For me, as is at the moment, without a scrollbar, it works fine. Problem is that on resize (e.g. message received), the scrollbar moves the handle, then the handle moves the flickable. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/#review82287 --- On July 9, 2015, 8:03 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/ --- (Updated July 9, 2015, 8:03 p.m.) Review request for Plasma and Telepathy. Repository: ktp-desktop-applets Description --- This improves the chat widget by: - Not hardcoding the hader size - Using a heading for the name so it stands out a bit more (since it's often not obvious at a glance who you're writing to) - Giving it a proper scroll bar - Making the auto scrolling a bit more robust - Cleaing up the code a bit Diffs - chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 4e802c7 Diff: https://git.reviewboard.kde.org/r/124310/diff/ Testing --- Been using it for a while, works nicely and looks better Thanks, Kai Uwe Broulik ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124310: Improve quick chat widget
On July 9, 2015, 8:37 p.m., Martin Klapetek wrote: Can you also have a look at https://bugs.kde.org/show_bug.cgi?id=298306 and perhaps include the fix for that as well? Basically the chat applet should collapse when it has nothing to show, just like eg. desktop pager or system tray. The fix for this bug should end up somewhere else in the code, so it should be another review. Also I looked into the code and I'm not sure that the bug is entirely in the plasmoid or in plasma-framework. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/#review82292 --- On July 9, 2015, 8:03 p.m., Kai Uwe Broulik wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124310/ --- (Updated July 9, 2015, 8:03 p.m.) Review request for Plasma and Telepathy. Repository: ktp-desktop-applets Description --- This improves the chat widget by: - Not hardcoding the hader size - Using a heading for the name so it stands out a bit more (since it's often not obvious at a glance who you're writing to) - Giving it a proper scroll bar - Making the auto scrolling a bit more robust - Cleaing up the code a bit Diffs - chat/org.kde.ktp-chat/contents/ui/ChatWidget.qml 4e802c7 Diff: https://git.reviewboard.kde.org/r/124310/diff/ Testing --- Been using it for a while, works nicely and looks better Thanks, Kai Uwe Broulik ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124176: Introduce KPeople contact querying service
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124176/#review81784 --- src/service/service.h (line 33) https://git.reviewboard.kde.org/r/124176/#comment56100 Maybe org.kde.KPeople.LookupService? src/service/service.h (line 41) https://git.reviewboard.kde.org/r/124176/#comment56094 I wouldn't call it hint, I'd call it property or key. It's not a hint in fact, we're not looking for all matches, but specifically a field. As it's phrased, it looks like it's an optimization. src/service/service.cpp (line 46) https://git.reviewboard.kde.org/r/124176/#comment56099 It's more easily readable as: d-model-index(i).data(PersonsModel::PersonVCardRole) src/service/service.cpp (line 48) https://git.reviewboard.kde.org/r/124176/#comment56095 Yes, make it mandatory. src/service/service.cpp (line 49) https://git.reviewboard.kde.org/r/124176/#comment56096 I would just do: QVariantList dataList = person-customProperty(all-+hint).toList(); src/service/service.cpp (line 57) https://git.reviewboard.kde.org/r/124176/#comment56097 contact.simplified() can be done only once outside of all loops. src/service/service.cpp (line 58) https://git.reviewboard.kde.org/r/124176/#comment56098 It's more easily readable as: d-model-index(i).data(PersonsModel::PersonUriRole) - Aleix Pol Gonzalez On June 25, 2015, 3:33 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124176/ --- (Updated June 25, 2015, 3:33 p.m.) Review request for KDEPIM and Telepathy. Repository: kpeople Description --- This is a dbus service that runs in the background and returns contact ids for queried contact details, for example asking for a phone number would return a contact id from which a PersonData class can be constructed, thus making all contact data available for display. This is very useful when eg. receiving a phone call, a tp approver starts up showing the incoming call (this needs to be instant) and once it's shown up, it should show a contact name for the number as soon as possible. However loading all the datasources and populating the model can be slow and speed is critical in this case. Therefore it's a dbus service which responds very fast and there's no need to initialize anything as it can be loaded in the background after system starts and it keeps itself always up to date. Diffs - autotests/CMakeLists.txt 233e7d9 autotests/servicetest.h PRE-CREATION autotests/servicetest.cpp PRE-CREATION src/CMakeLists.txt 59bc915 src/backends/abstractcontact.h ce22cbc src/backends/abstractcontact.cpp f01236b src/global.h e1d07ce src/global.cpp b3595ca src/service/CMakeLists.txt PRE-CREATION src/service/main.cpp PRE-CREATION src/service/org.kde.KPeople.service.in PRE-CREATION src/service/service.h PRE-CREATION src/service/service.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/124176/diff/ Testing --- Unit test included; also tested with qdbus. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
On June 22, 2015, 11:19 a.m., Martin Klapetek wrote:
What may be a, possibly minor, issue is that this advertises qml support,
however all the UI plugins are currently qwidgets-based only and there's no
detection done to prevent loading non-qml plugins. At some point there
should be if (qml) { load_plugins_for_qml } else {
load_plugins_for_widgets } plus it should probably also filter providers
which it can create from a qml only environment (providers have plugin
entry).
Yes, that would be ideal.
More than if (qml) it should be if (desktop), I'd say...
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124137/#review81650
---
On June 22, 2015, 8:01 a.m., Aleix Pol Gonzalez wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124137/
---
(Updated June 22, 2015, 8:01 a.m.)
Review request for KDEPIM and Telepathy.
Repository: kaccounts-integration
Description
---
Adds a plugin and an example that uses it. Seems to work.
Diffs
-
CMakeLists.txt 7431dba
example/accounts.qml PRE-CREATION
src/CMakeLists.txt f28ebb6
src/declarative/CMakeLists.txt PRE-CREATION
src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION
src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION
src/declarative/qmldir PRE-CREATION
src/jobs/createaccount.h c9d23b6
src/jobs/createaccount.cpp 883283f
Diff: https://git.reviewboard.kde.org/r/124137/diff/
Testing
---
Played around with the example.
Thanks,
Aleix Pol Gonzalez
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/ --- (Updated June 22, 2015, 5:28 p.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Changes --- Submitted with commit aed652ee0a37dcd5797ecef0a8ceabe559481c06 by Aleix Pol to branch master. Repository: kaccounts-integration Description --- Adds a plugin and an example that uses it. Seems to work. Diffs - CMakeLists.txt 7431dba example/accounts.qml PRE-CREATION src/CMakeLists.txt f28ebb6 src/declarative/CMakeLists.txt PRE-CREATION src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION src/declarative/qmldir PRE-CREATION src/jobs/createaccount.h c9d23b6 src/jobs/createaccount.cpp 883283f Diff: https://git.reviewboard.kde.org/r/124137/diff/ Testing --- Played around with the example. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/ --- (Updated June 22, 2015, 8:01 a.m.) Review request for KDEPIM and Telepathy. Changes --- Address issues pointed out by David. Repository: kaccounts-integration Description --- Adds a plugin and an example that uses it. Seems to work. Diffs (updated) - CMakeLists.txt 7431dba example/accounts.qml PRE-CREATION src/CMakeLists.txt f28ebb6 src/declarative/CMakeLists.txt PRE-CREATION src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION src/declarative/qmldir PRE-CREATION src/jobs/createaccount.h c9d23b6 src/jobs/createaccount.cpp 883283f Diff: https://git.reviewboard.kde.org/r/124137/diff/ Testing --- Played around with the example. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
On June 22, 2015, 7:14 a.m., David Edmundson wrote: Seems odd to use a mix of Ubuntu's imports and our stuff. Does their import not cover creating accounts? Probably worth exporting our model too, so apps don't have to import two things. It's only weird because they branded the QML module (and so we do). OTOH, they developped the rest of the stuff and it's part of the stack. https://gitlab.com/accounts-sso/accounts-qml-module - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/#review81643 --- On June 22, 2015, 8:01 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/ --- (Updated June 22, 2015, 8:01 a.m.) Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- Adds a plugin and an example that uses it. Seems to work. Diffs - CMakeLists.txt 7431dba example/accounts.qml PRE-CREATION src/CMakeLists.txt f28ebb6 src/declarative/CMakeLists.txt PRE-CREATION src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION src/declarative/qmldir PRE-CREATION src/jobs/createaccount.h c9d23b6 src/jobs/createaccount.cpp 883283f Diff: https://git.reviewboard.kde.org/r/124137/diff/ Testing --- Played around with the example. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
On June 21, 2015, 2:30 p.m., Alexandr Akulich wrote: Does it make sense to make the plugin optional, so Qml would not be a hard dependency? Also, there is several style issue (QObject* parent - QObject *parent), but it seems to fit exists code. ? Qml is a dependency already as it's part of Qt 5. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/#review81615 --- On June 21, 2015, 1:39 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/ --- (Updated June 21, 2015, 1:39 a.m.) Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- Adds a plugin and an example that uses it. Seems to work. Diffs - CMakeLists.txt 7431dba example/accounts.qml PRE-CREATION src/CMakeLists.txt f28ebb6 src/declarative/CMakeLists.txt PRE-CREATION src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION src/declarative/qmldir PRE-CREATION src/jobs/createaccount.h c9d23b6 src/jobs/createaccount.cpp 883283f Diff: https://git.reviewboard.kde.org/r/124137/diff/ Testing --- Played around with the example. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 124137: Make it possible to access OnlineAccounts using QtQuick
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124137/ --- Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- Adds a plugin and an example that uses it. Seems to work. Diffs - CMakeLists.txt 7431dba example/accounts.qml PRE-CREATION src/CMakeLists.txt f28ebb6 src/declarative/CMakeLists.txt PRE-CREATION src/declarative/kaccountsdeclarativeplugin.h PRE-CREATION src/declarative/kaccountsdeclarativeplugin.cpp PRE-CREATION src/declarative/qmldir PRE-CREATION src/jobs/createaccount.h c9d23b6 src/jobs/createaccount.cpp 883283f Diff: https://git.reviewboard.kde.org/r/124137/diff/ Testing --- Played around with the example. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123938: Don't add send delivery messages in the messages vector
On May 30, 2015, 11:02 p.m., Alexandr Akulich wrote: Hello. I know about the issue for a long time, but did not figured out the root of problem. You're right. :) I just checked it, specs says: Incoming messages and delivery reports are signalled by MessageReceived. I'll fix it tomorrow. Next time you'll find a mistake (or some unclear moment) in CM, please let me know. :) Alexandr Akulich wrote: Fixed in commit http://commits.kde.org/telepathy-morse/4161790d13048ed264853197e4a2b92aeb2ccf95 . Thanks. Thank you very much Alexandr! :) Now what do we do with this patch? - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123938/#review80984 --- On May 29, 2015, 7:58 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123938/ --- (Updated May 29, 2015, 7:58 p.m.) Review request for Telepathy. Repository: ktp-common-internals Description --- Currently, when using Telegram, we keep getting empty lines, this skips them. OTOH, it _really_ looks like a bug in the CM, as these messages should go through the received messages rather than the sent, nevertheless I'm unsure if the patch should still go in. Diffs - KTp/Declarative/messages-model.cpp dc1088c Diff: https://git.reviewboard.kde.org/r/123938/diff/ Testing --- The chat plasmoid works fine. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123907: Make log viewer usable in multiple instances + fix the KDEInit couldn't launch log viewer error and Plasma being blocked
On May 26, 2015, 3:40 p.m., Aleix Pol Gonzalez wrote: Wouldn't it help just to remove the: X-DBUS-StartupType=Unique ? PS: +1 if that's what it takes. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/#review80847 --- On May 26, 2015, 3:30 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/ --- (Updated May 26, 2015, 3:30 p.m.) Review request for Release Team and Telepathy. Bugs: 346395 http://bugs.kde.org/show_bug.cgi?id=346395 Repository: ktp-text-ui Description --- This fixes bug 346395 - when it is launched from Plasma, it will block all of Plasma until the app quits, which is announced by an error message box with that KDEInit couldn't launch error. I'd like to commit this to stable too however this adds a new dependency on KDBusAddons. I think the severity of this issue warrants that new dependency, but can someone from the release team please ack this? Additionally it makes it Multiple allowing for multiple running instances. I think it can be useful to eg. compare logs, but if anyone has a good reason for why it shouldn't be Multiple, please speak up. Diffs - CMakeLists.txt 72c2f8f logviewer/CMakeLists.txt ac35ff6 logviewer/ktp-log-viewer.desktop 1674566 logviewer/main.cpp 00dbc03 logviewer/org.kde.ktplogviewer.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123907/diff/ Testing --- Plasma is no longer blocked when the log viewer is launched from Kicker. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123907: Make log viewer usable in multiple instances + fix the KDEInit couldn't launch log viewer error and Plasma being blocked
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/#review80847 --- Wouldn't it help just to remove the: X-DBUS-StartupType=Unique ? - Aleix Pol Gonzalez On May 26, 2015, 3:30 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/ --- (Updated May 26, 2015, 3:30 p.m.) Review request for Release Team and Telepathy. Bugs: 346395 http://bugs.kde.org/show_bug.cgi?id=346395 Repository: ktp-text-ui Description --- This fixes bug 346395 - when it is launched from Plasma, it will block all of Plasma until the app quits, which is announced by an error message box with that KDEInit couldn't launch error. I'd like to commit this to stable too however this adds a new dependency on KDBusAddons. I think the severity of this issue warrants that new dependency, but can someone from the release team please ack this? Additionally it makes it Multiple allowing for multiple running instances. I think it can be useful to eg. compare logs, but if anyone has a good reason for why it shouldn't be Multiple, please speak up. Diffs - CMakeLists.txt 72c2f8f logviewer/CMakeLists.txt ac35ff6 logviewer/ktp-log-viewer.desktop 1674566 logviewer/main.cpp 00dbc03 logviewer/org.kde.ktplogviewer.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123907/diff/ Testing --- Plasma is no longer blocked when the log viewer is launched from Kicker. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123907: Make log viewer usable in multiple instances + fix the KDEInit couldn't launch log viewer error and Plasma being blocked
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/#review80851 --- Ship it! Ship It! - Aleix Pol Gonzalez On May 26, 2015, 3:30 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123907/ --- (Updated May 26, 2015, 3:30 p.m.) Review request for Release Team and Telepathy. Bugs: 346395 http://bugs.kde.org/show_bug.cgi?id=346395 Repository: ktp-text-ui Description --- This fixes bug 346395 - when it is launched from Plasma, it will block all of Plasma until the app quits, which is announced by an error message box with that KDEInit couldn't launch error. I'd like to commit this to stable too however this adds a new dependency on KDBusAddons. I think the severity of this issue warrants that new dependency, but can someone from the release team please ack this? Additionally it makes it Multiple allowing for multiple running instances. I think it can be useful to eg. compare logs, but if anyone has a good reason for why it shouldn't be Multiple, please speak up. Diffs - CMakeLists.txt 72c2f8f logviewer/CMakeLists.txt ac35ff6 logviewer/ktp-log-viewer.desktop 1674566 logviewer/main.cpp 00dbc03 logviewer/org.kde.ktplogviewer.desktop PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123907/diff/ Testing --- Plasma is no longer blocked when the log viewer is launched from Kicker. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123893: Add option to use different emoticons for each account
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123893/#review80772 --- I think I understand where you're coming from, but I'm unsure it makes sense. Won't it be weird if you change your emoticons theme and then it only gets applied for some accounts? - Aleix Pol Gonzalez On May 24, 2015, 1:50 p.m., David Rosca wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123893/ --- (Updated May 24, 2015, 1:50 p.m.) Review request for Telepathy. Repository: ktp-text-ui Description --- The name of emoticon theme for account is stored in ktelepathyrc and then used by ktp-text-ui. If there is no specified emoticon theme, the default emoticon theme is used. This needs UI somewhere in settings to configure the emoticons for each account. I'm not sure where to put it (in kaccounts-kcm?), so suggestions welcome. Diffs - app/emoticon-text-edit-action.h a5ae531 app/emoticon-text-edit-action.cpp 2ee628e app/emoticon-text-edit-selector.h fb1584f app/emoticon-text-edit-selector.cpp 22742fb filters/emoticons/CMakeLists.txt 447b223 filters/emoticons/emoticon-filter.h 0f5a1ec filters/emoticons/emoticon-filter.cpp 565ae84 lib/CMakeLists.txt d6af848 lib/emoticons-manager.h PRE-CREATION lib/emoticons-manager.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123893/diff/ Testing --- Works fine Thanks, David Rosca ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123545: Fix crash when closing new account dialog with close button
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123545/#review79648 --- Ship it! This should go in Plasma/5.3 as well. - Aleix Pol Gonzalez On April 28, 2015, 9:36 p.m., David Rosca wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123545/ --- (Updated April 28, 2015, 9:36 p.m.) Review request for Telepathy and Martin Klapetek. Repository: ktp-accounts-kcm Description --- Connect QDialogButtonBox rejected signal to QDialog reject slot. This fixes crash when closing add new account dialog with close button (or Alt+F4). Diffs - plugins/kaccounts/kaccounts-ui-provider.cpp a7569cf Diff: https://git.reviewboard.kde.org/r/123545/diff/ Testing --- No longer crashes when closing the dialog with close button and then trying to open new dialog - this can be triggered just by opening Gadu-Gadu new acc dialog - closing it with Alt+F4 and then opening it again. However, it does not fix a crash when clicking on Create entry in accounts list view - then switching to some configured account - switch back to Create and try to open few new account dialogs. The crash here seems to be related to KWidgetItemDelegate (particularly to createItemWidgets), but I'm not really sure what might be wrong as widgets in createItemWidgets are created correctly without parent. This is a warning message when closing the dialog: KWidgetItemDelegateEventListener::eventFilter-e : User of KWidgetItemDelegate should not delete widgets created by createItemWidgets! Thanks, David Rosca ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123447: Populate AccountsQt paths when finding KAccounts on cmake
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123447/ --- (Updated April 21, 2015, 12:04 p.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Changes --- Submitted with commit f522e556bbea37d0a48d6c4141f425b278e2d41b by Aleix Pol to branch master. Repository: kaccounts-integration Description --- Instead of copying the FindAccountsFileDir.cmake file over any project using it, do it from KAccounts. Diffs - src/lib/CMakeLists.txt 6478cee src/lib/KAccountsConfig.cmake.in f083bd7 src/lib/cmake/CMakeLists.txt PRE-CREATION src/lib/cmake/FindAccountsFileDir.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123447/diff/ Testing --- Ported kaccounts-providers, still works. http://pastebin.com/TR2XBL3W Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123445: Collapse the chat applet when there are no chats open
On April 21, 2015, 12:33 p.m., Aleix Pol Gonzalez wrote: chat/org.kde.ktp-chat/contents/ui/FullChatList.qml, line 31 https://git.reviewboard.kde.org/r/123445/diff/1/?file=362235#file362235line31 I don't really understand why you changed all the types either. QtQuick.Layouts operates on qreal. Martin Klapetek wrote: We use ints everywhere in plasma as reals cause sub-pixel alignment, which is unwanted. I see no reason to have that? Mh, whatever. Do it in a separate patch. On April 21, 2015, 12:33 p.m., Aleix Pol Gonzalez wrote: chat/org.kde.ktp-chat/contents/ui/FullChatList.qml, line 33 https://git.reviewboard.kde.org/r/123445/diff/1/?file=362235#file362235line33 Please don't change that. It's not correct. The plasmoid supports being on a vertical and horizontal layout, therefore thickness and length nomenclature. When the width exceedes thickness, the dimensions are swapped. Martin Klapetek wrote: This nomenclature however is not used anywhere else and it's confusing when working with it; it still is width/height even when rotated, isn't it? No. On a vertical set up, length is the height and thickness is the width. On a horizontal set up, height is the thickness and vice-versa. In fact, it's the same nomenclature as in the PanelView: plasma-workspace/shell/panelview.h - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123445/#review79277 --- On April 21, 2015, 12:10 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123445/ --- (Updated April 21, 2015, 12:10 p.m.) Review request for Telepathy. Bugs: 298306 http://bugs.kde.org/show_bug.cgi?id=298306 Repository: ktp-desktop-applets Description --- Just like the Pager applet, collapse when there's nothing to show. Consistency++ Also fix the property type name - Length - Width, Thickness - Height, real - int Diffs - chat/org.kde.ktp-chat/contents/ui/FullChatList.qml db6d420 Diff: https://git.reviewboard.kde.org/r/123445/diff/ Testing --- Added to panel, takes no space, opened a chat, opened another chat, closed chat, opened another chat, closed chat, closed chat, takes no space in panel. And it resized always correctly when opening/closing. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123445: Collapse the chat applet when there are no chats open
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123445/#review79277 --- chat/org.kde.ktp-chat/contents/ui/FullChatList.qml (line 31) https://git.reviewboard.kde.org/r/123445/#comment54146 I don't really understand why you changed all the types either. QtQuick.Layouts operates on qreal. chat/org.kde.ktp-chat/contents/ui/FullChatList.qml (line 33) https://git.reviewboard.kde.org/r/123445/#comment54145 Please don't change that. It's not correct. The plasmoid supports being on a vertical and horizontal layout, therefore thickness and length nomenclature. When the width exceedes thickness, the dimensions are swapped. - Aleix Pol Gonzalez On April 21, 2015, 12:10 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123445/ --- (Updated April 21, 2015, 12:10 p.m.) Review request for Telepathy. Bugs: 298306 http://bugs.kde.org/show_bug.cgi?id=298306 Repository: ktp-desktop-applets Description --- Just like the Pager applet, collapse when there's nothing to show. Consistency++ Also fix the property type name - Length - Width, Thickness - Height, real - int Diffs - chat/org.kde.ktp-chat/contents/ui/FullChatList.qml db6d420 Diff: https://git.reviewboard.kde.org/r/123445/diff/ Testing --- Added to panel, takes no space, opened a chat, opened another chat, closed chat, opened another chat, closed chat, closed chat, takes no space in panel. And it resized always correctly when opening/closing. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123285: Export KAccounts dependencies automatically
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123285/ --- (Updated April 8, 2015, 10:20 a.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Changes --- Submitted with commit 0c300584fe8ca5747c324202e0f6a8cf8c8c5f8e by Aleix Pol to branch master. Repository: kaccounts-integration Description --- Exposes what libraries and include directories are exposed by KAccounts. This way it's easier to link against it since we just need to specify the dependency. Diffs - CMakeLists.txt 35aac8d src/CMakeLists.txt f6aeb41 src/daemon/CMakeLists.txt 59b14ad src/lib/CMakeLists.txt 704f10a tests/CMakeLists.txt cc0ef5a Diff: https://git.reviewboard.kde.org/r/123285/diff/ Testing --- I rebuilt all the modules I could find using KAccounts. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123228: Extend the unit tests to also cover merging and unmerging
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123228/#review78414 --- Ship it! Good stuff! - Aleix Pol Gonzalez On April 2, 2015, 5:16 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123228/ --- (Updated April 2, 2015, 5:16 p.m.) Review request for Telepathy. Repository: kpeople Description --- Adds more contacts and actually tests the merging and unmerging. Diffs - autotests/fakecontactsource.cpp aab6ba7 autotests/persondatatests.cpp 9b39654 autotests/personsmodeltest.h 3428db7 autotests/personsmodeltest.cpp 4b39f65 Diff: https://git.reviewboard.kde.org/r/123228/diff/ Testing --- All tests pass. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123129: Centralize the cmake code for generating services and providers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123129/ --- (Updated April 1, 2015, 4:44 p.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Changes --- Submitted with commit 44bb1a9e46432099c4fc1d184a1ce350279b0e27 by Aleix Pol to branch master. Repository: kaccounts-integration Description --- I wanted to fix the code to generate it at build time rather than configure time, I saw the code was copypasted both in kaccounts-providers and ktp-accounts-kcm. If we get this in, I can look into doing the generation properly. Diffs - src/lib/CMakeLists.txt ddd79b7 src/lib/KAccountsConfig.cmake.in ffc6cb5 src/lib/KAccountsMacros.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123129/diff/ Testing --- Changed it in both places, still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123200: Replace toAscii with toLatin1
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123200/#review78309 --- Ship it! Ship It! - Aleix Pol Gonzalez On April 1, 2015, 1:25 a.m., David Narváez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123200/ --- (Updated April 1, 2015, 1:25 a.m.) Review request for Telepathy. Repository: ktp-call-ui Description --- toAscii is obsolete in Qt5 so these should be replaced in the frameworks branch. Diffs - libktpcall/private/tf-video-content-handler.cpp 301077c Diff: https://git.reviewboard.kde.org/r/123200/diff/ Testing --- Fixed compilation in my Qt5 setup. Thanks, David Narváez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123184: Add enum for ActionsType (+ fix documentation)
On March 30, 2015, 5:48 p.m., Martin Klapetek wrote: If the host application is required to check for a property, then this should be documented as well. I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp? Eike Hein wrote: I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp? We don't want frontends to be tied to KTp in the long run - we want other messengers (e.g. Konversation) to integrate with KPeople, and a programatic way to start a conversation using the user's preferred messenger (we already have a Default Apps setting for one), depending on presence. Something like startTextChat(PersonData *data) would be much better to hide the implementation instead of relying on action list sorting to return the preferred TextChatAction first, but as KTp is the only available backend right now this is fine for use in Plasma 5.3, there's no need to rush the API design. There was some email forth and back on this you were CC'd on btw. Martin Klapetek wrote: Clients are not required for checking the property, they can if they want/need to, as Eike explains. Adding more metadata to the actions only allows for better flexibility, this would also allow for example to group related actions together in say address book. Aleix Pol Gonzalez wrote: Still, the property is not documented. We don't need to rush an API design, but we definitely need to find ways to move away from the current QWidgets-based approach. Anyway, I guess this works, I'm happy to have it in and see how it develops. Martin Klapetek wrote: Also I've put it with PersonData because the plugins implementing the actions do not include actions.h but they always include persondata.h because it's being passed to it. I don't think that putting it into actions.h makes too much sense because that's client only, not for plugins which also need that enum (and the plugins don't link to KPeopleWidgets either). But if you think it should go there, I'll move it there. I prefer orienting these things for clients than plugins, as it explains better what it's about. Also, if we ever decide to come up with a new API, we'll still have an enum only targetting Actions. Please move to actions.h and explain how to fetch the flag from the QAction instance. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123184/#review78223 --- On March 30, 2015, 5:03 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123184/ --- (Updated March 30, 2015, 5:03 p.m.) Review request for Telepathy and Eike Hein. Repository: kpeople Description --- Some clients need/want to filter actions based on types, this adds the actions enum with currently known actions. Plugins returning QListQAction* should then use this enum to set the appropriate type on the action (via setProperty). I've put it to PersonData because all code dealing with actions currently need to include PersonData header and I'm not sure if it makes sense anywhere else. Diffs - src/persondata.h c3f99a9 src/widgets/actions.h 2931ef8 Diff: https://git.reviewboard.kde.org/r/123184/diff/ Testing --- Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123184: Add enum for ActionsType (+ fix documentation)
On March 30, 2015, 5:48 p.m., Martin Klapetek wrote: If the host application is required to check for a property, then this should be documented as well. I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp? Eike Hein wrote: I'm unsure about the use-case there though. Why do we need to filter the actions? If we just want chat, wouldn't it be better to directly request it to KTp? We don't want frontends to be tied to KTp in the long run - we want other messengers (e.g. Konversation) to integrate with KPeople, and a programatic way to start a conversation using the user's preferred messenger (we already have a Default Apps setting for one), depending on presence. Something like startTextChat(PersonData *data) would be much better to hide the implementation instead of relying on action list sorting to return the preferred TextChatAction first, but as KTp is the only available backend right now this is fine for use in Plasma 5.3, there's no need to rush the API design. There was some email forth and back on this you were CC'd on btw. Martin Klapetek wrote: Clients are not required for checking the property, they can if they want/need to, as Eike explains. Adding more metadata to the actions only allows for better flexibility, this would also allow for example to group related actions together in say address book. Still, the property is not documented. We don't need to rush an API design, but we definitely need to find ways to move away from the current QWidgets-based approach. Anyway, I guess this works, I'm happy to have it in and see how it develops. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123184/#review78223 --- On March 30, 2015, 5:03 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123184/ --- (Updated March 30, 2015, 5:03 p.m.) Review request for Telepathy and Eike Hein. Repository: kpeople Description --- Some clients need/want to filter actions based on types, this adds the actions enum with currently known actions. Plugins returning QListQAction* should then use this enum to set the appropriate type on the action (via setProperty). I've put it to PersonData because all code dealing with actions currently need to include PersonData header and I'm not sure if it makes sense anywhere else. Diffs - src/persondata.h c3f99a9 src/widgets/actions.h 2931ef8 Diff: https://git.reviewboard.kde.org/r/123184/diff/ Testing --- Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 123129: Centralize the cmake code for generating services and providers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123129/ --- Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- I wanted to fix the code to generate it at build time rather than configure time, I saw the code was copypasted both in kaccounts-providers and ktp-accounts-kcm. If we get this in, I can look into doing the generation properly. Diffs - Diff: https://git.reviewboard.kde.org/r/123129/diff/ Testing --- Changed it in both places, still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123129: Centralize the cmake code for generating services and providers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123129/ --- (Updated March 25, 2015, 10:16 p.m.) Review request for KDEPIM and Telepathy. Repository: kaccounts-integration Description --- I wanted to fix the code to generate it at build time rather than configure time, I saw the code was copypasted both in kaccounts-providers and ktp-accounts-kcm. If we get this in, I can look into doing the generation properly. Diffs (updated) - src/lib/CMakeLists.txt ddd79b7 src/lib/KAccountsConfig.cmake.in ffc6cb5 src/lib/KAccountsMacros.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123129/diff/ Testing --- Changed it in both places, still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123087: Port away from kDebug / kWarning / kError
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123087/#review77859 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 20, 2015, 8:26 p.m., Niels Ole Salscheider wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123087/ --- (Updated March 20, 2015, 8:26 p.m.) Review request for Telepathy. Repository: ktp-call-ui Description --- Port away from kDebug / kWarning / kError Diffs - libktpcall/CMakeLists.txt 512c9f1476636487bd3165bc599d0ca703c62398 libktpcall/call-channel-handler.cpp 8779b602028b6812892b5c1a8fb01300781eee7a libktpcall/call-content-handler.cpp d167835693d6429da11f7f55cd99af92ba7e0d5d libktpcall/ktp_call_ui_debug.h PRE-CREATION libktpcall/ktp_call_ui_debug.cpp PRE-CREATION libktpcall/private/device-element-factory.cpp 2d425003a98f87cf45695220bbddf6eb15ffa4c2 libktpcall/private/phonon-integration.cpp ece6a8b63020ae23cdff202c32942843ed46aa6c libktpcall/private/sink-controllers.cpp 7ed56139feda9cc73a7366a771ebf63f0aa278bf libktpcall/private/sink-manager.cpp 1df1c1734d4616bbb15e0cd90a34dad428ef9bde libktpcall/private/tf-audio-content-handler.cpp 965c19ce2ff057430298d4c4aca5a83d6e7641f3 libktpcall/private/tf-channel-handler.cpp 9d906c571e16323dc8bf8b26b77bad864556ba19 libktpcall/private/tf-content-handler.cpp a8456e2ccce20ea3076cdd97c21ccf2c5e37c898 libktpcall/private/tf-video-content-handler.cpp a8a9eca2a8eb88bb3140643f66529d410bc7087c libktpcall/private/video-sink-bin.cpp 0b95b5c4a4cb30d4aca4ad93cc940f47bb5d9769 libktpcall/tests/CMakeLists.txt f0377cc5f5873624a62714922284ab1c8375876c libktpcall/tests/configuration_test.cpp 6b5da6b0e325cda0509a35b862071424807f8cd9 src/CMakeLists.txt 3cb382648243129a2126cd546059786a4f1fbc77 src/approver.cpp cab426b7f1241190bb3bfa1b0062aea9e06be8b6 src/call-handler.cpp 75c798491058e0bdd29b0a7618baf86b17048f5c src/call-manager.cpp df6e7017f9fb6248c5e5488eb45f7c891d18aa0f src/call-window.cpp b4d7e3dfe0b6ded113c9ff7c226f4a3c2cf2d9f4 src/dialout/CMakeLists.txt 242fc2b9691875bf8b285c61849d7c9c7dd6e396 src/dialout/dialout-widget.cpp 2cf5e7f28519f573a3eac1587bc0034a602c46b4 src/ktp_call_ui_debug.h PRE-CREATION src/ktp_call_ui_debug.cpp PRE-CREATION src/status-area.cpp b8a36d3855ee9ab67a8e3ad6107e9e3615a41148 Diff: https://git.reviewboard.kde.org/r/123087/diff/ Testing --- Builds, starts. Thanks, Niels Ole Salscheider ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123056: Merge remote-tracking branch 'origin/master' into frameworks
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123056/#review2 --- +1 should be fine, provided all the rest works. - Aleix Pol Gonzalez On March 19, 2015, 7:55 p.m., Niels Ole Salscheider wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123056/ --- (Updated March 19, 2015, 7:55 p.m.) Review request for Telepathy. Repository: ktp-call-ui Description --- Merge master (containing the new QML interface) to frameworks Diffs - CMakeLists.txt 873dd93 Messages.sh a9e0e28 src/CMakeLists.txt 3cb3826 src/call-window.h 07fd01e src/call-window.cpp b4d7e3d src/call-window.ui 32c7dad src/callwindowui.rc 6c390b9 src/dtmf-handler.h 91960dc src/dtmf-handler.cpp d8d7970 src/dtmf-qml.h PRE-CREATION src/dtmf-qml.cpp PRE-CREATION src/dtmf-widget.h b936bcf src/dtmf-widget.cpp f3436b2 src/dtmf-widget.ui 67f60b9 src/qml-interface.h PRE-CREATION src/qml-interface.cpp PRE-CREATION src/qml/Main.qml PRE-CREATION src/qml/core/Button.qml PRE-CREATION src/qml/core/Dtmf.qml PRE-CREATION src/qml/core/DtmfButton.qml PRE-CREATION src/qml/core/Label.qml PRE-CREATION src/qml/core/Separator.qml PRE-CREATION src/qml/core/ToggleButton.qml PRE-CREATION src/qml/core/Toolbar.qml PRE-CREATION src/qml/core/Tooltip.qml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123056/diff/ Testing --- This does not build because it imports the old QtDeclarative code. This fill be fixed by the next commit. Thanks, Niels Ole Salscheider ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 123059: Port to QtQuick 2.0
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123059/#review1 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 19, 2015, 8:05 p.m., Niels Ole Salscheider wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123059/ --- (Updated March 19, 2015, 8:05 p.m.) Review request for Telepathy. Repository: ktp-call-ui Description --- Port to QtQuick 2.0 Diffs - CMakeLists.txt 873dd93eba18e046b20d89a16d4c843f31377a22 src/CMakeLists.txt 3cb382648243129a2126cd546059786a4f1fbc77 src/call-window.cpp b4d7e3dfe0b6ded113c9ff7c226f4a3c2cf2d9f4 src/dtmf-qml.cpp PRE-CREATION src/qml-interface.h PRE-CREATION src/qml-interface.cpp PRE-CREATION src/qml/Main.qml PRE-CREATION src/qml/core/Button.qml PRE-CREATION src/qml/core/Dtmf.qml PRE-CREATION src/qml/core/DtmfButton.qml PRE-CREATION src/qml/core/Label.qml PRE-CREATION src/qml/core/Separator.qml PRE-CREATION src/qml/core/ToggleButton.qml PRE-CREATION src/qml/core/Toolbar.qml PRE-CREATION src/qml/core/Tooltip.qml PRE-CREATION Diff: https://git.reviewboard.kde.org/r/123059/diff/ Testing --- Builds, video calls work Thanks, Niels Ole Salscheider ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122872: Fix capitalization
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122872/ --- (Updated March 9, 2015, 4:16 p.m.) Status -- This change has been discarded. Review request for Telepathy. Repository: ktp-common-internals Description --- Don't randomly capitalize words. Diffs - kpeople/actionsplugin/kpeople-actions-plugin.cpp 8c60bc7 Diff: https://git.reviewboard.kde.org/r/122872/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122872: Fix capitalization
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122872/ --- Review request for Telepathy. Repository: ktp-common-internals Description --- Don't randomly capitalize words. Diffs - kpeople/actionsplugin/kpeople-actions-plugin.cpp 8c60bc7 Diff: https://git.reviewboard.kde.org/r/122872/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122778: Show full text
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122778/ --- (Updated March 5, 2015, 5:13 p.m.) Status -- This change has been marked as submitted. Review request for Telepathy. Repository: ktp-contact-list Description --- There's no real gain by shortening Information into Info. Diffs - context-menu.cpp 58e3d97 Diff: https://git.reviewboard.kde.org/r/122778/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122778: Show full text
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122778/ --- (Updated March 5, 2015, 1:34 p.m.) Review request for Telepathy. Changes --- Fix the rest of the entries: - Remove redundant Contact mentions in the contact context menu. - Added ellipsis on entries that will have further interaction. Repository: ktp-contact-list Description --- There's no real gain by shortening Information into Info. Diffs (updated) - context-menu.cpp 58e3d97 Diff: https://git.reviewboard.kde.org/r/122778/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122825: Remove glib includes from TelepathyLoggerQt headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122825/ --- Review request for Telepathy and Daniel Vrátil. Repository: telepathy-logger-qt Description --- For simplicity mainly, everything still just works. Diffs - TelepathyLoggerQt/CMakeLists.txt 1e2fea3 TelepathyLoggerQt/global.h c04d8a5 TelepathyLoggerQt/object.h 4067a90 TelepathyLoggerQt/object.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122825/diff/ Testing --- All TelepathyLoggerQt and KTp built fine. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122825: Remove glib includes from TelepathyLoggerQt headers
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122825/ --- (Updated March 5, 2015, 1:03 p.m.) Status -- This change has been marked as submitted. Review request for Telepathy and Daniel Vrátil. Repository: telepathy-logger-qt Description --- For simplicity mainly, everything still just works. Diffs - TelepathyLoggerQt/CMakeLists.txt 1e2fea3 TelepathyLoggerQt/global.h c04d8a5 TelepathyLoggerQt/object.h 4067a90 TelepathyLoggerQt/object.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122825/diff/ Testing --- All TelepathyLoggerQt and KTp built fine. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122778: Show full text
On March 3, 2015, 4:25 p.m., Martin Klapetek wrote: Show Contact Information... ...and ship to master Why add Contact? It's a context menu, and the context is the contact, no? - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122778/#review76952 --- On March 2, 2015, 5:42 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122778/ --- (Updated March 2, 2015, 5:42 p.m.) Review request for Telepathy. Repository: ktp-contact-list Description --- There's no real gain by shortening Information into Info. Diffs - context-menu.cpp 2870aad Diff: https://git.reviewboard.kde.org/r/122778/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122794: Make actions plugin return actions also for all subcontacts
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122794/#review76959 --- Ship it! Ship It! - Aleix Pol Gonzalez On March 3, 2015, 6:36 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122794/ --- (Updated March 3, 2015, 6:36 p.m.) Review request for Telepathy. Repository: ktp-common-internals Description --- Till now the plugin returns only actions for most online contact. With this it iterates over the subcontacts and creates list of actions for them too, restoring the 0.9 version behavior. Single actions for subcontacts remain broken as the actions plugin now always operates on person. Diffs - kpeople/actionsplugin/kpeople-actions-plugin.cpp 24712a8 Diff: https://git.reviewboard.kde.org/r/122794/diff/ Testing --- See screenshot; one of the actions is there twice because there are two different contacts from the same account merged into one. Making this more clever would require changing strings which is not allowed now. File Attachments context-menu.png https://git.reviewboard.kde.org/media/uploaded/files/2015/03/03/0cbbbedd-a0a1-4c5c-a592-3aba41829112__context-menu.png Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122778: Show full text
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122778/ --- Review request for Telepathy. Repository: ktp-contact-list Description --- There's no real gain by shortening Information into Info. Diffs - context-menu.cpp 2870aad Diff: https://git.reviewboard.kde.org/r/122778/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122729: Drop Baloo things
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122729/#review76674 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 26, 2015, 4:55 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122729/ --- (Updated Feb. 26, 2015, 4:55 p.m.) Review request for Telepathy. Repository: kpeople Description --- Quoting Vishesh it will never work. Baloo KF5, has no knowledge about emails. That code also uses Akonadi APIs. Diffs - src/widgets/personemailview.h 1f9e862 src/widgets/personemailview.cpp eda0a25 src/widgets/plugins/emaillistmodel.h 9430929 src/widgets/plugins/emaillistmodel.cpp a83bc96 src/widgets/plugins/emaillistviewdelegate.h 43c111f src/widgets/plugins/emaillistviewdelegate.cpp ba6d29e src/widgets/plugins/emails.h 45df621 src/widgets/plugins/emails.cpp dede2d1 CMakeLists.txt 0e17cf6 src/widgets/CMakeLists.txt 213c6bf Diff: https://git.reviewboard.kde.org/r/122729/diff/ Testing --- It builds fine Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122692: Move the OTR config module from ktp-text-ui to here
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122692/#review76479 --- +1 LGTM - Aleix Pol Gonzalez On Feb. 23, 2015, 5:08 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122692/ --- (Updated Feb. 23, 2015, 5:08 p.m.) Review request for Telepathy. Repository: ktp-common-internals Description --- It should be built and installed only if otr itself is built and installed Diffs - otr-proxy/config/otr-config.ui PRE-CREATION otr-proxy/CMakeLists.txt 6cfe999 otr-proxy/config/CMakeLists.txt PRE-CREATION otr-proxy/config/kcm_ktp_chat_otr.desktop PRE-CREATION otr-proxy/config/otr-config.h PRE-CREATION otr-proxy/config/otr-config.cpp PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122692/diff/ Testing --- Builds fine Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122639: Improve the Person applet config page
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122639/#review76299 --- Ship it! Looks much better, thanks! Regarding the index, you probably will need a method that returns the row for a given Uri. - Aleix Pol Gonzalez On Feb. 19, 2015, 3:48 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122639/ --- (Updated Feb. 19, 2015, 3:48 p.m.) Review request for Telepathy. Repository: ktp-desktop-applets Description --- Turns the combobox into a grid with filter (like it used to be sometime ago). See screenshots. Diffs - person/contents/ui/settingsGeneral.qml 25f6df0 Diff: https://git.reviewboard.kde.org/r/122639/diff/ Testing --- Selecting a contact works, everything gets saved properly. Problem comes when reopening the config - I'm not sure how to properly find the index of the saved person. I've tried to add another SortFilterModel and filter by the personUri role, however that takes only regexp and passing the uri seems to make it parse like regexp and so returns nothing. Any ideas on that? File Attachments Before https://git.reviewboard.kde.org/media/uploaded/files/2015/02/19/4caf0d02-fabc-4f49-8af1-ced285fa4464__person1.png After https://git.reviewboard.kde.org/media/uploaded/files/2015/02/19/395f3a27-a6d3-469c-81d3-5950d3d59c61__person2.png Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122641: Improve the Person applet itself
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122641/#review76309 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 19, 2015, 5:39 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122641/ --- (Updated Feb. 19, 2015, 5:39 p.m.) Review request for Telepathy. Repository: ktp-desktop-applets Description --- Changes briefly: * now it displays the avatar when in panel * does not display the actions when in panel (or not big enough) * does display only button without text if there's not enough room to fit the whole text * does switch the Flow from Left-to-Right for icons-only buttons to Top-to-Bottom for buttons with text (the text is so long that it cannot fit two buttons next to each other) * starts 1st action on mouse click * uses now expanded- and compactRepresentation thing --I'd like to further add some presence indication that would be different in both cases Diffs - person/contents/ui/Person.qml PRE-CREATION person/contents/ui/main.qml 871c1c0 Diff: https://git.reviewboard.kde.org/r/122641/diff/ Testing --- Added applet to panel, I now see an avatar, clicking it starts a chat. Trying it in plasmawindowed with various sizes, actions show/hide/expand/move as supposed to. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122641: Improve the Person applet itself
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122641/#review76308 --- Ship it! Ship It! - Aleix Pol Gonzalez On Feb. 19, 2015, 5:39 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122641/ --- (Updated Feb. 19, 2015, 5:39 p.m.) Review request for Telepathy. Repository: ktp-desktop-applets Description --- Changes briefly: * now it displays the avatar when in panel * does not display the actions when in panel (or not big enough) * does display only button without text if there's not enough room to fit the whole text * does switch the Flow from Left-to-Right for icons-only buttons to Top-to-Bottom for buttons with text (the text is so long that it cannot fit two buttons next to each other) * starts 1st action on mouse click * uses now expanded- and compactRepresentation thing --I'd like to further add some presence indication that would be different in both cases Diffs - person/contents/ui/Person.qml PRE-CREATION person/contents/ui/main.qml 871c1c0 Diff: https://git.reviewboard.kde.org/r/122641/diff/ Testing --- Added applet to panel, I now see an avatar, clicking it starts a chat. Trying it in plasmawindowed with various sizes, actions show/hide/expand/move as supposed to. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122543: Re-use AbstractContacts generated in the KTp KPeople Data Source
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122543/ --- (Updated Feb. 12, 2015, 11:23 p.m.) Status -- This change has been marked as submitted. Review request for Telepathy. Repository: ktp-common-internals Description --- Re-use the instances instead of creating them every time the cache is loaded. Diffs - kpeople/datasourceplugin/im-persons-data-source.cpp 77cb67f Diff: https://git.reviewboard.kde.org/r/122543/diff/ Testing --- KTp Contact List plasmoid: 1. Open the Contact List 2. Go online 3. Go offline 4. Go online It doesn't assert anymore. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122543: Re-use AbstractContacts generated in the KTp KPeople Data Source
On Feb. 12, 2015, 6:46 p.m., Martin Klapetek wrote:
kpeople/datasourceplugin/im-persons-data-source.cpp, lines 182-183
https://git.reviewboard.kde.org/r/122543/diff/1/?file=348508#file348508line182
Given that found is nowhere else used, wouldn't it be sufficient to
just do
```if (it != m_contactVCards.constEnd()) {``` ?
Oh yes, I wanted to re-use it instead of the constFind below.
I'll do the change directly when shipping it.
- Aleix
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122543/#review75933
---
On Feb. 12, 2015, 6:20 p.m., Aleix Pol Gonzalez wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122543/
---
(Updated Feb. 12, 2015, 6:20 p.m.)
Review request for Telepathy.
Repository: ktp-common-internals
Description
---
Re-use the instances instead of creating them every time the cache is loaded.
Diffs
-
kpeople/datasourceplugin/im-persons-data-source.cpp 77cb67f
Diff: https://git.reviewboard.kde.org/r/122543/diff/
Testing
---
KTp Contact List plasmoid:
1. Open the Contact List
2. Go online
3. Go offline
4. Go online
It doesn't assert anymore.
Thanks,
Aleix Pol Gonzalez
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122543: Re-use AbstractContacts generated in the KTp KPeople Data Source
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122543/ --- Review request for Telepathy. Repository: ktp-common-internals Description --- Re-use the instances instead of creating them every time the cache is loaded. Diffs - kpeople/datasourceplugin/im-persons-data-source.cpp 77cb67f Diff: https://git.reviewboard.kde.org/r/122543/diff/ Testing --- KTp Contact List plasmoid: 1. Open the Contact List 2. Go online 3. Go offline 4. Go online It doesn't assert anymore. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122453: Improve KPeople documentation, in order to become a Framework
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122453/ --- (Updated Feb. 11, 2015, 2:04 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, KDEPIM and Telepathy. Repository: libkpeople Description --- Adds a README.md and a metainfo.yaml. It also documents some classes and methods that didn't have any documentation. Diffs - src/widgets/CMakeLists.txt bd2cd7e src/widgets/mergedialog.h 305d07b src/widgets/persondetailsview.h 0769d51 src/persondata.h 0fab902 src/personsmodel.h 32dc1f5 metainfo.yaml PRE-CREATION src/CMakeLists.txt 7e01976 src/duplicatesfinder_p.h 05f0063 CMakeLists.txt 00177f3 README.md PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122453/diff/ Testing --- Unit tests still pass ;). Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122453: Improve KPeople documentation, in order to become a Framework
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122453/ --- (Updated Feb. 11, 2015, 1:11 p.m.) Review request for KDE Frameworks, KDEPIM and Telepathy. Changes --- Addressed mck comments. Thanks! Repository: libkpeople Description --- Adds a README.md and a metainfo.yaml. It also documents some classes and methods that didn't have any documentation. Diffs (updated) - src/widgets/CMakeLists.txt bd2cd7e src/widgets/mergedialog.h 305d07b src/widgets/persondetailsview.h 0769d51 src/persondata.h 0fab902 src/personsmodel.h 32dc1f5 metainfo.yaml PRE-CREATION src/CMakeLists.txt 7e01976 src/duplicatesfinder_p.h 05f0063 CMakeLists.txt 00177f3 README.md PRE-CREATION Diff: https://git.reviewboard.kde.org/r/122453/diff/ Testing --- Unit tests still pass ;). Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122444: Improve the contacts cache for KPeople
On Feb. 8, 2015, 11:47 a.m., Martin Klapetek wrote: ship this if you want for now, but even medium term we need to fix the above comment. +1 - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122444/#review75589 --- On Feb. 5, 2015, 7:11 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122444/ --- (Updated Feb. 5, 2015, 7:11 p.m.) Review request for Telepathy. Repository: ktp-common-internals Description --- Currently when account goes online from offline, the contact cache is completely nuked and recreated. This is fast but has one drawback - it will store avatars only of those contacts that are online in the moment of connection, so basically with each connection the available avatars change a bit. To compensate, we've been storing the avatar token in a config file, but that's used only for KTp::Contact. Now with KPeople, if the account is offline, we don't have access to KTp::Contact and have avatars only from the last cache recreation (that means that when you go offline, half of your contacts suddenly loose avatars). This patch now reloads the cache for the account that went offline (second part of this patch is that I've modified the cache to update the database on avatar data change, so for each contact coming online the avatar gets stored in the cache). But as the contact may have not came online during the last connection, it now also loads the avatars from the config file if the database has nothing (this matches the KTp::Contact behavior). Diffs - kpeople/datasourceplugin/CMakeLists.txt 8068977 kpeople/datasourceplugin/im-persons-data-source.cpp f0868b8 Diff: https://git.reviewboard.kde.org/r/122444/diff/ Testing --- I go offline and all my contacts keep their avatars. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122444: Improve the contacts cache for KPeople
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122444/#review75614 --- kpeople/datasourceplugin/im-persons-data-source.cpp https://git.reviewboard.kde.org/r/122444/#comment52291 ';' kpeople/datasourceplugin/im-persons-data-source.cpp https://git.reviewboard.kde.org/r/122444/#comment52290 If you want to construct a QString from a literal, use QStringLiteral. kpeople/datasourceplugin/im-persons-data-source.cpp https://git.reviewboard.kde.org/r/122444/#comment52292 You probably want this as an actual slot. Otherwise there will be different versions of this method in memory for no reason. - Aleix Pol Gonzalez On Feb. 5, 2015, 7:11 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122444/ --- (Updated Feb. 5, 2015, 7:11 p.m.) Review request for Telepathy. Repository: ktp-common-internals Description --- Currently when account goes online from offline, the contact cache is completely nuked and recreated. This is fast but has one drawback - it will store avatars only of those contacts that are online in the moment of connection, so basically with each connection the available avatars change a bit. To compensate, we've been storing the avatar token in a config file, but that's used only for KTp::Contact. Now with KPeople, if the account is offline, we don't have access to KTp::Contact and have avatars only from the last cache recreation (that means that when you go offline, half of your contacts suddenly loose avatars). This patch now reloads the cache for the account that went offline (second part of this patch is that I've modified the cache to update the database on avatar data change, so for each contact coming online the avatar gets stored in the cache). But as the contact may have not came online during the last connection, it now also loads the avatars from the config file if the database has nothing (this matches the KTp::Contact behavior). Diffs - kpeople/datasourceplugin/CMakeLists.txt 8068977 kpeople/datasourceplugin/im-persons-data-source.cpp f0868b8 Diff: https://git.reviewboard.kde.org/r/122444/diff/ Testing --- I go offline and all my contacts keep their avatars. Thanks, Martin Klapetek ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122453: Improve KPeople documentation, in order to become a Framework
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122453/ --- Review request for KDE Frameworks, KDEPIM and Telepathy. Repository: libkpeople Description --- Adds a README.md and a metainfo.yaml. It also documents some classes and methods that didn't have any documentation. Diffs - src/widgets/CMakeLists.txt bd2cd7e src/widgets/mergedialog.h 305d07b src/widgets/persondetailsview.h 0769d51 CMakeLists.txt 00177f3 README.md PRE-CREATION metainfo.yaml PRE-CREATION src/CMakeLists.txt 7e01976 src/duplicatesfinder_p.h 05f0063 src/persondata.h 0fab902 src/personsmodel.h 32dc1f5 Diff: https://git.reviewboard.kde.org/r/122453/diff/ Testing --- Unit tests still pass ;). Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122428: Move private methods/slots and slots from PersonsModel to PersonsModelPrivate
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/ --- (Updated Feb. 6, 2015, 12:32 p.m.) Status -- This change has been marked as submitted. Review request for KDEPIM and Telepathy. Repository: libkpeople Description --- Move the private methods into the p-impl. First, it's the correct thing to do. Secondly, it was pulling an awkward dependency on abstractcontact.h from the backends into a public header. Diffs - src/CMakeLists.txt d8b4875 src/backends/abstractcontact.h 4d1edcd src/personsmodel.h 239f4ed src/personsmodel.cpp bc2bee6 Diff: https://git.reviewboard.kde.org/r/122428/diff/ Testing --- Builds, tests pass, ktp-contactlist still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122428: Move private methods/slots and slots from PersonsModel to PersonsModelPrivate
On Feb. 5, 2015, 3:32 p.m., Martin Klapetek wrote: src/personsmodel.cpp, line 37 https://git.reviewboard.kde.org/r/122428/diff/1/?file=347013#file347013line37 I was always told that making private d-pointer a QObject is a very bad idea as it makes things unnecessarily heavy. Do we really need this as a QObject? If so, then it misses the Q_OBJECT macro too Yes, well, I think it's better like this than with all the Q_PRIVATE_SLOT, because these too will exposte the forward declaration of AbstractContact.. That's why I did it like this. - Aleix --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/#review75474 --- On Feb. 4, 2015, 5:55 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/ --- (Updated Feb. 4, 2015, 5:55 p.m.) Review request for KDEPIM and Telepathy. Repository: libkpeople Description --- Move the private methods into the p-impl. First, it's the correct thing to do. Secondly, it was pulling an awkward dependency on abstractcontact.h from the backends into a public header. Diffs - src/CMakeLists.txt d8b4875 src/backends/abstractcontact.h 4d1edcd src/personsmodel.h 239f4ed src/personsmodel.cpp bc2bee6 Diff: https://git.reviewboard.kde.org/r/122428/diff/ Testing --- Builds, tests pass, ktp-contactlist still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Re: Review Request 122428: Move private methods/slots and slots from PersonsModel to PersonsModelPrivate
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/ --- (Updated Feb. 4, 2015, 5:55 p.m.) Review request for KDEPIM and Telepathy. Changes --- Forgot to upload the patch... Repository: libkpeople Description --- Move the private methods into the p-impl. First, it's the correct thing to do. Secondly, it was pulling an awkward dependency on abstractcontact.h from the backends into a public header. Diffs (updated) - src/CMakeLists.txt d8b4875 src/backends/abstractcontact.h 4d1edcd src/personsmodel.h 239f4ed src/personsmodel.cpp bc2bee6 Diff: https://git.reviewboard.kde.org/r/122428/diff/ Testing --- Builds, tests pass, ktp-contactlist still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
Review Request 122428: Move private methods/slots and slots from PersonsModel to PersonsModelPrivate
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122428/ --- Review request for KDEPIM and Telepathy. Repository: libkpeople Description --- Move the private methods into the p-impl. First, it's the correct thing to do. Secondly, it was pulling an awkward dependency on abstractcontact.h from the backends into a public header. Diffs - Diff: https://git.reviewboard.kde.org/r/122428/diff/ Testing --- Builds, tests pass, ktp-contactlist still works. Thanks, Aleix Pol Gonzalez ___ KDE-Telepathy mailing list KDE-Telepathy@kde.org https://mail.kde.org/mailman/listinfo/kde-telepathy
