WIP: Interface for mail headers in MimeTreeParser
ClosedPublic

Authored by knauss on Sep 26 2018, 7:37 AM.

Details

Summary

To make it possible to display memoryhole correcly to the user,
MimeTreeParser needs to have the ability to update/change headers.
For this we need an interface to change the headers.

Diff Detail

Repository
R94 PIM: Message Library
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
knauss created this revision.Sep 26 2018, 7:37 AM
Restricted Application added a project: KDE PIM. · View Herald TranscriptSep 26 2018, 7:37 AM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
knauss requested review of this revision.Sep 26 2018, 7:37 AM
knauss updated this revision to Diff 42348.Sep 26 2018, 7:39 AM

remove parts that shuld not have got to the review request.

knauss updated this revision to Diff 42349.Sep 26 2018, 7:45 AM

now realy remove the part I don't want the RR.

@vkrause: Unfortunately I forgotten what we discussed at Akademy what a better interface would look like. That's why I started now to implement that one. I think we thought at something like a "HeaderObject":

const auto headerObject = nodeHelper->headerObject(message);
headerObject->hasHeader("to");
headerObject->header("simple"); /return Base
headerObject->asAddressList("to");
headerObject->date();
vkrause added inline comments.Oct 3 2018, 8:05 AM
mimetreeparser/src/nodehelper.cpp
526

where is this deleted again?

529

This encodes and re-parses the header, is there no way to avoid that?

knauss added a comment.Oct 3 2018, 1:12 PM

And what about the idea of a "HeaderObject" instead of methos inside nodehelper?

mimetreeparser/src/nodehelper.cpp
526

Good point ;D I haven't thought about it - but yes we have to pass QSharedPointer around.

529

I don't understand the magic inside KMime not that deeply about all these headers and classes and subclasses. I just copied that part from messageviewer/src/header/headerstyle_util.cpp
If you have any ideas how to improve this...

vkrause: ping

vkrause added inline comments.Oct 20 2018, 8:45 AM
mimetreeparser/src/nodehelper.cpp
529

Well, first of all most of those headers are already of type AddressList, so those should be usable directly. For the one exception (From IIRC) the question is whether we need to convert that or if the consumer could handle that as well. Alternatively it might be worth looking if whatever the consumer expects can be added to From's type in KMime directly.

The KMime header class hierarchy pretty much follows the MIME RFCs one to one and uses the same names as defined in the standard.

knauss added inline comments.Oct 21 2018, 6:57 PM
messageviewer/src/header/grantleeheaderformatter.cpp
305

should be Resent-To :D

mimetreeparser/src/nodehelper.cpp
529

we we could enable the shortcuts, but we have more headers that are expected as AddressList: Form. Resent-To, Resent-To, List-Id
I think it the correct place where we do the conversion, because otherwise we would need to move this to the viewer code and the viewer should just ask: I want this header as AddressList and not to the conversion on it's own.
Or where you would do this conversion?

vkrause added inline comments.Oct 27 2018, 10:22 AM
mimetreeparser/src/nodehelper.cpp
529

Well, do we need the full header object or is this ultimately only used for getting the list of addresses in that header? If so, how about we just get that and pass it along instead of the header object itself? If necessary by adding extra getters to KMime.

knauss added inline comments.May 28 2019, 10:24 AM
mimetreeparser/src/nodehelper.cpp
529

Yes but it would make sense to have the name and email addresses already splitted...

This revision was not accepted when it landed; it landed in state Needs Review.Aug 22 2019, 10:37 PM
This revision was automatically updated to reflect the committed changes.