Review Request: Image Plugin

[David Edmundson]

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



filters/images/images-filter.cpp
<http://git.reviewboard.kde.org/r/105313/#comment11727>

    
    width -> max-width.
    
    If I send a 30px image, I don't want it scaled up to fill my window.
    
    Pretty sure we had this in an earlier version.



filters/images/images-filter.cpp
<http://git.reviewboard.kde.org/r/105313/#comment11726>

    is that alt text really explaining anything?
    
    If you had a screen reader would it help?
    Also it's  user presented string, therefore needs i18n().
    
    



lib/message-processor.cpp
<http://git.reviewboard.kde.org/r/105313/#comment11728>

    what is this?
    
    you load all the plugins in any order, then you prepend the UrlFilter, then you loop through the list, find the escape filter and move that to the front....?
    
    You need to sort them when you load, maybe by some value in the .desktop file.
    
    Also is UrlFilter now in the filter twice?
    
    



tests/message-processor-basic-tests.cpp
<http://git.reviewboard.kde.org/r/105313/#comment11729>

    minor comment
    
    striek -> strike


- David Edmundson


On June 21, 2012, 4:43 a.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105313/
> -----------------------------------------------------------
> 
> (Updated June 21, 2012, 4:43 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> This is a plugin to Embed Images directly into a chat. (I pretty much rewrote the old code from scratch because I realized I wasn't very happy with it).
> 
> I've also bumped the version number of all the plugins. 
> 
> This diff depends on diff of the previous review (which I still haven't merged yet). I'll merge them both once this gets reviewed. 
> 
> 
> Diffs
> -----
> 
>   filters/images/ktptextui_message_filter_images.desktop PRE-CREATION 
>   lib/message-processor.cpp a9b409e 
>   filters/images/images-filter.cpp PRE-CREATION 
>   filters/escape/ktptextui_message_filter_escape.desktop PRE-CREATION 
>   filters/formatting/format-filter.cpp PRE-CREATION 
>   filters/formatting/ktptextui_message_filter_formatting.desktop PRE-CREATION 
>   filters/images/CMakeLists.txt PRE-CREATION 
>   filters/images/images-filter.h PRE-CREATION 
>   filters/CMakeLists.txt PRE-CREATION 
>   filters/emoticons/ktptextui_message_filter_emoticons.desktop PRE-CREATION 
>   tests/message-processor-basic-tests.h PRE-CREATION 
>   tests/message-processor-basic-tests.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/105313/diff/
> 
> 
> Testing
> -------
> 
> Passed Unit tests, saw memes in chat. 
> 
> 
> Thanks,
> 
> Lasath Fernando
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20120621/edea926f/attachment-0001.html>