[Kde-pim] Review Request: Apply template when switching identity in message composer

[Thomas McGuire]

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


Seems very good overall.
When replying with a global reply template (as configured in Settings->Composer->Custom Templates, switching the identity does not update the template, which is correct, as the user explicitly used a global template :)

However, I noticed a bug, not sure if it was also before your changes or not:
0. Your default identity needs a signature, signature appending needs to be enabled
1. Reply to a message with global template
2. Switch the identity with the identity combobox to antother identity with signature enabled
    Result: The old signature is not replaced, now you have two signatures

Another bug:
When I reply only to a part of the message, by selecting the message text in the reader and then hitten "r", this selection is lost when switching identities. Not sure how to solve this elegantly (I guess we can't simply remove the reply-to-selected-text feature, probably many people using it)


/trunk/KDE/kdepim/kmail/composer.h
<http://reviewboard.kde.org/r/833/#comment852>

    spaces around operators, please



/trunk/KDE/kdepim/kmail/kmcomposewin.h
<http://reviewboard.kde.org/r/833/#comment853>

    spaces around operators (also below)



/trunk/KDE/kdepim/kmail/kmcomposewin.cpp
<http://reviewboard.kde.org/r/833/#comment856>

    can be const I think



/trunk/KDE/kdepim/kmail/kmcomposewin.cpp
<http://reviewboard.kde.org/r/833/#comment854>

    I was just about to complain that this doesn't work with online IMAP, but then I tested it, and it works.
    
    So nothing wrong here, I just wonder _why_ it works, getMsg() does AFAIK only return the headers for online IMAP.



/trunk/KDE/kdepim/kmail/templateparser.h
<http://reviewboard.kde.org/r/833/#comment855>

    Hungarian notation a la aFoo is strange, but then you only copied that from other strange code, so that is ok.


- Thomas


On 2009-06-15 09:19:20, Jonathan Armond wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/833/
> -----------------------------------------------------------
> 
> (Updated 2009-06-15 09:19:20)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> Apply new template when switching identity in the message composer, as long the message has not been modified. NB: changing identity no longer marks message as modified.
> 
> Apply template to Forward As Attachment messages.
> 
> First attempt at fixing bug 143199.
> 
> 
> This addresses bugs 143199, 179754 and 186843.
>     https://bugs.kde.org/show_bug.cgi?id=143199
>     https://bugs.kde.org/show_bug.cgi?id=179754
>     https://bugs.kde.org/show_bug.cgi?id=186843
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/kmail/composer.h 982275 
>   /trunk/KDE/kdepim/kmail/kmcommands.cpp 982275 
>   /trunk/KDE/kdepim/kmail/kmcomposewin.h 982275 
>   /trunk/KDE/kdepim/kmail/kmcomposewin.cpp 982275 
>   /trunk/KDE/kdepim/kmail/kmkernel.cpp 982275 
>   /trunk/KDE/kdepim/kmail/kmmainwidget.cpp 982275 
>   /trunk/KDE/kdepim/kmail/templateparser.h 982275 
>   /trunk/KDE/kdepim/kmail/templateparser.cpp 982275 
> 
> Diff: http://reviewboard.kde.org/r/833/diff
> 
> 
> Testing
> -------
> 
> Template properly parsed for new, reply, reply all and forward. Switches between custom, global and no templates properly. Doesn't switch after modifying message.
> 
> 
> 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/