Review Request: Allow plugins to execute scripts after appending a message

Lasath Fernando kde at lasath.org
Tue Sep 4 15:59:07 UTC 2012


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


Seems like a good patch, I'm impressed at how consistent you've been with the rest of the code. (Even with some of my unusual conventions which I don't know what state of mind I was in when I came up with).

However, I have one (rather serious) question: what's the point of this?


lib/message.h
<http://git.reviewboard.kde.org/r/106301/#comment14610>

    "that will be executed from the main view."
    Technically, that shouldn't be part of the documentation for this method.



lib/message.cpp
<http://git.reviewboard.kde.org/r/106301/#comment14611>

    QStringBuilder should be more appropriate, for performance.
    
    Actually, since m_scripts is a QStringList, you could just as easily go
    m_scripts.removeDuplicates();
    finalScript = m_scripts.join("");


- Lasath Fernando


On Sept. 1, 2012, 9:18 a.m., Daniele Elmo Domenichelli wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106301/
> -----------------------------------------------------------
> 
> (Updated Sept. 1, 2012, 9:18 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Add a script property to AdiumThemeMessageInfo to be run after appending the message
> Add a string list to Message enable plugins to append scripts
> Set the script to the AdiumThemeMessageInfo
> Execute the script after appending a message
> 
> 
> This addresses bug 306020.
>     http://bugs.kde.org/show_bug.cgi?id=306020
> 
> 
> Diffs
> -----
> 
>   lib/adium-theme-message-info.h 7b7587ecb7e418258cc6154587dff43f22a86f7b 
>   lib/adium-theme-message-info.cpp e32910f0a165ec66df73e10c5dfcf6fdfc26a9a0 
>   lib/adium-theme-view.h c3ba08946f081bb054198d4eaeefe19223e1fb3e 
>   lib/adium-theme-view.cpp df2b6d7677176a1385ce1f7eca490766cea7a7b3 
>   lib/chat-widget.cpp 6c9c7ed7f39f0b9f9ee8fbb49d9fa27d2b6b0857 
>   lib/logmanager.cpp ac311a20234feb6be961aa4617fe63fa5979f987 
>   lib/message.h 74321300c9fed3491924de96ffa40eb7e7b9afd6 
>   lib/message.cpp 74db8e166f710ae37a2e88e057b8bd09d546e634 
> 
> Diff: http://git.reviewboard.kde.org/r/106301/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Daniele Elmo Domenichelli
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120904/0308dc4c/attachment.html>


More information about the KDE-Telepathy mailing list