[Kde-pim] [patch] Printing emails [version 2]

[Ingo Klöcker]

On Saturday 12 July 2008, Jaroslaw Staniek wrote:
> Ingo Klöcker said the following, On 2008-07-11 21:37:
> > On Friday 11 July 2008, Jaroslaw Staniek wrote:
> >> Jaroslaw Staniek said the following, On 2008-07-11 10:39:
> >>> Ingo Klöcker said the following, On 2008-07-11 01:04:
> >>>> On Wednesday 09 July 2008, Jaroslaw Staniek wrote:
> >>>>> Thomas McGuire said the following, On 2008-07-08 18:43:
> >>>>>> Please go for a signal/slot solution.
> >>>>>
> >>>>> Attached for review.
> >>>>>
> >>>>> - added KHtmlPartHtmlWriter::finished() signal, connected to
> >>>>> KMReaderWin::slotPrintMsg().
> >>>>> - previous KMReaderWin::printMsg() code splitted into two
> >>>>> pieces: 1st - KMReaderWin::printMsg( KMMessage* aMsg ) -
> >>>>> connects the finished() signal and calls setMsg(),
> >>>>>    2nd - KMReaderWin::slotPrintMsg() - is a response for
> >>>>> finished() signal, calls mViewer->view()->print() and
> >>>>> deletesLater() the KMReaderWin.
> >>>>>
> >>>>> - at KMPrintCommand level, we keep QPointer<KMReaderWin>
> >>>>> s_printerWin globally until the KMReaderWin object is
> >>>>> destroyed; - on KMPrintCommand::execute() we call
> >>>>> s_printerWin->htmlWriter()->reset() to stop any previous
> >>>>> processing
> >>>>
> >>>> Using a static object is a no-go.
> >>>>
> >>>> +  if ( s_printerWin && s_printerWin->htmlWriter() )
> >>>> +    s_printerWin->htmlWriter()->reset(); // stop any previous
> >>>> processing
> >>>>
> >>>> This will cause problems if the user manages to print twice in
> >>>> rapid succession because the second print job will kill the
> >>>> first one.
> >>>
> >>> KMReaderWin::printMsg() is async now, so if we keep using
> >>> KMReaderWin on the stack, it'll be destroyed just a (see the
> >>> patch in my previous post).
> >>
> >> "just after the printMsg() call" I mean
> >
> > Exactly. That's why in the part of my message you have snipped away
> > I
> >
> > wrote:
> >>>> In fact, I do not see why the static variable is necessary at
> >>>> all. It is only used in a single method so keeping the pointer
> >>>> to the KMReaderWin in a variable appears to be superfluous. You
> >>>> can simply create several KMReaderWin on the heap without
> >>>> keeping the
> >
> >                                         ===========
> >
> >>>> pointer. The KMReaderWin's will destroy themselves at the end of
> >>>> slotPrintMsg().
> >
> > I do not see why you try to prevent several instances of
> > KMReaderWin from being created on the heap by storing the pointer
> > to a KMReaderWin used for printing in a static variable. I do not
> > see any problems with creating several instances of KMReaderWin
> > used for printing on the heap. Maybe I am missing something, but I
> > doubt it. The user can create several separate reader windows, so
> > why should it not be possible to create several KMReaderWin's used
> > for printing?
> >
> > What I propose is:
> >  KMCommand::Result KMPrintCommand::execute()
> >  {
> > -  KMReaderWin printWin( 0, 0, 0 );
> > -  printWin.setPrinting( true );
> > [...]
> > +  KMReaderWin *printWin = new KMReaderWin( kmkernel->mainWin(), 0,
> > 0 ); +  printWin->setPrinting( true );
> > [...]
> >
> > i.e. basically the same as what you proposed except that I do not
> > use a static variable for keeping the KMReaderWin pointer around
> > because it is not necessary to keep the pointer in a variable.
>
> Ingo
> OK, thanks for the explanation of basically the same idea :)
> Please see the attached patch for review.

Looks good.


Regards,
Ingo
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 194 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-pim/attachments/20080712/2fce6166/attachment.sig>
-------------- next part --------------
_______________________________________________
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/