Re: Review Request 128960: [KTp/Declarative/MessagesModel] Use message token to prevent message duplication on scrollback received

[Alexandr Akulich]

> On Sept. 20, 2016, 6:39 p.m., Aleix Pol Gonzalez wrote:
> > Ship It!
> 
> Albert Astals Cid wrote:
> Ping any reason this is not commited?

I think I said that in IRC: spec says that we should not rely on message-token 
in this case.
https://telepathy.freedesktop.org/spec/Channel_Interface_Messages.html#Simple-Type:Protocol_Message_Token
As we discussed at FOSDEM with George Kiagiadakis, we need to either add an 
another key to message-header, or completely rewrite logging support.

We need to discuss this a bit more to decide, if this patch should be 
completely discarded, or reworked for another header key.


- Alexandr


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


On Sept. 20, 2016, 6: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, 6: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: Telepathy KDE maintainership

[Alexandr Akulich]

Hi all.

On Mon, Jan 30, 2017 at 3:00 AM, Aleix Pol  wrote:
> Hi James,
> Do all these messages mean that you'll take over KDE Telepathy
> maintainrship as Martin suggested?
>
> I'd say that Telepathy KDE needs more than just refactoring on the
> presence code.
>
> Aleix

I second that.

Telepathy KDE needs much more, IMO we need to redesign a part of
Telepathy bindings for Qt to make KTp and other clients easier. We
need to make an extensible API for 'Message' classes and finally
propose a declarative API.

Aleix, I would like to share my ideas with you and other Telepathy
devs at FOSDEM. I'll contact you later, if you're still interesting in
Telepathy. I'm going to get Schengen Visa tomorrow, so I will be able
to speak about Tp meeting for sure.

Re: Review Request 129069: [Quick Chat] Remove unused popupSide property

[Alexandr Akulich]

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


Ship it!




Ship It!

- Alexandr Akulich


On Sept. 29, 2016, 1:20 p.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, 1:20 p.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 129016: [ktp-auth-handler] Fixed uninitialized variable usage

[Alexandr Akulich]

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

(Updated Sept. 26, 2016, 7:55 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy and Aleix Pol Gonzalez.


Changes
---

Submitted with commit 03508d05afe02c17ed7066d78d6f42283c2e14e5 by Alexandr 
Akulich to branch master.


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 129017: [ktp-auth-handler] Ported from deprecated KAboutData::setProgramIconName()

[Alexandr Akulich]

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

(Updated Sept. 26, 2016, 7:55 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit be85071f0bb735e1b6ab90dcc0049db40cdc3fe8 by Alexandr 
Akulich to branch master.


Repository: ktp-auth-handler


Description
---

KAboutData::setProgramIconName() replaced by 
QApplication::setWindowIcon(QIcon::fromTheme()), as it is recommended at the 
KAboutData header file


Diffs
-

  main.cpp 2243100 

Diff: https://git.reviewboard.kde.org/r/129017/diff/


Testing
---

Compiles


Thanks,

Alexandr Akulich

Review Request 129017: [ktp-auth-handler] Ported from deprecated KAboutData::setProgramIconName()

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-auth-handler


Description
---

KAboutData::setProgramIconName() replaced by 
QApplication::setWindowIcon(QIcon::fromTheme()), as it is recommended at the 
KAboutData header file


Diffs
-

  main.cpp 2243100 

Diff: https://git.reviewboard.kde.org/r/129017/diff/


Testing
---

Compiles


Thanks,

Alexandr Akulich

Review Request 129016: [ktp-auth-handler] Fixed uninitialized variable usage

[Alexandr Akulich]

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

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 123504: ktp-kded-module Now Playing multiple players for status handler multiple account presence

[Alexandr Akulich]

> On Sept. 23, 2016, 1:03 p.m., Alexandr Akulich wrote:
> > telepathy-mpris.cpp, line 96
> > <https://git.reviewboard.kde.org/r/123504/diff/5/?file=477191#file477191line96>
> >
> > Here you're iterating over non-const list of dbus services (usually 
> > 60-100 entries).
> 
> James Smith wrote:
> According to the documentation this is const.
> 
> Alexandr Akulich wrote:
> According to the documentation, this is type T, which is 
> ```QStringList``` for ```QDBusPendingReply```, which is not 
> const. (I'm not about constness of the QDBusPendingReply::value() method, but 
> about the type of container in the ```for```. We have ```for (const QString & 
> : QStringList)``` here. Container is not const, so iteration will cause 
> detach. Correct me, if I'm wrong.

There is a typical rvalue and this is what qAsConst can be for.


- Alexandr


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


On Sept. 23, 2016, 5:41 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123504/
> ---
> 
> (Updated Sept. 23, 2016, 5:41 a.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> ---
> 
> New features:
> -Much improved multiple player handling; a number of service availability 
> bugs were fixed.
> -Ignore tracks with inadequate metadata
> -Separator for empty metadata info fields with (currently) hidden config
> 
> 
> Diffs
> -
> 
>   telepathy-mpris.h 05b77c90a50372fd9ed66bde0ab8a287caf34b51 
>   telepathy-mpris.cpp ee0e622c68bdd156e45914f542d2fe13f0ddb610 
> 
> Diff: https://git.reviewboard.kde.org/r/123504/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 123504: ktp-kded-module Now Playing multiple players for status handler multiple account presence

[Alexandr Akulich]

> On Sept. 23, 2016, 1:03 p.m., Alexandr Akulich wrote:
> > telepathy-mpris.cpp, line 96
> > <https://git.reviewboard.kde.org/r/123504/diff/5/?file=477191#file477191line96>
> >
> > Here you're iterating over non-const list of dbus services (usually 
> > 60-100 entries).
> 
> James Smith wrote:
> According to the documentation this is const.

According to the documentation, this is type T, which is ```QStringList``` for 
```QDBusPendingReply```, which is not const. (I'm not about 
constness of the QDBusPendingReply::value() method, but about the type of 
container in the ```for```. We have ```for (const QString & : QStringList)``` 
here. Container is not const, so iteration will cause detach. Correct me, if 
I'm wrong.


- Alexandr


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


On Sept. 23, 2016, 5:41 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123504/
> ---
> 
> (Updated Sept. 23, 2016, 5:41 a.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> ---
> 
> New features:
> -Much improved multiple player handling; a number of service availability 
> bugs were fixed.
> -Ignore tracks with inadequate metadata
> -Separator for empty metadata info fields with (currently) hidden config
> 
> 
> Diffs
> -
> 
>   telepathy-mpris.h 05b77c90a50372fd9ed66bde0ab8a287caf34b51 
>   telepathy-mpris.cpp ee0e622c68bdd156e45914f542d2fe13f0ddb610 
> 
> Diff: https://git.reviewboard.kde.org/r/123504/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 123504: ktp-kded-module Now Playing multiple players for status handler multiple account presence

[Alexandr Akulich]

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




telepathy-mpris.cpp (line 80)
<https://git.reviewboard.kde.org/r/123504/#comment66915>

Here you're iterating over non-const list of dbus services (usually 60-100 
entries).


- Alexandr Akulich


On Sept. 23, 2016, 5:41 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123504/
> ---
> 
> (Updated Sept. 23, 2016, 5:41 a.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> ---
> 
> New features:
> -Much improved multiple player handling; a number of service availability 
> bugs were fixed.
> -Ignore tracks with inadequate metadata
> -Separator for empty metadata info fields with (currently) hidden config
> 
> 
> Diffs
> -
> 
>   telepathy-mpris.h 05b77c90a50372fd9ed66bde0ab8a287caf34b51 
>   telepathy-mpris.cpp ee0e622c68bdd156e45914f542d2fe13f0ddb610 
> 
> Diff: https://git.reviewboard.kde.org/r/123504/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Re: Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)

[Alexandr Akulich]

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

(Updated Sept. 22, 2016, 12:25 p.m.)


Status
--

This change has been marked as submitted.


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)

[Alexandr Akulich]

> On Sept. 21, 2016, 8:32 p.m., Aleix Pol Gonzalez wrote:
> > otr-proxy/KTpProxy/svc-channel-proxy.cpp, line 63
> > <https://git.reviewboard.kde.org/r/128979/diff/1/?file=477266#file477266line63>
> >
> > Wait, this should be >=0

No, the fix is correct. :-)

If (index of the method < 0) then method is not implemented.

No difference with https://git.reviewboard.kde.org/r/128844/diff/ or 
https://cgit.freedesktop.org/telepathy/telepathy-qt/commit/?id=a7941ac1de8ca9f3599d539646638d0c3c82b7d2


- Alexandr


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


On Sept. 21, 2016, 7: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, 7: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 123504: ktp-kded-module Now Playing multiple players for status handler multiple account presence

[Alexandr Akulich]

> On Sept. 21, 2016, 8:19 p.m., Martin Klapetek wrote:
> > > Use c++11 for loops instead of Q_FOREACH for new code.
> > 
> > Please don't do that, it's slow. See 
> > http://www.dvratil.cz/2015/06/qt-containers-and-c11-range-based-loops/ for 
> > more details.
> > 
> > That said, I still don't agree with this patch.

qAsConst?


- Alexandr


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


On Sept. 18, 2016, 3:03 a.m., James Smith wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123504/
> ---
> 
> (Updated Sept. 18, 2016, 3:03 a.m.)
> 
> 
> Review request for Telepathy and Martin Klapetek.
> 
> 
> Repository: ktp-kded-module
> 
> 
> Description
> ---
> 
> New features:
> -Much improved multiple player handling; a number of service availability 
> bugs were fixed.
> -Ignore tracks with inadequate metadata
> -Separator for empty metadata info fields with (currently) hidden config
> 
> 
> Diffs
> -
> 
>   telepathy-mpris.h 05b77c90a50372fd9ed66bde0ab8a287caf34b51 
>   telepathy-mpris.cpp ee0e622c68bdd156e45914f542d2fe13f0ddb610 
> 
> Diff: https://git.reviewboard.kde.org/r/123504/diff/
> 
> 
> Testing
> ---
> 
> Compile, run.
> 
> 
> Thanks,
> 
> James Smith
> 
>

Review Request 128979: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance (ChannelProxyInterfaceOTRAdaptor at this time)

[Alexandr Akulich]

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

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 128961: [KTp/Declarative/MessagesModel] Fixed allocation of modelIndex variable in onMessageSent()

[Alexandr Akulich]

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

(Updated Sept. 20, 2016, 2:37 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit 2706d8d60cedcdcc31067122c909797d506ff629 by Alexandr 
Akulich to branch master.


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

Review Request 128961: [KTp/Declarative/MessagesModel] Fixed allocation of modelIndex variable in onMessageSent()

[Alexandr Akulich]

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

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

[Alexandr Akulich]

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

(Updated Sept. 20, 2016, 6:27 p.m.)


Review request for Telepathy.


Changes
---

Fixed modelIndex variable type.


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 (updated)
-

  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

[Alexandr Akulich]

> On Sept. 20, 2016, 6:13 p.m., Aleix Pol Gonzalez wrote:
> > KTp/Declarative/messages-model.cpp, line 245
> > <https://git.reviewboard.kde.org/r/128960/diff/1/?file=477175#file477175line245>
> >
> > this shouldn't be a reference, as you're actually storing it.

Copied from the first usage:
https://github.com/KDE/ktp-common-internals/blob/master/KTp/Declarative/messages-model.cpp#L257

But I agree, this is a bug which I missed.

I will add another PR to fix it in the first place and update this PR too.


- Alexandr


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


On Sept. 20, 2016, 6: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, 6: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
> 
>

Review Request 128960: [KTp/Declarative/MessagesModel] Used token to prevent incoming message duplication

[Alexandr Akulich]

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

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

[Alexandr Akulich]

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

(Updated Sept. 20, 2016, 5:46 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit cbd7bb27f5caa776c3a41bcb5d35bcf4cb2376b2 by Alexandr 
Akulich to branch master.


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

[Alexandr Akulich]

> On Sept. 20, 2016, 3: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.

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!


- Alexandr


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


On Sept. 20, 2016, 2:51 p.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, 2:51 p.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

[Alexandr Akulich]

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

(Updated Sept. 20, 2016, 7:05 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit 431db458de1417764ecdef00974fca4c412e19f8 by Alexandr 
Akulich to branch master.


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

[Alexandr Akulich]

> On Sept. 20, 2016, 3: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

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?


- Alexandr


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


On Sept. 20, 2016, 2:51 p.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, 2:51 p.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

[Alexandr Akulich]

> On Sept. 20, 2016, 3: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.

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.


- Alexandr


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


On Sept. 20, 2016, 2:51 p.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, 2:51 p.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
> 
>

Review Request 128954: [KTp/Declarative/MessagesModel] Implemented message sort by sent timestamp

[Alexandr Akulich]

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

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 128884: Disable QWebView's object cache when the "disableStyleCache" option is set

[Alexandr Akulich]

> On Sept. 11, 2016, 1:50 a.m., Alexandr Akulich wrote:
> > lib/adium-theme-view.cpp, line 81
> > <https://git.reviewboard.kde.org/r/128884/diff/1/?file=476708#file476708line81>
> >
> > Please, use KTpStyleDebug here.
> 
> Mariusz Glebocki wrote:
> OK. Wouldn't it be a good idea to also change it in 
> chat-window-style-manager.cpp?
> 
> Alexandr Akulich wrote:
> You're right. The source of the file seems to be taken from Kopete 
> without changes.
> Probably it should go into a different PR, but it should not delay the 
> change, because I'll review it fast.
> Offtop: do you visit our irc channel (#kde-telepathy at freenode)?
> 
> Mariusz Glebocki wrote:
> I attached renaming patch as a file, isn't it a problem?
> OT: not yet, but I'll join in a week or two.

Works for me. :)


- Alexandr


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


On Sept. 11, 2016, 3 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128884/
> ---
> 
> (Updated Sept. 11, 2016, 3 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> The patch disables QWebView's object cache when the "disableStyleCache" 
> option is set.
> 
> When the style cache is disabled (see 
> https://github.com/KDE/ktp-text-ui/blob/master/lib/chat-window-style-manager.cpp#L310)
>  all style files should be reloaded on a style change. This is done for at 
> least all .html files. However, .css (and possibly .js) files are cached in 
> QWebView, so they are read from the cache when referenced by `@import`.
> 
> ## How to reproduce the problem
> 
> - Append following setting to `~/.config/ktp-text-uirc`:
> ```
> [KopeteStyleDebug]
> disableStyleCache=true
> ```
> 
> - Run ktp-text-ui, open any chat
> 
> - optionally, change the chat style to one which uses main.css file (like 
> SimKete)
> 
> - without closing the chat window, backup the style's main.css file  
> (`~/.local/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`
>  or 
> `/usr/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`)
>  and modify it in some noticeable way. Example modification for SimKete:
> ```
> body {
> font-size: 12px;
> /* background-color: #e3e3e3; */
> background-color: red;
> }
> ```
> 
> - reload the style in ktp-text-ui by changing it to another one and back in 
> the settings
> 
> - the result is visible in a chat preview
> 
> - optionally, click apply/ok to verify results in real chat window
> 
> ### Results:
> 
> - without patch: the style looks like before; ktp-text-ui needs restart to 
> use a new stylesheet
> 
> - with patch: the style uses changed main.css file (red background for 
> SimKete example)
> 
> *NOTE:* Don't forget to restore original main.css
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-view.cpp 26e6d50 
> 
> Diff: https://git.reviewboard.kde.org/r/128884/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> - multiple similar tests with own style: OK
> - the cache is not disabled when "disableStyleCache" is not set: OK
> 
> 
> File Attachments
> 
> 
> Companion patch: Rename "KopeteStyleDebug" to "KTpStyleDebug"
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/09/10/bfd8608a-f528-43c7-9905-1b2a018a455a__0001-lib-Rename-KopeteStyleDebug-to-KTpStyleDebug.patch
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128884: Disable QWebView's object cache when the "disableStyleCache" option is set

[Alexandr Akulich]

> On Sept. 11, 2016, 1:50 a.m., Alexandr Akulich wrote:
> > lib/adium-theme-view.cpp, line 81
> > <https://git.reviewboard.kde.org/r/128884/diff/1/?file=476708#file476708line81>
> >
> > Please, use KTpStyleDebug here.
> 
> Mariusz Glebocki wrote:
> OK. Wouldn't it be a good idea to also change it in 
> chat-window-style-manager.cpp?

You're right. The source of the file seems to be taken from Kopete without 
changes.
Probably it should go into a different PR, but it should not delay the change, 
because I'll review it fast.
Offtop: do you visit our irc channel (#kde-telepathy at freenode)?


- Alexandr


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


On Sept. 11, 2016, 12:56 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128884/
> ---
> 
> (Updated Sept. 11, 2016, 12:56 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> The patch disables QWebView's object cache when the "disableStyleCache" 
> option is set.
> 
> When the style cache is disabled (see 
> https://github.com/KDE/ktp-text-ui/blob/master/lib/chat-window-style-manager.cpp#L310)
>  all style files should be reloaded on a style change. This is done for at 
> least all .html files. However, .css (and possibly .js) files are cached in 
> QWebView, so they are read from the cache when referenced by `@import`.
> 
> ## How to reproduce the problem
> 
> - Append following setting to `~/.config/ktp-text-uirc`:
> ```
> [KopeteStyleDebug]
> disableStyleCache=true
> ```
> 
> - Run ktp-text-ui, open any chat
> 
> - optionally, change the chat style to one which uses main.css file (like 
> SimKete)
> 
> - without closing the chat window, backup the style's main.css file  
> (`~/.local/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`
>  or 
> `/usr/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`)
>  and modify it in some noticeable way. Example modification for SimKete:
> ```
> body {
> font-size: 12px;
> /* background-color: #e3e3e3; */
> background-color: red;
> }
> ```
> 
> - reload the style in ktp-text-ui by changing it to another one and back in 
> the settings
> 
> - the result is visible in a chat preview
> 
> - optionally, click apply/ok to verify results in real chat window
> 
> ### Results:
> 
> - without patch: the style looks like before; ktp-text-ui needs restart to 
> use a new stylesheet
> 
> - with patch: the style uses changed main.css file (red background for 
> SimKete example)
> 
> *NOTE:* Don't forget to restore original main.css
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-view.cpp 26e6d50 
> 
> Diff: https://git.reviewboard.kde.org/r/128884/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> - multiple similar tests with own style: OK
> - the cache is not disabled when "disableStyleCache" is not set: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128884: Disable QWebView's object cache when the "disableStyleCache" option is set

[Alexandr Akulich]

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


Ship it!




Ship It!

- Alexandr Akulich


On Sept. 11, 2016, 12:56 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128884/
> ---
> 
> (Updated Sept. 11, 2016, 12:56 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> The patch disables QWebView's object cache when the "disableStyleCache" 
> option is set.
> 
> When the style cache is disabled (see 
> https://github.com/KDE/ktp-text-ui/blob/master/lib/chat-window-style-manager.cpp#L310)
>  all style files should be reloaded on a style change. This is done for at 
> least all .html files. However, .css (and possibly .js) files are cached in 
> QWebView, so they are read from the cache when referenced by `@import`.
> 
> ## How to reproduce the problem
> 
> - Append following setting to `~/.config/ktp-text-uirc`:
> ```
> [KopeteStyleDebug]
> disableStyleCache=true
> ```
> 
> - Run ktp-text-ui, open any chat
> 
> - optionally, change the chat style to one which uses main.css file (like 
> SimKete)
> 
> - without closing the chat window, backup the style's main.css file  
> (`~/.local/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`
>  or 
> `/usr/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`)
>  and modify it in some noticeable way. Example modification for SimKete:
> ```
> body {
> font-size: 12px;
> /* background-color: #e3e3e3; */
> background-color: red;
> }
> ```
> 
> - reload the style in ktp-text-ui by changing it to another one and back in 
> the settings
> 
> - the result is visible in a chat preview
> 
> - optionally, click apply/ok to verify results in real chat window
> 
> ### Results:
> 
> - without patch: the style looks like before; ktp-text-ui needs restart to 
> use a new stylesheet
> 
> - with patch: the style uses changed main.css file (red background for 
> SimKete example)
> 
> *NOTE:* Don't forget to restore original main.css
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-view.cpp 26e6d50 
> 
> Diff: https://git.reviewboard.kde.org/r/128884/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> - multiple similar tests with own style: OK
> - the cache is not disabled when "disableStyleCache" is not set: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128884: Disable QWebView's object cache when the "disableStyleCache" option is set

[Alexandr Akulich]

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




lib/adium-theme-view.cpp (line 81)
<https://git.reviewboard.kde.org/r/128884/#comment66703>

Please, use KTpStyleDebug here.


- Alexandr Akulich


On Sept. 11, 2016, 12:56 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128884/
> ---
> 
> (Updated Sept. 11, 2016, 12:56 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> The patch disables QWebView's object cache when the "disableStyleCache" 
> option is set.
> 
> When the style cache is disabled (see 
> https://github.com/KDE/ktp-text-ui/blob/master/lib/chat-window-style-manager.cpp#L310)
>  all style files should be reloaded on a style change. This is done for at 
> least all .html files. However, .css (and possibly .js) files are cached in 
> QWebView, so they are read from the cache when referenced by `@import`.
> 
> ## How to reproduce the problem
> 
> - Append following setting to `~/.config/ktp-text-uirc`:
> ```
> [KopeteStyleDebug]
> disableStyleCache=true
> ```
> 
> - Run ktp-text-ui, open any chat
> 
> - optionally, change the chat style to one which uses main.css file (like 
> SimKete)
> 
> - without closing the chat window, backup the style's main.css file  
> (`~/.local/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`
>  or 
> `/usr/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`)
>  and modify it in some noticeable way. Example modification for SimKete:
> ```
> body {
> font-size: 12px;
> /* background-color: #e3e3e3; */
> background-color: red;
> }
> ```
> 
> - reload the style in ktp-text-ui by changing it to another one and back in 
> the settings
> 
> - the result is visible in a chat preview
> 
> - optionally, click apply/ok to verify results in real chat window
> 
> ### Results:
> 
> - without patch: the style looks like before; ktp-text-ui needs restart to 
> use a new stylesheet
> 
> - with patch: the style uses changed main.css file (red background for 
> SimKete example)
> 
> *NOTE:* Don't forget to restore original main.css
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-view.cpp 26e6d50 
> 
> Diff: https://git.reviewboard.kde.org/r/128884/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> - multiple similar tests with own style: OK
> - the cache is not disabled when "disableStyleCache" is not set: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128884: Disable QWebView's object cache when the "disableStyleCache" option is set

[Alexandr Akulich]

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



Looking good, but I think we should use KTpStyleDebug instead of 
KopeteStyleDebug.

Fix & ship it!

- Alexandr Akulich


On Sept. 11, 2016, 12:56 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128884/
> ---
> 
> (Updated Sept. 11, 2016, 12:56 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> The patch disables QWebView's object cache when the "disableStyleCache" 
> option is set.
> 
> When the style cache is disabled (see 
> https://github.com/KDE/ktp-text-ui/blob/master/lib/chat-window-style-manager.cpp#L310)
>  all style files should be reloaded on a style change. This is done for at 
> least all .html files. However, .css (and possibly .js) files are cached in 
> QWebView, so they are read from the cache when referenced by `@import`.
> 
> ## How to reproduce the problem
> 
> - Append following setting to `~/.config/ktp-text-uirc`:
> ```
> [KopeteStyleDebug]
> disableStyleCache=true
> ```
> 
> - Run ktp-text-ui, open any chat
> 
> - optionally, change the chat style to one which uses main.css file (like 
> SimKete)
> 
> - without closing the chat window, backup the style's main.css file  
> (`~/.local/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`
>  or 
> `/usr/share/ktelepathy/styles/.AdiumMessageStyle/Contents/Resources/main.css`)
>  and modify it in some noticeable way. Example modification for SimKete:
> ```
> body {
> font-size: 12px;
> /* background-color: #e3e3e3; */
> background-color: red;
> }
> ```
> 
> - reload the style in ktp-text-ui by changing it to another one and back in 
> the settings
> 
> - the result is visible in a chat preview
> 
> - optionally, click apply/ok to verify results in real chat window
> 
> ### Results:
> 
> - without patch: the style looks like before; ktp-text-ui needs restart to 
> use a new stylesheet
> 
> - with patch: the style uses changed main.css file (red background for 
> SimKete example)
> 
> *NOTE:* Don't forget to restore original main.css
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-view.cpp 26e6d50 
> 
> Diff: https://git.reviewboard.kde.org/r/128884/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> - multiple similar tests with own style: OK
> - the cache is not disabled when "disableStyleCache" is not set: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128867: [KTp/Message] Direction of received message now depends on sender

[Alexandr Akulich]

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




KTp/message.cpp (line 84)
<https://git.reviewboard.kde.org/r/128867/#comment66679>

Sure, I can rewrite it without the isLocalToRemote variable, but it is 
cleaner in this way IMO.


- Alexandr Akulich


On Sept. 9, 2016, 3: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, 3: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
> 
>

Review Request 128867: [KTp/Message] Direction of received message now depends on sender

[Alexandr Akulich]

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

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 128847: [ktp-common-internals] [debugger] Split logic and UI

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 7:54 p.m.)


Review request for Telepathy.


Changes
---

Added a link to draft of the next commit


Repository: ktp-common-internals


Description (updated)
---

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 128847: [ktp-common-internals] [debugger] Split logic and UI

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 6 p.m.)


Review request for Telepathy.


Changes
---

Removed m_process member from the debug view class.
Thanks to Anthony Fieroni.


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.


Diffs (updated)
-

  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 128847: [ktp-common-internals] [debugger] Split logic and UI

[Alexandr Akulich]

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




tools/debugger/debug-message-view.cpp (line 83)
<https://git.reviewboard.kde.org/r/128847/#comment66613>

Thank you. I leaked this too early. :)


- Alexandr Akulich


On Sept. 6, 2016, 5:26 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, 5:26 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.
> 
> 
> 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
> 
>

Review Request 128847: [ktp-common-internals] [debugger] Split logic and UI

[Alexandr Akulich]

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

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) show one process just once, independently of a number 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.


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 128845: [ktp-common-internals] [debugger] Cleanup and reformat

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 12:11 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit 17b0f5e4a7600487b9d92e766c6f1f44b8aa014a by Alexandr 
Akulich to branch master.


Repository: ktp-common-internals


Description
---

Removed unneeded includes, destructor moved to be the next after constructor, 
removed incorrect "virtual" usage, fixed braces and so on.

This is a preparation to logic and ui split up, which I can not upload until 
this commit will be landed.


Diffs
-

  tools/debugger/debug-message-view.h 0bde707 
  tools/debugger/debug-message-view.cpp af09715 
  tools/debugger/main-window.h 2e7c968 

Diff: https://git.reviewboard.kde.org/r/128845/diff/


Testing
---

Compiles


Thanks,

Alexandr Akulich

Re: Review Request 128844: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 11:58 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit ccdf6a3c006a0f385a1224ee45f89a3776aa0fb9 by Alexandr 
Akulich to branch master.


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 128845: [ktp-common-internals] [debugger] Cleanup and reformat

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 4:17 p.m.)


Review request for Telepathy.


Changes
---

Reformatted code. Destructor moved to be the next after constructor, removed 
incorrect "virtual" usage, fixed braces and so on.


Summary (updated)
-

[ktp-common-internals] [debugger] Cleanup and reformat


Repository: ktp-common-internals


Description (updated)
---

Removed unneeded includes, destructor moved to be the next after constructor, 
removed incorrect "virtual" usage, fixed braces and so on.


Diffs (updated)
-

  tools/debugger/debug-message-view.h 0bde707 
  tools/debugger/debug-message-view.cpp af09715 
  tools/debugger/main-window.h 2e7c968 

Diff: https://git.reviewboard.kde.org/r/128845/diff/


Testing
---

Compiles


Thanks,

Alexandr Akulich

Review Request 128845: [ktp-common-internals] [debugger] Removed unneeded includes

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-common-internals


Description
---

Removed unneeded includes


Diffs
-

  tools/debugger/debug-message-view.cpp af09715 

Diff: https://git.reviewboard.kde.org/r/128845/diff/


Testing
---

Compiles


Thanks,

Alexandr Akulich

Review Request 128844: [ktp-common-internals] [otr-proxy] Fixed incorrect check of adaptee method existance

[Alexandr Akulich]

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

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 128807: [k-c-i/kpeople] Final initialization step moved from loadCache to constructor

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 11:22 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit 3989778fafde7fbf8bf2e4b7b439a0b401050187 by Alexandr 
Akulich to branch master.


Repository: ktp-common-internals


Description
---

The moved code should be called just once on construction and *not* on each 
cache load (which is called on (any) account presence changed to offline).

I removed Qt::UniqueConnection argument, because it is not needed anymore.


Diffs
-

  kpeople/datasourceplugin/im-persons-data-source.cpp 7f1d08e 

Diff: https://git.reviewboard.kde.org/r/128807/diff/


Testing
---

Works


Thanks,

Alexandr Akulich

Re: Review Request 128809: [k-c-i / debugger] Fixed timestamp format (ms width)

[Alexandr Akulich]

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

(Updated Sept. 6, 2016, 9:22 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Changes
---

Submitted with commit b814213ac6cfdcdbfab3986fed15ed4732168e95 by Alexandr 
Akulich to branch master.


Repository: ktp-common-internals


Description
---

Unfixed ms width ("%d") leads to vary digits from 1 to 6.
Now the width is statically 6 digits, filled with leading zeros ("%06d")


Diffs
-

  tools/debugger/debug-message-view.cpp b29bc71 

Diff: https://git.reviewboard.kde.org/r/128809/diff/


Testing
---

Works. No more dancing of columns after timestamp.


Thanks,

Alexandr Akulich

Re: Review Request 128842: Insert chat text edit's history as plain text

[Alexandr Akulich]

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


Ship it!




Ship It!

- Alexandr Akulich


On Sept. 6, 2016, 2:50 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128842/
> ---
> 
> (Updated Sept. 6, 2016, 2:50 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> How to reproduce bug:
> 
> - type/paste some HTML code in chat text edit, eg. ` style="background-color: deeppink">test`
> - press down key
> - press up key to insert last history item
> 
> result: the text is formatted, HTML markup is lost
> result with the patch: the text is inserted in a form it was before
> 
> The patch changes nick completion to plain text too, I guess there is someone 
> with `creatve nckname` ;)
> 
> 
> Diffs
> -
> 
>   lib/chat-text-edit.cpp 4d16e20 
> 
> Diff: https://git.reviewboard.kde.org/r/128842/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - use case from the description gives expected result: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128841: Fix message classes for history items

[Alexandr Akulich]

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


Ship it!




Thank you! You fixed a known bug:
https://bugs.kde.org/show_bug.cgi?id=348929

- Alexandr Akulich


On Sept. 6, 2016, 1:17 a.m., Mariusz Glebocki wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128841/
> ---
> 
> (Updated Sept. 6, 2016, 1:17 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-text-ui
> 
> 
> Description
> ---
> 
> "incoming" and "outgoing" classes for history messages are swapped - they are 
> used for local to remote and remote to local messages, respectively. This bug 
> is visible in styles that use a single .html file for sent and received 
> messages, relying on a class.
> I don't know which theme could be used to test it - I spotted this while 
> playing with theme creation. Everything without an "Outgoing" directory and 
> with distinguishable incoming/outgoing messages should be ok.
> 
> 
> Diffs
> -
> 
>   lib/adium-theme-message-info.cpp ce1d5cd 
> 
> Diff: https://git.reviewboard.kde.org/r/128841/diff/
> 
> 
> Testing
> ---
> 
> - compile/run: OK
> - talk with someone, close the window, open the chat again, check the last 
> chat message's direction: OK
> 
> 
> Thanks,
> 
> Mariusz Glebocki
> 
>

Re: Review Request 128809: [k-c-i / debugger] Fixed timestamp format (ms width)

[Alexandr Akulich]

> On Sept. 1, 2016, 3:51 p.m., David Edmundson wrote:
> >

What'd you mean?


- Alexandr


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


On Sept. 1, 2016, 12:59 p.m., Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128809/
> ---
> 
> (Updated Sept. 1, 2016, 12:59 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-common-internals
> 
> 
> Description
> ---
> 
> Unfixed ms width ("%d") leads to vary digits from 1 to 6.
> Now the width is statically 6 digits, filled with leading zeros ("%06d")
> 
> 
> Diffs
> -
> 
>   tools/debugger/debug-message-view.cpp b29bc71 
> 
> Diff: https://git.reviewboard.kde.org/r/128809/diff/
> 
> 
> Testing
> ---
> 
> Works. No more dancing of columns after timestamp.
> 
> 
> Thanks,
> 
> Alexandr Akulich
> 
>

Review Request 128809: [k-c-i / debugger] Fixed timestamp format (ms width)

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-common-internals


Description
---

Unfixed ms width ("%d") leads to vary digits from 1 to 6.
Now the width is statically 6 digits, filled with leading zeros ("%06d")


Diffs
-

  tools/debugger/debug-message-view.cpp b29bc71 

Diff: https://git.reviewboard.kde.org/r/128809/diff/


Testing
---

Works. No more dancing of columns after timestamp.


Thanks,

Alexandr Akulich

Re: Review Request 126602: Fix linking against gobject-2.0 on FreeBSD

[Alexandr Akulich]

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



Tobias, is this still needed? It looks like we need to apply the same patch to 
TelepathyQt too. I would like to ship this and to make a similar commit to 
TpQt, but I can't test it. What do you think?

- Alexandr Akulich


On Aug. 18, 2016, 12:03 a.m., Tobias Berner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126602/
> ---
> 
> (Updated Aug. 18, 2016, 12:03 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: telepathy-logger-qt
> 
> 
> Description
> ---
> 
> The cmake module file cmake/modules/FindGObject.cmake uses pkgconfig. 
> Therefore
> the library returned in GOBJECT_LIBRARY does not contain the absolute path.
> 
> This makes linking fail with
> | [ 91%] Linking CXX shared library libtelepathy-logger-qt.so
> | /usr/bin/ld: cannot find -lgobject-2.0
> 
> This patch extend FindGObject.cmake to also export GOBJECT_LIBRARY_DIRS as
> provided by pkgconfig, and adding it to the link_directories in
> TelepathyLoggerQt/CMakeLists.txt
> 
> 
> This probably is not the most elegant way to fix the issue...
> 
> 
> Diffs
> -
> 
>   TelepathyLoggerQt/CMakeLists.txt 9790bdf 
>   cmake/modules/FindGObject.cmake 1507b43 
> 
> Diff: https://git.reviewboard.kde.org/r/126602/diff/
> 
> 
> Testing
> ---
> 
> build locally.
> 
> 
> Thanks,
> 
> Tobias Berner
> 
>

Re: Review Request 128774: ktp-text-ui: Added a filter to show info about linked github issue or pull request (WIP)

[Alexandr Akulich]

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

(Updated Aug. 26, 2016, 11:19 p.m.)


Review request for Telepathy.


Changes
---

Forgot to say "thanks!" to bugzilla authors for the escapeHTML() function.


Repository: ktp-text-ui


Description (updated)
---

!Not intended to be shipped yet!
Preview of a new filter, which works similar to bugzilla. The filter shows 
issue info retrieved for github via API.
There is some known issues with the filter, but I would like to share it early 
to get your valueable reviews :)
I would like to discuss:
- Is it appropriate name? This filter can be extended to support gitlab and may 
be more providers later.
- What do you think about format of the info?

I thought on it and I should say that this filter has nothing in share with 
"bugzilla" filter. Except the purpose. :)

P.S. Thanks to the bugzilla plugin for the escapeHTML() function.


Diffs
-

  filters/CMakeLists.txt ce06ba7 
  filters/github/CMakeLists.txt PRE-CREATION 
  filters/github/github-filter.h PRE-CREATION 
  filters/github/github-filter.cpp PRE-CREATION 
  filters/github/ktptextui_message_filter_github.desktop.cmake PRE-CREATION 
  filters/github/showGithubInfo.js PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/128774/diff/


Testing
---

Does not work reliably yet.


File Attachments


Filter in action
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/08/26/c171785f-3e77-4517-a32b-1a134504da77__out2.gif


Thanks,

Alexandr Akulich

Review Request 128774: ktp-text-ui: Added a filter to show info about linked github issue or pull request (WIP)

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-text-ui


Description
---

!Not intended to be shipped yet!
Preview of a new filter, which works similar to bugzilla. The filter shows 
issue info retrieved for github via API.
There is some known issues with the filter, but I would like to share it early 
to get your valueable reviews :)
I would like to discuss:
- Is it appropriate name? This filter can be extended to support gitlab and may 
be more providers later.
- What do you think about format of the info?

I thought on it and I should say that this filter has nothing in share with 
"bugzilla" filter. Except the purpose. :)


Diffs
-

  filters/CMakeLists.txt ce06ba7 
  filters/github/CMakeLists.txt PRE-CREATION 
  filters/github/github-filter.h PRE-CREATION 
  filters/github/github-filter.cpp PRE-CREATION 
  filters/github/ktptextui_message_filter_github.desktop.cmake PRE-CREATION 
  filters/github/showGithubInfo.js PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/128774/diff/


Testing
---

Does not work reliably yet.


File Attachments


Filter in action
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/08/26/c171785f-3e77-4517-a32b-1a134504da77__out2.gif


Thanks,

Alexandr Akulich

Re: Review Request 128711: avoid accesing QCoreApplication in static constant before an instance was created

[Alexandr Akulich]

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


Ship it!




Ship It!

- Alexandr Akulich


On Aug. 18, 2016, 9:32 p.m., Martin Koller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128711/
> ---
> 
> (Updated Aug. 18, 2016, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-contact-list
> 
> 
> Description
> ---
> 
> Without this patch starting ktp-contactlist prints 4 times
> QCoreApplication::arguments: Please instantiate the QApplication object first
> even before hitting a breakpoint in main.
> The reason is the constant definition this patch fixes, since it uses 
> IconSize() which internally uses KIconLoader which somehow accesses 
> QCoreApplication::arguments
> (don't have the full stack here right now).
> The result is also that no icons are shown and when closing the window the 
> program crashes.
> 
> The patch fixes this by not using a const variable which is initialized too 
> early but just uses
> a #define which produces the code which is only run when the class instances 
> are created.
> 
> Now no messages are printed from Qt, icons are shown, and the program no 
> longer crashes on exit.
> 
> 
> Diffs
> -
> 
>   contact-overlays.cpp e8604be 
> 
> Diff: https://git.reviewboard.kde.org/r/128711/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Martin Koller
> 
>

Re: Review Request 128711: avoid accesing QCoreApplication in static constant before an instance was created

[Alexandr Akulich]

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



I have the same issue on my working machine, but I could not reproduce it at 
home or anywhere else, so I decided that it is an issue of my six years old 
system after crazy live migration from funtoo with qt4 to gentoo with qt5.
I fixed it a bit differently; replaced with a function, which returns the same 
static var.

- Alexandr Akulich


On Aug. 18, 2016, 9:32 p.m., Martin Koller wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128711/
> ---
> 
> (Updated Aug. 18, 2016, 9:32 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Repository: ktp-contact-list
> 
> 
> Description
> ---
> 
> Without this patch starting ktp-contactlist prints 4 times
> QCoreApplication::arguments: Please instantiate the QApplication object first
> even before hitting a breakpoint in main.
> The reason is the constant definition this patch fixes, since it uses 
> IconSize() which internally uses KIconLoader which somehow accesses 
> QCoreApplication::arguments
> (don't have the full stack here right now).
> The result is also that no icons are shown and when closing the window the 
> program crashes.
> 
> The patch fixes this by not using a const variable which is initialized too 
> early but just uses
> a #define which produces the code which is only run when the class instances 
> are created.
> 
> Now no messages are printed from Qt, icons are shown, and the program no 
> longer crashes on exit.
> 
> 
> Diffs
> -
> 
>   contact-overlays.cpp e8604be 
> 
> Diff: https://git.reviewboard.kde.org/r/128711/diff/
> 
> 
> Testing
> ---
> 
> yes
> 
> 
> Thanks,
> 
> Martin Koller
> 
>

Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated Aug. 8, 2016, 11:59 p.m.)


Status
--

This change has been marked as submitted.


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 (RFC 5870).
Uses OpenStreetMap as "backend".


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

Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated Aug. 8, 2016, 11:57 p.m.)


Review request for Telepathy.


Changes
---

Fixed compilation after string to number conversion check removed. Somehow I 
uploaded broken code.


Repository: ktp-text-ui


Description
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).
Uses OpenStreetMap as "backend".


Diffs (updated)
-

  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

Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated Aug. 7, 2016, 2:28 a.m.)


Review request for Telepathy.


Changes
---

Removed checks for string to number conversions. It is pointless, because of 
regular expression.
Previously proposed code is broken, because it doesn't increase index (which 
leads to infinity loop).


Repository: ktp-text-ui


Description
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).
Uses OpenStreetMap as "backend".


Diffs (updated)
-

  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

Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated Aug. 7, 2016, 2:18 a.m.)


Review request for Telepathy.


Changes
---

- Added zoom optional argument support
- Used QStringRef instead of QString where possible
- Fixed constness
- Fixed style of CMakeFiles.txt
- Plaintext of the geo now will be replaced by href.


Repository: ktp-text-ui


Description
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).
Uses OpenStreetMap as "backend".


Diffs (updated)
-

  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

Re: Review Request 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote:
> > As I saw inside my browser - marble registers to handle the geo links. As I 
> > read somewhere you can also use marble to include a map / so maybe you 
> > should connect the marble guys for that ( but this is only an improvement) 
> > nothing that holds back this review.

I thought about marble too, but Image and Video messages would have much bigger 
priority for me.


> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote:
> > filters/geopoint/CMakeLists.txt, line 4
> > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471820#file471820line4>
> >
> > use same indent format in a file either a static indent or indent with (

I'm agree with the issue, but I copied this from "highlight" filter. For some 
(unknown for me) reason we have such indent in all other filters, so I just 
followed the style.
Do we have some CMake coding convension somewhere?

I changed the style to the static four spaces indentation.


> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote:
> > filters/geopoint/geopoint-filter.cpp, line 62
> > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line62>
> >
> > is this compiling with the ; ?

Yes. Actually, it does *not* compiles without the colon. We have this in all 
filters.


> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote:
> > filters/geopoint/geopoint-filter.cpp, line 42
> > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line42>
> >
> > osm using this improved geo link style (if you use the share button at 
> > the right panel at openstreetmap.org):
> > 
> > geo:50.8295,12.9225?z=16
> > 
> > so add at the end to match the zoom property:
> > 
> > (?z=(?\d+))?
> > 
> > and replace all [0-9] with \d or is there any need to mix [0-9] and \d ?
> > 
> > for sure you need to add the logic to set the zoom level to the iframe, 
> > too.

I thought to do it later, but ok, just done.


> On July 16, 2016, 12:32 p.m., Sandro Knauß wrote:
> > filters/geopoint/geopoint-filter.cpp, line 38
> > <https://git.reviewboard.kde.org/r/128460/diff/2/?file=471822#file471822line38>
> >
> > geo:%1,%2?z=%3...
> > 
> > a user still can see the geo link, that was sent. and can open/copy it 
> > like he wants.

Good idea to add a href, but, as I see (just tried it), it would be better to 
replace a geo text with geo link and add the iframe(s) to the end of message, 
rather than add href to the end.


- Alexandr


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


On July 16, 2016, 7:04 a.m., Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128460/
> ---
> 
> (Updated July 16, 2016, 7:04 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 (RFC 5870).
> Uses OpenStreetMap as "backend".
> 
> 
> 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
> 
>

Re: Review Request 128459: ktp-text-ui: Cleanup headers

[Alexandr Akulich]

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

(Updated Aug. 4, 2016, 2:54 p.m.)


Status
--

This change has been marked as submitted.


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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated July 16, 2016, 7:04 a.m.)


Review request for Telepathy.


Repository: ktp-text-ui


Description (updated)
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).
Uses OpenStreetMap as "backend".


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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

> On July 15, 2016, 3:57 p.m., Aleix Pol Gonzalez wrote:
> > filters/geopoint/geopoint-filter.cpp, line 34
> > <https://git.reviewboard.kde.org/r/128460/diff/1/?file=471790#file471790line34>
> >
> > Can we maybe use something that is more free software friendly than 
> > Google Maps?
> 
> Alexandr Akulich wrote:
> As I said, I tried OSM first, but not managed to make it to works in a 
> hour, so I switched to Google Maps for the "prove of concept" at least.
> 
> I would try again to make it works with OSM after 5 August.
> 
> Sandro Knauß wrote:
> not that difficult:
> 
> http://www.openstreetmap.org/?mlat=50.8295=12.9204#map=16/50.8295/12.9204
> 
> mlot/mlat are the positions of the marker and 16 is the zoom level
> 
> Alexandr Akulich wrote:
> Big thank you! This is what I need!
> I'll change it right now then (without configuration support).

It doesn't work in iframe. I had to replace it with

https://wiki.openstreetmap.org/wiki/OpenLinkMap#Embed_map_in_another_website


- Alexandr


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


On July 16, 2016, 6:58 a.m., Alexandr Akulich wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128460/
> ---
> 
> (Updated July 16, 2016, 6:58 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 (RFC 5870).
> 
> 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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated July 16, 2016, 6:58 a.m.)


Review request for Telepathy.


Changes
---

The map changed from GoogleMaps to OpenStreetMap (via OpenLinkMap)


Repository: ktp-text-ui


Description
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).

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 (updated)
-

  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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

> On July 15, 2016, 3:57 p.m., Aleix Pol Gonzalez wrote:
> > filters/geopoint/geopoint-filter.cpp, line 34
> > <https://git.reviewboard.kde.org/r/128460/diff/1/?file=471790#file471790line34>
> >
> > Can we maybe use something that is more free software friendly than 
> > Google Maps?
> 
> Alexandr Akulich wrote:
> As I said, I tried OSM first, but not managed to make it to works in a 
> hour, so I switched to Google Maps for the "prove of concept" at least.
> 
> I would try again to make it works with OSM after 5 August.
> 
> Sandro Knauß wrote:
> not that difficult:
> 
> http://www.openstreetmap.org/?mlat=50.8295=12.9204#map=16/50.8295/12.9204
> 
> mlot/mlat are the positions of the marker and 16 is the zoom level

Big thank you! This is what I need!
I'll change it right now then (without configuration support).


- Alexandr


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


On July 15, 2016, 4:15 p.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, 4:15 p.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 (RFC 5870).
> 
> 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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

> On July 15, 2016, 3:57 p.m., Aleix Pol Gonzalez wrote:
> > I really like the idea!

Thanks!


I have no idea how to implement other multimedia messages with current 
KTp::Message API.
Locally I changed it to pass all MessageParts (currently we have only text), 
and may be I would come with bolder changes next month or two. :)


> On July 15, 2016, 3:57 p.m., Aleix Pol Gonzalez wrote:
> > filters/geopoint/geopoint-filter.cpp, line 34
> > <https://git.reviewboard.kde.org/r/128460/diff/1/?file=471790#file471790line34>
> >
> > Can we maybe use something that is more free software friendly than 
> > Google Maps?

As I said, I tried OSM first, but not managed to make it to works in a hour, so 
I switched to Google Maps for the "prove of concept" at least.

I would try again to make it works with OSM after 5 August.


- Alexandr


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


On July 15, 2016, 4:15 p.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, 4:15 p.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 (RFC 5870).
> 
> 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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated July 15, 2016, 4:15 p.m.)


Review request for Telepathy.


Repository: ktp-text-ui


Description (updated)
---

Added a geopoint filter which adds an iframe with a map for each 
geo:<double,double> in message (RFC 5870).

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 128460: ktp-text-ui: Added a filter for geo uri support

[Alexandr Akulich]

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

(Updated July 15, 2016, 2:18 p.m.)


Review request for Telepathy.


Summary (updated)
-

ktp-text-ui: Added a filter for geo uri support


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

Review Request 128460: Added a filter for geo uri support

[Alexandr Akulich]

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

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

Review Request 128459: ktp-text-ui: Cleanup headers

[Alexandr Akulich]

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

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

OTR Proxy breaks support of multiply clients.

[Alexandr Akulich]

Hi all.

OtrProxyChannel::Adaptee::connectProxy() at [1] line 170 doesn't
connect to messageSent() signal, which makes ktp client to ignore
messages, sent by other clients. This isn't trivial to fix, because if
we connect to the signal, then we should filter out our own emission
at [1] line 466.

The proxy is involved regardless of settings and plugin enablement and
it is especially annoying, because it affects telegram connection
manager (telepathy-morse), which works fine with any other Telepathy
client, such as Empathy, jolla-messages (Sailfish client) and ktp
without the proxy.

[1] 
https://quickgit.kde.org/?p=ktp-common-internals.git=blob=otr-proxy%2FKTpProxy%2Fotr-proxy-channel-adaptee.cpp
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: telepathy-qt-service library fix link errors or make shared?

[Alexandr Akulich]

On Wed, Jan 20, 2016 at 12:13 AM, Diane Trout  wrote:
>
> There was just a comment saying build the service library as a static
> library because the API was still too unstable. That comment could
> easily be wrong.
At the time of comment the API was been incomplete and nonoperable. I
think now (13 days ago with commit
932ad104989e3ee8d30e06d05c975097c3e16097) we reached stability.

> And it is sounding like releasing it as shared library is the way to
> go.
I think so, but we need to polish FileTransfer for the release, so
don't expect it to happen this week.
> Diane
> ___
> KDE-Telepathy mailing list
> KDE-Telepathy@kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: telepathy-qt-service library fix link errors or make shared?

[Alexandr Akulich]

On Mon, Jan 18, 2016 at 5:52 PM, Daniele E. Domenichelli
 wrote:
> Hello,
>
> I don't know what is the cmake minimum required version for telepathy-qt
> at the moment, but if you use the CMAKE_POSITION_INDEPENDENT_CODE[1]
> variable (CMake 2.8.10 or later) or set the POSITION_INDEPENDENT_CODE[2]
> (CMake 2.8.9 or later) property on the target, cmake will automatically
> enable -fPIC on the compilers that support it.
>
Nice catch! TelepathyQt requires CMake-2.8.12 as minimum version.
> Cheers,
>  Daniele
>
>
> [1]https://cmake.org/cmake/help/git-master/variable/CMAKE_POSITION_INDEPENDENT_CODE.html
> [2]https://cmake.org/cmake/help/git-master/prop_tgt/POSITION_INDEPENDENT_CODE.html
> ___
> KDE-Telepathy mailing list
> KDE-Telepathy@kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: telepathy-qt-service library fix link errors or make shared?

[Alexandr Akulich]

Hi.

How do you pass the -fPIC option? Does the patch fix the problem if we
change CMAKE_SHARED_LIBRARY_C_FLAGS to CMAKE_SHARED_LIBRARY_CXX_FLAGS?

I would like to take (push to fd.o) the patch, which fixes fPIC, but
we need to have a proper review and more fix confirmation.

Still, probably it's a time to make telepathy-qt-service to be a shared library.
What's about pushing the two commits to the freedesktop.org repository
(https://github.com/TelepathyQt/telepathy-qt/commit/5eeddedd04c9a4d18c92b4eb5aa494d7abc9a1d5,
https://github.com/TelepathyQt/telepathy-qt/commit/2ff93cd0c0b73e2fc07d9ecbc115460224a1cbd1)?
Sure, I would take care on so versions.

I would like to include this into 0.9.7 release.

On Sat, Jan 16, 2016 at 1:52 AM, Rex Dieter  wrote:
> Diane Trout wrote:
>
>> Hi,
>>
>> Daniel Pocock wrote another telepathy SIP connection manager, and he'd
>> like to use the telepathy-qt-service library.
>>
>> In the cmake file it says "compile as a static library until there's a
>> stable API"
>>
>> However when he tried to link against it he got -fPIC errors
>> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=801817
>
> I used this,
> http://pkgs.fedoraproject.org/cgit/rpms/telepathy-qt.git/tree/telepathy-qt-0.9.5-static_fPIC.patch
>
> (sorry I suck for not working harder to upstream it, though I'm not 100%
> certain that's the right way to fix it either)
>
> -- Rex
>
> ___
> KDE-Telepathy mailing list
> KDE-Telepathy@kde.org
> https://mail.kde.org/mailman/listinfo/kde-telepathy
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Future of Telepathy

[Alexandr Akulich]

On Wed, Dec 30, 2015 at 12:30 PM, Diane Trout  wrote:
> But even if I do get everything reasonably merged, at some point I'm going to
> need opinions from more experienced Telepathy devs about changes to the spec.
> Though theres no need for you to worry about that until I actually have code
> merged that needs Telpathy API changes. (For example message carbons might
> need some changes).

+1 on this.
Despite of my silence at last few months, I will return to work on
TelepathyQt and related projects sometime soon.
We can add "Enable Carbon" option to a ConnectionManager. and it seems
that we need to change the spec only to support "Avoiding Carbons for
a single message".
___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Crash in plasma shell due to bad connection manager

[Alexandr Akulich]

 Hey,

 On Sun, Aug 2, 2015 at 8:24 AM, Diane Trout di...@ghic.org wrote:

 Hi,

 I was trying to develop a connection manager in python, and once had
 enough interfaces implemented for TelepathyQt to try reading contacts I was
 getting ktp-contactlist and plasmashell to crash.



 It looks like TelepathyQt was crashing at at TelepathyQt/contact.cpp:1048
 when it was parsing the mess I was returning for
 oft.Connection.Interface.ContactCapabilities/capabilities

 I suspect the plasma shell crash is because I had the ktp widget in the
 system tray. This is with the Debian builds of 15.04.3 [1] and TelepathyQt
 0.9.6.1

 For reference this is the function that returns the garbage that triggers
 the crash

 https://github.com/detrout/telepathy-gitter/blob/51cba4dacefb7fd10e4d107045f30a5679bd0912/glitter/contacts.py#L140


 I'm not entirely sure about Python and/or the struct you need to return,
 however you can check the Telegram CM which is done in Qt,
 see https://github.com/Kaffeine/telegram-qt/ ...maybe it will help :)

The correct URL is https://github.com/TelepathyQt/telepathy-morse

 Obviously I need to figure out how to properly construct the dbus
 structure (any hints would be appreciated). I was also wondering if causing
 plasmashell to crash because of my own bad code is something that should
 file a plasmashell bugreport for?


 Yeah, probably. Perhaps it should be fixed in tp-qt, even. Just
 file one and paste the backtrace there, we can follow it there.

I experienced a lot of crashes and craziness in KTp and Plasma during
Morse development.
It was too many different issues, so I didn't dive into that. After
all, there is no crashes, if connection manager behaves well. :-)
___
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

[Alexandr Akulich]

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


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.

- Alexandr Akulich


On June 21, 2015, 4: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, 4: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

Re: Review Request 123938: Don't add send delivery messages in the messages vector

[Alexandr Akulich]

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


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


On May 29, 2015, 10: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, 10: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 119880: [frameworks] Port TelepathyHandlerApplication to QApplication

[Alexandr Akulich]

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



KTp/telepathy-handler-application.cpp
https://git.reviewboard.kde.org/r/119880/#comment45402

Nitpick? Do we actually need this QApplication:: namespacing in 
QApplication subclass?

I mean, if in far future we will port this app to QuickControls, here will 
be QGuiApplication, but probably we can drop namespacing at all.


- Alexandr Akulich


On Авг. 21, 2014, 4:19 п.п., Martin Klapetek wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119880/
 ---
 
 (Updated Авг. 21, 2014, 4:19 п.п.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-common-internals
 
 
 Description
 ---
 
 KApplication is deprecated with Frameworks, this ports it to QApplication
 
 This also moves the setenv initHack into the applications instantiating the 
 THA, not sure if this would work but a code like
 
 int main(int argc, char *argv[])
 {
 setenv(KDE_FULL_SESSION, true, 0);
 setenv(KDE_SESSION_VERSION, 5, 0);
 
 KTp::TelepathyHandlerApplication app(argc, argv);   
 }
 
 should also do the trick I believe.
 
 
 Diffs
 -
 
   KTp/telepathy-handler-application.h 504ad52 
   KTp/telepathy-handler-application.cpp 245ff88 
 
 Diff: https://git.reviewboard.kde.org/r/119880/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Klapetek
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 119467: Implement the main UI in QML (Kadixt patches 1/3)

[Alexandr Akulich]

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



src/dtmf-qml.h
https://git.reviewboard.kde.org/r/119467/#comment43940

Module include?!


Excuse me, but do we really want such Quick 1.1 stuff?

- Alexandr Akulich


On Июль 25, 2014, 9:08 п.п., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/119467/
 ---
 
 (Updated Июль 25, 2014, 9:08 п.п.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-call-ui
 
 
 Description
 ---
 
 Implement the main UI in QML (Kadixt patches 1/3)
 
 
 Diffs
 -
 
   CMakeLists.txt 8c39e7c 
   src/CMakeLists.txt 250aeb5 
   src/call-window.h 07fd01e 
   src/call-window.cpp c38112d 
   src/call-window.ui 32c7dad 
   src/dtmf-handler.cpp d8d7970 
   src/dtmf-qml.h PRE-CREATION 
   src/dtmf-qml.cpp PRE-CREATION 
   src/dtmf-widget.h 9e1bc73 
   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 
   src/callwindowui.rc 6c390b9 
   src/dtmf-handler.h 91960dc 
 
 Diff: https://git.reviewboard.kde.org/r/119467/diff/
 
 
 Testing
 ---
 
 Rebased, patched and cherry-picked until I was red in the face.
 
 Compiles, not tested beyond that.
 
 
 Thanks,
 
 David Edmundson
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 117968: Map connection presence type to string

[Alexandr Akulich]

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

Ship it!


Looking good. I have just one question.


kpeople/datasourceplugin/im-persons-data-source.cpp
https://git.reviewboard.kde.org/r/117968/#comment39873

What is KABC standing for? It is a different product, isn't it?


- Alexandr Akulich


On May 3, 2014, 5:59 a.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/117968/
 ---
 
 (Updated May 3, 2014, 5:59 a.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-common-internals
 
 
 Description
 ---
 
 Two commits:
 
 
 Share the same QString for all KABC Custom fields
 
 We should have a theoretically smaller memory footprint.
 
 
 
 
 Map connection presence type to string
 
 In libkpeople we pass the presence around as a string as that's all
 KABC::custom() supports.
 
 We used to use presence-status(); this is problematic as it is free
 text which can contain anything; especially when on multiple protocols.
 
 A presence might have the status chat which is mapped to the presence
 type Tp::ConnectionPresenceTypeOnline. We want to use this mapping and
 only have a subset of presence strings.
 
 BUG: 334207
 
 
 Diffs
 -
 
   kpeople/datasourceplugin/im-persons-data-source.cpp 44e2d5c 
 
 Diff: https://git.reviewboard.kde.org/r/117968/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116706: ktp-common-internals: KPeople plugin: Implemented groups cache support.

[Alexandr Akulich]

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

(Updated April 10, 2014, 5:26 p.m.)


Review request for Telepathy.


Changes
---

Updated to emit emitInitialFetchComplete properly.


Bugs: 331272
http://bugs.kde.org/show_bug.cgi?id=331272


Repository: ktp-common-internals


Description
---

Implemented groups cache support.


Diffs (updated)
-

  kpeople/datasourceplugin/im-persons-data-source.cpp e1261ab 

Diff: https://git.reviewboard.kde.org/r/116706/diff/


Testing
---

Works as expected, but ktp-kded-module patch needed.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116706: ktp-common-internals: KPeople plugin: Implemented groups cache support.

[Alexandr Akulich]

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

(Updated April 10, 2014, 6:24 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Bugs: 331272
http://bugs.kde.org/show_bug.cgi?id=331272


Repository: ktp-common-internals


Description
---

Implemented groups cache support.


Diffs
-

  kpeople/datasourceplugin/im-persons-data-source.cpp e1261ab 

Diff: https://git.reviewboard.kde.org/r/116706/diff/


Testing
---

Works as expected, but ktp-kded-module patch needed.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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

(Updated April 3, 2014, 11:35 a.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Repository: ktp-kded-module


Description
---

Implemented in most simple and short way. I hope, there is no significant flaws.


Diffs
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116707/diff/


Testing
---

Working good.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116706: ktp-common-internals: KPeople plugin: Implemented groups cache support.

[Alexandr Akulich]

 On March 24, 2014, 5:31 p.m., David Edmundson wrote:
  /home/david/projects/kde4/src/ktp-common-internals/kpeople/datasourceplugin/im-persons-data-source.cpp:124:33:
   error: use of undeclared identifier 'contactGroups'
  addressee.setCategories(contactGroups);
  ^
  /home/david/projects/kde4/src/ktp-common-internals/kpeople/datasourceplugin/im-persons-data-source.cpp:117:48:
   warning: comparison of integers of different signs: 'uint' (aka 'unsigned 
  int') and 'int' [-Wsign-compare]
  if ((!convSuccess) || (groupId = groupsList.count()))
 ~~~ ^  ~~
  1 warning and 1 error generated.
  kpeople/datasourceplugin/CMakeFiles/im_persons_data_source_plugin.dir/build.make:80:
   recipe for target 
  'kpeople/datasourceplugin/CMakeFiles/im_persons_data_source_plugin.dir/im-persons-data-source.cpp.o'
   failed
  make[2]: *** 
  [kpeople/datasourceplugin/CMakeFiles/im_persons_data_source_plugin.dir/im-persons-data-source.cpp.o]
   Error 1
  CMakeFiles/Makefile2:224: recipe for target 
  'kpeople/datasourceplugin/CMakeFiles/im_persons_data_source_plugin.dir/all' 
  failed
 

Excuse me! I has playing with QtCreator thus much, so needed to remove profile. 
After it, I forget to setup some autosave function.


- Alexandr


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


On March 14, 2014, 4:56 p.m., Alexandr Akulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116706/
 ---
 
 (Updated March 14, 2014, 4:56 p.m.)
 
 
 Review request for Telepathy.
 
 
 Bugs: 331272
 http://bugs.kde.org/show_bug.cgi?id=331272
 
 
 Repository: ktp-common-internals
 
 
 Description
 ---
 
 Implemented groups cache support.
 
 
 Diffs
 -
 
   kpeople/datasourceplugin/im-persons-data-source.cpp 94d8de1 
 
 Diff: https://git.reviewboard.kde.org/r/116706/diff/
 
 
 Testing
 ---
 
 Works as expected, but ktp-kded-module patch needed.
 
 
 Thanks,
 
 Alexandr Akulich
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116706: ktp-common-internals: KPeople plugin: Implemented groups cache support.

[Alexandr Akulich]

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

(Updated March 24, 2014, 7 p.m.)


Review request for Telepathy.


Changes
---

Fixed broken code. Sorry.


Bugs: 331272
http://bugs.kde.org/show_bug.cgi?id=331272


Repository: ktp-common-internals


Description
---

Implemented groups cache support.


Diffs (updated)
-

  kpeople/datasourceplugin/im-persons-data-source.cpp 94d8de1 

Diff: https://git.reviewboard.kde.org/r/116706/diff/


Testing
---

Works as expected, but ktp-kded-module patch needed.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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

(Updated March 14, 2014, 4:35 p.m.)


Review request for Telepathy.


Changes
---

Implemented old version table detection.
Fixed groupName column type.


Repository: ktp-kded-module


Description
---

Implemented in most simple and short way. I hope, there is no significant flaws.


Diffs (updated)
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116707/diff/


Testing
---

Working good.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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

(Updated March 14, 2014, 4:35 p.m.)


Review request for Telepathy.


Changes
---

Branch changed to last stable.


Repository: ktp-kded-module


Description
---

Implemented in most simple and short way. I hope, there is no significant flaws.


Diffs
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116707/diff/


Testing
---

Working good.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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



contact-cache.cpp
https://git.reviewboard.kde.org/r/116707/#comment37253

I'm not sure in this line, but it works for me.


- Alexandr Akulich


On March 14, 2014, 4:35 p.m., Alexandr Akulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116707/
 ---
 
 (Updated March 14, 2014, 4:35 p.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-kded-module
 
 
 Description
 ---
 
 Implemented in most simple and short way. I hope, there is no significant 
 flaws.
 
 
 Diffs
 -
 
   contact-cache.h 8fa6fac 
   contact-cache.cpp 47fe0cd 
 
 Diff: https://git.reviewboard.kde.org/r/116707/diff/
 
 
 Testing
 ---
 
 Working good.
 
 
 Thanks,
 
 Alexandr Akulich
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116706: ktp-common-internals: KPeople plugin: Implemented groups cache support.

[Alexandr Akulich]

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

(Updated March 14, 2014, 4:56 p.m.)


Review request for Telepathy.


Changes
---

Implemented handling of case, when there is no groups cached.
Reassigned to last stable branch.

Added bug number.


Bugs: 331272
http://bugs.kde.org/show_bug.cgi?id=331272


Repository: ktp-common-internals


Description
---

Implemented groups cache support.


Diffs (updated)
-

  kpeople/datasourceplugin/im-persons-data-source.cpp 94d8de1 

Diff: https://git.reviewboard.kde.org/r/116706/diff/


Testing
---

Works as expected, but ktp-kded-module patch needed.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

 On March 12, 2014, 8:02 p.m., David Edmundson wrote:
  Can I make sure I'm understand this correctly:
  
  Lets pretend I have contactA in groups KDE and friends and contactB is 
  in the group friends. The databases will be as follows
  
  Contacts:
  contactA 1,2
  contactB 1
  
  Groups:
  friends
  KDE
  
  
 
 
 David Edmundson wrote:
 *Can I make sure I'm understanding this correctly

Yes, you're almost right. It will looks like follow:
sqlite select * from contacts;
account|contactA|NameA||0,1
account|contactB|NameB||1
sqlite select * from groups;
0|friends
1|KDE
sqlite


- Alexandr


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


On March 11, 2014, 3:19 p.m., Alexandr Akulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116707/
 ---
 
 (Updated March 11, 2014, 3:19 p.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-kded-module
 
 
 Description
 ---
 
 Implemented in most simple and short way. I hope, there is no significant 
 flaws.
 
 
 Diffs
 -
 
   contact-cache.h 8fa6fac 
   contact-cache.cpp 47fe0cd 
 
 Diff: https://git.reviewboard.kde.org/r/116707/diff/
 
 
 Testing
 ---
 
 Working good.
 
 
 Thanks,
 
 Alexandr Akulich
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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



contact-cache.cpp
https://git.reviewboard.kde.org/r/116707/#comment37224

Somehow there is missed type of groupName column. Query should be
CREATE TABLE groups (groupId INTEGER, groupName VARCHAR);


- Alexandr Akulich


On March 11, 2014, 3:19 p.m., Alexandr Akulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116707/
 ---
 
 (Updated March 11, 2014, 3:19 p.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-kded-module
 
 
 Description
 ---
 
 Implemented in most simple and short way. I hope, there is no significant 
 flaws.
 
 
 Diffs
 -
 
   contact-cache.h 8fa6fac 
   contact-cache.cpp 47fe0cd 
 
 Diff: https://git.reviewboard.kde.org/r/116707/diff/
 
 
 Testing
 ---
 
 Working good.
 
 
 Thanks,
 
 Alexandr Akulich
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116709: ktp-common-internals: Core: Tp::Connection::FeatureConnected added to ConnectionFactory options.

[Alexandr Akulich]

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

(Updated March 12, 2014, 5:12 p.m.)


Status
--

This change has been marked as submitted.


Review request for Telepathy.


Repository: ktp-common-internals


Description
---

This patch fixes caching on onlineness changes.


Diffs
-

  KTp/core.cpp 1898dc1 

Diff: https://git.reviewboard.kde.org/r/116709/diff/


Testing
---

Works perfectly.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Review Request 116704: ktp-kded-module: Fixed caching.

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-kded-module


Description
---

This patch fixes (for me) caching on onlineness changes.
Without this patch caching happens only if accounts already online before 
module execution, which is usually not true. (was stucked on 
onAccountConnectionChanged)


Diffs
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116704/diff/


Testing
---

After patch caching work just as planned.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116704: ktp-kded-module: Fixed caching.

[Alexandr Akulich]

 On March 11, 2014, 2:48 p.m., David Edmundson wrote:
  Ship it if you want, but I think there's an easier way.
  
  IMHO it would be easier to use
  connection-becomeReady(Connection::FeatureConnection) because then we can 
  combine it with the becomeReady(roster) loading that's also here and drop 
  the number of async things we're watching. 
  
  Or we can probably add FeatureConnection to the default factories.
  
 

Where I can add it? I have tried to change line 148 to 
connection-becomeReady(Tp::Features()  Tp::Connection::FeatureRoster  
Tp::Connection::FeatureConnected);, but it doesn't fix issue.


- Alexandr


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


On March 11, 2014, 2:43 p.m., Alexandr Akulich wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116704/
 ---
 
 (Updated March 11, 2014, 2:43 p.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-kded-module
 
 
 Description
 ---
 
 This patch fixes (for me) caching on onlineness changes.
 Without this patch caching happens only if accounts already online before 
 module execution, which is usually not true. (was stucked on 
 onAccountConnectionChanged)
 
 
 Diffs
 -
 
   contact-cache.h 8fa6fac 
   contact-cache.cpp 47fe0cd 
 
 Diff: https://git.reviewboard.kde.org/r/116704/diff/
 
 
 Testing
 ---
 
 After patch caching work just as planned.
 
 
 Thanks,
 
 Alexandr Akulich
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Review Request 116707: KTp KDED Module: Implemented groups caching.

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-kded-module


Description
---

Implemented in most simple and short way. I hope, there is no significant flaws.


Diffs
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116707/diff/


Testing
---

Working good.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116704: ktp-kded-module: Fixed caching.

[Alexandr Akulich]

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

(Updated March 11, 2014, 4:04 p.m.)


Status
--

This change has been discarded.


Review request for Telepathy.


Repository: ktp-kded-module


Description
---

This patch fixes (for me) caching on onlineness changes.
Without this patch caching happens only if accounts already online before 
module execution, which is usually not true. (was stucked on 
onAccountConnectionChanged)


Diffs
-

  contact-cache.h 8fa6fac 
  contact-cache.cpp 47fe0cd 

Diff: https://git.reviewboard.kde.org/r/116704/diff/


Testing
---

After patch caching work just as planned.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Review Request 116709: ktp-common-internals: Core: Tp::Connection::FeatureConnected added to ConnectionFactory options.

[Alexandr Akulich]

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

Review request for Telepathy.


Repository: ktp-common-internals


Description
---

This patch fixes caching on onlineness changes.


Diffs
-

  KTp/core.cpp 1898dc1 

Diff: https://git.reviewboard.kde.org/r/116709/diff/


Testing
---

Works perfectly.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116560: Adds the feature to save the log files in the debugger.

[Alexandr Akulich]

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



tools/debugger/main-window.cpp
https://git.reviewboard.kde.org/r/116560/#comment36829

I propose to rewrite it as:
switch (m_ui.tabWidget-currentWidget()) {
  case m_ui.mcTab:
m_ui.mcLogsView-saveLogFile();
break;
  case m_ui.gabbleTab:
m_ui.gabbleLogsView-saveLogFile();
break;
…
}
Such change will make method independent on tabs order.

At very least, you should add space before open brace.
I'm not sure that placing couple of instructions on same line is acceptable.


- Alexandr Akulich


On March 3, 2014, 5:16 p.m., mayank jha wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116560/
 ---
 
 (Updated March 3, 2014, 5:16 p.m.)
 
 
 Review request for Telepathy.
 
 
 Bugs: 303564
 http://bugs.kde.org/show_bug.cgi?id=303564
 
 
 Repository: ktp-common-internals
 
 
 Description
 ---
 
 Adds a button Save Log file to save the log in the currentTab of the 
 TabWidget.
 
 
 Diffs
 -
 
   tools/debugger/debug-message-view.h 49a4b2f 
   tools/debugger/debug-message-view.cpp c2ead13 
   tools/debugger/main-window.h 3876767 
   tools/debugger/main-window.cpp faf7c22 
   tools/debugger/main-window.ui 0813149 
 
 Diff: https://git.reviewboard.kde.org/r/116560/diff/
 
 
 Testing
 ---
 
 Runs fine!
 
 
 File Attachments
 
 
 The Button added
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/03/03/e4d604cc-f67a-482a-9c81-0de3cbf3f2ea__savebutton.png
 The File Dialog Box
   
 https://git.reviewboard.kde.org/media/uploaded/files/2014/03/03/b6bb5033-dae3-4edc-bd60-f6428d9cfeda__savedialog.png
 
 
 Thanks,
 
 mayank jha
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116520: Renames the label of the File Dialog Button from Open to Send

[Alexandr Akulich]

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

Ship it!


Seems like you run astyle script only on sendfiledialog.cpp file.
Good job, though :).


sendfiledialog.h
https://git.reviewboard.kde.org/r/116520/#comment36718

I think that this should be in sendfiledialog.cpp.



sendfiledialog.h
https://git.reviewboard.kde.org/r/116520/#comment36727

Spacing is still wrong.
* Add space between include directive and included file.
* Add space before colon in class declaration.
* Remove space after ampersand at startDir argument.



sendfiledialog.cpp
https://git.reviewboard.kde.org/r/116520/#comment36731

You have long line (29), so why not move parent argument to previous line?
Colon should be moved to lines with method signature as well.
Just look at MainWindow constructor style.


Please, fix issues and ship it.

- Alexandr Akulich


On March 2, 2014, 1:15 p.m., mayank jha wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116520/
 ---
 
 (Updated March 2, 2014, 1:15 p.m.)
 
 
 Review request for Telepathy.
 
 
 Bugs: 331533
 http://bugs.kde.org/show_bug.cgi?id=331533
 
 
 Repository: ktp-send-file
 
 
 Description
 ---
 
 Creates a new widget which renames the Open button to Send.
 
 
 Diffs
 -
 
   CMakeLists.txt 04ded76 
   main.cpp a35c4e1 
   sendfiledialog.h PRE-CREATION 
   sendfiledialog.cpp PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/116520/diff/
 
 
 Testing
 ---
 
 Runs fine!
 
 
 Thanks,
 
 mayank jha
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116033: Also show overlays on metacontacts

[Alexandr Akulich]

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

Ship it!


Ship It!

- Alexandr Akulich


On Feb. 25, 2014, 4:48 a.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116033/
 ---
 
 (Updated Feb. 25, 2014, 4:48 a.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-contact-list
 
 
 Description
 ---
 
 Also show overlays on metacontacts
 
 
 Diffs
 -
 
   contact-overlays.cpp 2e663f3 
 
 Diff: https://git.reviewboard.kde.org/r/116033/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Re: Review Request 116033: Also show overlays on metacontacts

[Alexandr Akulich]

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



contact-overlays.cpp
https://git.reviewboard.kde.org/r/116033/#comment35798

There is extra space:   (.


- Alexandr Akulich


On Feb. 25, 2014, 4:48 a.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/116033/
 ---
 
 (Updated Feb. 25, 2014, 4:48 a.m.)
 
 
 Review request for Telepathy.
 
 
 Repository: ktp-contact-list
 
 
 Description
 ---
 
 Also show overlays on metacontacts
 
 
 Diffs
 -
 
   contact-overlays.cpp 2e663f3 
 
 Diff: https://git.reviewboard.kde.org/r/116033/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy

Review Request 116115: ktp-contact-list: Set application icon globally in aboutData, instead of in MainWidget only.

[Alexandr Akulich]

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

Review request for Telepathy.


Bugs: 331491
http://bugs.kde.org/show_bug.cgi?id=331491


Repository: ktp-contact-list


Description
---

We have set special program icon for main widget, but help menu and possible 
other places uses KAboutData information, so try to set icon = appName.


Diffs
-

  main-widget.cpp e4a9cda 
  main.cpp 65801df 

Diff: https://git.reviewboard.kde.org/r/116115/diff/


Testing
---

All just fine. MainWidget and help menu have proper icons.


Thanks,

Alexandr Akulich

___
KDE-Telepathy mailing list
KDE-Telepathy@kde.org
https://mail.kde.org/mailman/listinfo/kde-telepathy