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

[Ingo Klöcker]

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.


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/20080711/09c45a8b/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/