[Kde-pim] Review Request: kmail - please add ability to display and sort by date received

[Thomas McGuire]

> On 2010-07-06 13:35:37, Thomas McGuire wrote:
> > Hi Eric,
> > 
> > sorry for the late reply, I have been too busy with university.
> > 
> > I think the patch and the rationale behind it are fine, after reading your explanation and thinking about it a bit more. The good thing about the patch is that it doesn't affect users by default, it has to explicitly enabled in the UI and is therefore not intrusive. So after making some improvements to the patch, I would add it to KMail and KMime.
> > 
> > The quality of the patch is also good, well done.
> > 
> > The KMime part of the patch needs some improvements, code in public libraries like KMime should have high standards. I think the biggest problem is that the code in some places assumes that there is only one Received header, although in reality mails have multiple Received headers. The other thing that needs some attention is the parsing code, it should be a real parse(), and at the same time be based on Structured instead of Unstructured.
> > To make sure that parsing and assembling of mails with Received header_s_ works, please add some unit tests for that to KMime. You need to enable KDE4_BUILD_TESTS in CMake for that.
> > 
> > If you have any questions about all that, please contact, I shall answer faster the next time. Awaiting your next patch now! :)
> > 
> > I added some comments inline below as well, please look at them.

Oh, and another thing: Please make sure your patch doesn't introduce trailing whitespace. Reviewboard will show you those and color them in red.


- Thomas


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


On 2010-06-16 01:03:47, Eric Sanford wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/4337/
> -----------------------------------------------------------
> 
> (Updated 2010-06-16 01:03:47)
> 
> 
> Review request for KDE PIM and Thomas McGuire.
> 
> 
> Summary
> -------
> 
> KMail should have the ability to extract the date from the Received header field, display the date received and sort by date received. A significant number of emails have incorrect or missing Date header fields. When sorting by Date in KMail, these drop to the bottom as Unknown or appear at the top if the Date header field contains a future date. This is a basic patch to extract the date from the Received header field, add a Date Received column in KMail classic theme, and sort by Date Received in classic theme. This is industry-standard for comparable email applications.
> 
> 
> This addresses bug 227942.
>     https://bugs.kde.org/show_bug.cgi?id=227942
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdepim/messagelist/core/item.h 1137450 
>   trunk/KDE/kdepim/messagelist/core/item.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/item_p.h 1137450 
>   trunk/KDE/kdepim/messagelist/core/manager.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/model.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/sortorder.h 1137450 
>   trunk/KDE/kdepim/messagelist/core/sortorder.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/theme.h 1137450 
>   trunk/KDE/kdepim/messagelist/core/theme.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/themedelegate.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/core/view.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/storagemodel.cpp 1137450 
>   trunk/KDE/kdepim/messagelist/utils/themeeditor.cpp 1137450 
>   trunk/KDE/kdepim/nepomuk_email_feeder/messageanalyzer.cpp 1137450 
>   trunk/KDE/kdepimlibs/kmime/kmime_headers.h 1137426 
>   trunk/KDE/kdepimlibs/kmime/kmime_headers.cpp 1137426 
>   trunk/KDE/kdepimlibs/kmime/kmime_headers_p.h 1137426 
>   trunk/KDE/kdepimlibs/kmime/kmime_message.h 1137426 
>   trunk/KDE/kdepimlibs/kmime/kmime_message.cpp 1137426 
> 
> Diff: http://reviewboard.kde.org/r/4337/diff
> 
> 
> Testing
> -------
> 
> This patch has been tested on openSUSE 11.2 and openSUSE 11.3 Milestone 7. The attached patch is currently being tested on openSUSE 11.3 Milestone 7, with KDE executables and libraries built from trunk/KDE (revision 1137426) source code for kdelibs, kdepimlibs, kdebase and kdepim, and trunk/kdesupport source code for attica, soprano, polkit-qt, and akonadi.
> 
> 
> Thanks,
> 
> Eric
> 
>

_______________________________________________
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/