D8087: Make HtmlWriter compatible with Grantlee::OutputStream

Volker Krause noreply at phabricator.kde.org
Sun Oct 1 16:06:22 BST 2017


vkrause added a comment.


  Version bump will be done once the entire patch series is in, we don't have enough numbers to do that after every single commit ;-)

INLINE COMMENTS

> knauss wrote in defaultrenderer.cpp:348
> should this be flush() instead of end, because there is no rule not adding antoher thing after reading.

Theoretically, yes. However that makes no sense semantically, as you would duplicate the first part of the output.

> knauss wrote in defaultrenderer.cpp:955
> this looks very implictit and hard to understand what is going on, maybe a function to HtmlWriter to handle this directly, because we'll use this all over the place, but the disadvante is that we add Grantlee dependes to mimetreeparser, so it's a no-go
> 
>   htmlWriter->addGrantleeOutput(t, &c);
> 
> so maybe better add a util method:
> 
>   AddGrantlleOutputToHtmlWriter(t, &c, htmlWriter);

Just to save one line of code? And I don't see how that makes anything clearer, "OutputStream" looks pretty self-explanatory to me.

> knauss wrote in filehtmlwriter.cpp:59
> shouldn't we still flush, at the end?

HtmlWriter::end() flushes the stream, closing the file flushes the device, so that's all covered.

> knauss wrote in htmlwriter.h:82
> you removed everywhere the flush but add in the docu, that it needs to be flushed, that sounds wired

That's referring to QTextStream::flush() that you can call on stream() directly. For normal QString output you wont need that, just for raw device access.

REPOSITORY
  R94 PIM: Message Library

REVISION DETAIL
  https://phabricator.kde.org/D8087

To: vkrause, knauss
Cc: #kde_pim, dvasin, winterz, vkrause, mlaurent, knauss, dvratil
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20171001/0a6da034/attachment.html>


More information about the kde-pim mailing list