project_reviewers: KDE_PIM
Details
Diff Detail
- Lint
No Linters Available - Unit
No Unit Test Coverage
messageviewer/src/htmlwriter/queuehtmlwriter.h | ||
---|---|---|
30 | Copyright suggests this is entirely new code, the obsolete Trolltech exception suggests the opposite. If it's new code I'd suggest to use an up to date GPLv2+ or LGPLv2+ header. If it's based on code with actually that license, please amend the copyright header. | |
47 | At least a one line class documentation might be a good idea ;-) | |
62 | "replay" I assume? | |
66 | This would make Marc cry ;-) Even using a fairly primitive struct in that vector would be a lot more efficient: struct Command { enum { Begin, End, Reset, ... } type; QString s; QByteArray b; }; // 24 byte That is hardly larger than a single variant, not even considering the variant list around them, and avoiding the extra allocations due to that. And you can replace the string comparisons in replay() with a simple switch/case block. There are ways to make this even more compact if needed, for example by splitting out the QString/QByteArray members to separate vectors and just keeping the indexes in the struct: struct Command { enum {...} type: 3; uint bIndex : 29; uint sIndex; }; // 8 byte, limit of 2^29 embedded part commands and 2^32 commands taking strings, which is probably sufficient. In any case, both these solutions reduce allocations to just the growing vector, rather than one or two per command, which is far more important than the total memory consumption I think. | |
messageviewer/src/viewer/messagepart.cpp | ||
355 | Hm, this goes against the memory-saving streaming idea of the writer, is there no way we can avoid the record/replay approach? Ie. would it be possible to defer HTML generation to this point? |
All other issues are clear to me and I'll update the patch already.
messageviewer/src/viewer/messagepart.cpp | ||
---|---|---|
355 | At the moment no, our problem at the moment is, that mSubOtp->parseObjectTreeInternal(textNode); (line 208) directly creates html output and do not process first and than creates html afterwards. If everything in otp is rewritten with a clean first process than render, we can get rid of this and the QueueHTMLWriter again. Actually this is one of the main reasons I want to rewrite otp - with this QueueHTMLWriter we are now in the position, that i can split the process part from the redering part. |