[Kde-pim] Review Request: Add an 'Add tag' filter action

[Thomas McGuire]

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


Ah, now I understand your question about the kmcommand constructors on the ML.
Quite frankly, I have no idea why the set status command rolls its own method of getting the messages (via the sernum list) instead of relying on the stuff that the kmcommand base class already provides. Those methods of the kmcommand base class also allow to pass a kmmsgbase object, which maybe could be used in the filter action then. (Indeed, messages that are arriving have not been added to any folder yet, and therefore don't have a serial number yet). I don't know how well it would work to use a KMCommand inside a filter action, which has to be synchronous (filter framework sucks), but the commands are async by design. So this would probably not work together too well anyway.

All in all, I think this can be left as it is in the patch, unless someone like Ingo has a better idea.


/trunk/KDE/kdepim/kmail/kmcommands.h
<http://reviewboard.kde.org/r/798/#comment792>

    I'd prefer to see an enum with two self-describing values here, makes the code easier to read IMHO.



/trunk/KDE/kdepim/kmail/kmfilteraction.cpp
<http://reviewboard.kde.org/r/798/#comment795>

    use QStringList



/trunk/KDE/kdepim/kmail/kmfilteraction.cpp
<http://reviewboard.kde.org/r/798/#comment798>

    This should be const, I guess. (you can also move that to the foreach line if you want)



/trunk/KDE/kdepim/kmail/kmfilteraction.cpp
<http://reviewboard.kde.org/r/798/#comment800>

    this and the int below can be const



/trunk/KDE/kdepim/kmail/kmfilteraction.cpp
<http://reviewboard.kde.org/r/798/#comment799>

    merge that with the foreach:
    foreach( const QString &tag, mParam...)



/trunk/KDE/kdepim/kmail/kmfilteraction.cpp
<http://reviewboard.kde.org/r/798/#comment797>

    This is a new feature anyway, so we have to wait until all freezes are lifted, so you can add the real string here now.


- Thomas


On 2009-06-07 13:50:03, Jonathan Armond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/798/
> -----------------------------------------------------------
> 
> (Updated 2009-06-07 13:50:03)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Add a filter action for adding tags to messages. Create a KMCommand for adding/toggling tags. I had planned to use the KMCommand for the filter action but it seems newly arriving messages don't have a serial number so I couldn't find a clean way to do it.
> 
> 
> This addresses bug 171204.
>     https://bugs.kde.org/show_bug.cgi?id=171204
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/kmcommands.h 978277 
>   /trunk/KDE/kdepim/kmail/kmcommands.cpp 978277 
>   /trunk/KDE/kdepim/kmail/kmfilteraction.cpp 978277 
>   /trunk/KDE/kdepim/kmail/kmmainwidget.cpp 978277 
> 
> Diff: http://reviewboard.kde.org/r/798/diff
> 
> 
> Testing
> -------
> 
> Newly arrived filtering works. Manual filtering works.
> 
> 
> Thanks,
> 
> Jonathan
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/