[prev in list] [next in list] [prev in thread] [next in thread]
List: kmail-devel
Subject: Re: [Patch] kmail template facility
From: Ingo =?iso-8859-1?q?Kl=F6cker?= <kloecker () kde ! org>
Date: 2005-08-30 19:00:19
Message-ID: 200508302100.38257 () erwin ! ingo-kloecker ! de
[Download RAW message or body]
[Attachment #2 (multipart/signed)]
On Saturday 20 August 2005 14:55, Jonathan Marten wrote:
> Here's my try at a feature that lots of people have apparently been
> wanting for a long time: mail templates (bug 1015). Maybe too late
> for the 3.5 feature freeze, but hopefully suitable for inclusion in a
> future KMail release.
Again, thanks a lot for looking into this. I've now reviewed your patch.
> There is a new special folder "templates"; messages can be copied
> into here just like any other, or saved here using the 'Message -
> Save as Template' menu option in the composer window. This folder
> works similarly to the "drafts" folder; double-clicking on a message
> or choosing the "Use Template" option on the popup menu starts the
> composer on a copy of that message, but does not of course delete the
> original template message.
I think something like
File
->New Message from Template
->some template
another template
would be better than having to switch to the Templates folder in order
to create a new message from a template. But of course for editting
templates the above drafts-like functionality still makes sense.
> A template can also be used to insert text into a message being
> composed, via the 'Message - Insert Template Text' menu option. This
> will display a list of template messages (subject and summary of
> start of body); selecting one in this list and clicking "OK" will
> insert the body text into the message being composed. (The subject
> is ignored for this operation).
I'm not sure whether this is such a good idea because the user will
clutter his Template folder with two different types of things, namely
with message templates and with standard text blocks.
> There is (currently) no facility for inserting variable fields into
> the inserted template, but IMHO I agree with the bug comments that
> mail-merge does not really belong in KMail.
Something like Reply Using Template with macro replacement would be very
nice and is probably more often used then creating a new message from a
template without any context. Anyway, that can be added at a later
time.
> Still to do (if required): GUI for an identity-specific templates
> folder (if required), and the option to have the templates folder on
> an IMAP server.
Templates folder on the IMAP server will definitely be needed. I'm not
sure how useful identity specific templates folders are.
> I also haven't tested this facility with HTML
> messages, but see no reason why it shouldn't work...
Famous last words. :-)
> Patch follows (against KDE 3.4.2). There are two new source files
> (kmail/selecttemplatedialog.cpp and kmail/selecttemplatedialog.h) to
> handle the insert selection list as above. There are also some minor
> changes to libkdepim (sorting of the templates folder in the folder
> tree) and libkpimidentities (only needed if a per-identity template
> folder is required). The patch is also at
> http://www.keelhaul.demon.co.uk/kde/mailtemplates.patch in case it
> should get damaged in the post.
>
> Enjoy...
>
> -- patch starts
> --------------------------------------------------------- ---
[snip]
> --- kdepim/kmail/kmcommands.cpp.tem 2005-05-23 13:11:54.000000000
> +0100 +++ kdepim/kmail/kmcommands.cpp 2005-08-20 12:20:15.000000000
> +0100 @@ -629,6 +629,35 @@
> }
>
>
> +KMUseTemplateCommand::KMUseTemplateCommand( QWidget *parent,
> KMMessage *msg ) + :KMCommand( parent, msg )
> +{
> +}
> +
> +KMCommand::Result KMUseTemplateCommand::execute()
> +{
> + kdDebug(0) << k_funcinfo << endl;
> + KMMessage *msg = retrievedMessage();
> + if (!msg || !msg->parent() ||
> + !kmkernel->folderIsTemplates( msg->parent() ))
> + return Failed;
> +
> + // Take a copy of the original message, which remains unchanged.
> + KMMessage *newMsg = new KMMessage;
> + newMsg->setComplete(msg->isComplete());
> + newMsg->fromString(msg->asString());
> + newMsg->setStatus(msg->status());
Why do you copy the status of the template?
> + KMComposeWin *win = new KMComposeWin();
> + msg->setTransferInProgress(false); // From here on on, the
> composer owns the message.
The above line should be unnecessary.
> + newMsg->setTransferInProgress(false);
> + win->setMsg(newMsg, FALSE, TRUE);
> + win->show();
> +
> + return OK;
> +}
[snip]
> --- kdepim/kmail/kmcomposewin.cpp.tem 2005-05-23 13:11:54.000000000
> +0100 +++ kdepim/kmail/kmcomposewin.cpp 2005-08-18 16:44:03.000000000
[snip]
> @@ -3531,12 +3539,13 @@
>
>
>
> //-------------------------------------------------------------------
>--------- -void KMComposeWin::doSend(int aSendNow, bool saveInDrafts)
> +void KMComposeWin::doSend(int aSendNow, bool saveInDrafts, bool
> saveInTemplates) {
This code has changed quite a bit since KDE 3.4. It would really be
better if you'd develop against the development version. Also I don't
like the usage of another bool parameter. Using an enum would be much
better.
> mSendNow = aSendNow;
> mSaveInDrafts = saveInDrafts;
> + mSaveInTemplates = saveInTemplates;
>
> - if (!saveInDrafts)
> + if (!saveInDrafts && !saveInTemplates)
> {
> if ( KPIM::getFirstEmailAddress( from() ).isEmpty() ) {
> if ( !( mShowHeaders & HDR_FROM ) ) {
> @@ -3631,9 +3640,9 @@
> (!hf.isEmpty() && (hf != mTransport->text(0))))
> mMsg->setHeaderField("X-KMail-Transport",
> mTransport->currentText());
>
> - mDisableBreaking = saveInDrafts;
> + mDisableBreaking = (saveInDrafts || saveInTemplates);
>
> - const bool neverEncrypt = ( saveInDrafts &&
> GlobalSettings::neverEncryptDrafts() ) + const bool neverEncrypt = (
> (saveInDrafts || saveInTemplates) &&
> GlobalSettings::neverEncryptDrafts() )
>
> || mSigningAndEncryptionExplicitlyDisable
> ||d;
>
> connect( this, SIGNAL( applyChangesDone( bool ) ),
> SLOT( slotContinueDoSend( bool ) ) );
> @@ -3739,6 +3748,52 @@
>
> (static_cast<KMFolderImap*>(imapDraftsFolder->storage()))->getFolder
>(); }
>
The below duplication of the equivalent code for drafts is not nice.
Please try to come up with a way to avoid the code duplication.
> + } else if (mSaveInTemplates) {
> + KMFolder* templatesFolder = 0, *imapTemplatesFolder = 0;
> + // get the templatesFolder
> + if ( !(*it)->templates().isEmpty() ) {
> + templatesFolder = kmkernel->folderMgr()->findIdString(
> (*it)->templates() );
> + if ( templatesFolder == 0 )
> + // This is *NOT* supposed to be "imapTemplatesFolder",
> because a
> + // dIMAP folder works like a normal folder
> + templatesFolder =
> kmkernel->dimapFolderMgr()->findIdString( (*it)->templates() );
> + if ( templatesFolder == 0 )
> + imapTemplatesFolder =
> kmkernel->imapFolderMgr()->findIdString( (*it)->templates() );
> + if ( !templatesFolder && !imapTemplatesFolder ) {
> + const KPIM::Identity & id = kmkernel->identityManager()
> + ->identityForUoidOrDefault( (*it)->headerField(
> "X-KMail-Identity" ).stripWhiteSpace().toUInt() );
> + KMessageBox::information(0, i18n("The custom templates
> folder for identity "
> + "\"%1\" does not exist
> (anymore); "
> + "therefore, the default
> templates folder "
> + "will be used.")
> + .arg( id.identityName() ) );
> + }
> + }
> + if (imapTemplatesFolder && imapTemplatesFolder->noContent())
> + imapTemplatesFolder = 0;
> +
> + if ( templatesFolder == 0 ) {
> + templatesFolder = kmkernel->templatesFolder();
> + } else {
> + templatesFolder->open();
> + }
> + kdDebug(5006) << "saveintemplates: templates=" <<
> templatesFolder->name() << endl;
> + if (imapTemplatesFolder)
> + kdDebug(5006) << "saveintemplates: imaptemplates="
> + << imapTemplatesFolder->name() << endl;
> +
> + sentOk = !(templatesFolder->addMsg((*it)));
> +
> + //Ensure the templates message is correctly and fully parsed
> + templatesFolder->unGetMsg(templatesFolder->count() - 1);
> + (*it) = templatesFolder->getMsg(templatesFolder->count() - 1);
> +
> + if (imapTemplatesFolder) {
> + // move the message to the imap-folder and highlight it
> + imapTemplatesFolder->moveMsg((*it));
> + (static_cast<KMFolderImap*>(imapTemplatesFolder->storage()))->getFo
>lder();
> + }
> +
> } else {
> (*it)->setTo( KMMessage::expandAliases( to() ));
> (*it)->setCc( KMMessage::expandAliases( cc() ));
> @@ -3788,6 +3843,14 @@
>
>
>
> //-------------------------------------------------------------------
>--------- +void KMComposeWin::slotSaveTemplate() {
> + kdDebug(0) << k_funcinfo << endl;
> + if ( mEditor->checkExternalEditorFinished() )
> + doSend( false, false, true );
I guess the above line makes pretty clear why using bool parameters is
evil. If an enum with descriptive values would be used the code would
be much better readable. FWIW, the first bool has already been replaced
by an enum in the development version.
> +}
> +
> +
> +//------------------------------------------------------------------
>---------- void KMComposeWin::slotSendNow() {
> if ( !mEditor->checkExternalEditorFinished() )
> return;
> @@ -4284,6 +4347,7 @@
> */
> void KMComposeWin::slotFolderRemoved(KMFolder* folder)
> {
> + // TODO: need to handle templates here?
I don't think so. This is only necessary to deal with the case where
editting a draft is aborted and the draft has to be re-added to the
drafts folder.
> if ( (mFolder) && (folder->idString() == mFolder->idString()) )
> {
> mFolder = kmkernel->draftsFolder();
> @@ -4417,6 +4481,35 @@
> alignRightAction->setChecked( ( a & AlignRight ) );
> }
>
> +void KMComposeWin::slotInsertTemplate()
> +{
> + kdDebug(0) << k_funcinfo << endl;
> +
> + KMFolder *temFolder = kmkernel->templatesFolder();
Here obviously the identity specific template folder would have to be
used (if we allow this). But as I already wrote above I don't like this
solution that much. I'd prefer a separate solution for inserting,
saving and managing text blocks.
> + if (!temFolder) return;
> +
> + if (temFolder->count()==0)
> + {
> + KMessageBox::sorry(this,i18n("There are no templates
> available.\nCopy or save messages into the 'templates' folder in
> order to use them."));
> + return;
> + }
> +
> + SelectTemplateDialog d(temFolder,this);
> +
> + d.show();
> + if (d.exec())
> + {
> + QString s = d.selectedTemplateText();
> + if (!s.isEmpty()) mEditor->insert(s);
> + }
> +}
> +
[snip]
> --- kdepim/kmail/kmmainwidget.cpp.tem 2005-08-09 18:04:39.000000000
> +0100
> +++ kdepim/kmail/kmmainwidget.cpp 2005-08-20 12:20:44.000000000
> +0100
> @@ -2430,6 +2445,9 @@
> mEditAction = new KAction( i18n("&Edit Message"), "edit", Key_T,
> this, SLOT(slotEditMsg()), actionCollection(), "edit" );
>
> + mUseAction = new KAction( i18n("&Use Template"), "edit", Key_U,
> this,
I think something like "New Message from Template" would be better.
> + SLOT(slotUseTemplate()),
> actionCollection(), "use" );
> +
> //----- "Mark Message" submenu
> mStatusMenu = new KActionMenu ( i18n( "Mar&k Message" ),
> actionCollection(), "set_status" );
Since I don't think the Insert Template Text functionality is a good
solution I skipped the implementation of the template selection dialog.
If you take out this functionality and address the other issues I
raised then the patch will be included in the next version of KMail.
Please note, that you will most likely have to adapt your patch to Qt 4
since the next version of KMail will be based on Qt 4.
Regards,
Ingo
[Attachment #5 (application/pgp-signature)]
_______________________________________________
KMail developers mailing list
KMail-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmail-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic