[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kmail-devel
Subject:    Re: [PATCH] Copy Filter Action - review please
From:       Don Sanders <sanders () kde ! org>
Date:       2004-11-30 7:05:32
Message-ID: 200411301705.33202.sanders () kde ! org
[Download RAW message or body]

On Tuesday 30 November 2004 09:12, Ingo Klöcker wrote:
> On Monday 29 November 2004 20:48, Andreas Gungl wrote:
> > On Sunday 28 November 2004 23:23, Ingo Klöcker wrote:
> > > On Sunday 28 November 2004 21:17, Andreas Gungl wrote:
> > > > Hi,
> > > >
> > > > The attached patch introduces a Copy filter action. I've tested
> > > > with incoming messages as well as when applying manually. Can you
> > > > please have a look at it and tell me if you find some critical
> > > > code?
> > >
> > > Do we really need a separate "Copy into Folder" filter action? IMO,
> > > no. Shouldn't multiple "File into Folder" filter actions just do
> > > the same? IMO, yes. The only problem with "File into Folder" is
> > > that it's always done as last action. This makes it impossible to
> > > do for example the following: File a copy into a backup folder,
> > > modify the message (with a script or the header modification
> > > actions), file the modified copy into another folder.
> >
> > Do you really think that "File into Folder" should put a copy of a
> > message into a folder?
>
> Yes.
>
> > What would be the semantic of subsequent "File
> > into Folder" actions within one filter rule?
>
> Put a copy of the message into the given folder, of course.
>
> > What would happen with a
> > message having one "File into Folder" actions in each of some rules
> > which are all applied to the message?
>
> A copy of the message would be put into several folders.
>
> > Don't you think that it's much easier for a user to handle a "Move
> > into Folder" plus a "Copy into Folder" action?
>
> Neither "move" nor "copy" make sense for fetched messages because both
> actions require a source (from where the message is deleted in the
> "move" case and where the "original" message stays in the "copy" case).
> At least from a semantical POV "File into Folder" makes much more
> sense. OTOH, this terminology is also flawed due to the digital nature
> of email messages. There's no "the message". There are just copies of
> the message (on the POP server, in memory, in folders).
>
> Anyway, regardless of what label we use for the action IMO we only need
> one. A distinction between "move" and "copy" would just confuse the
> user. For example, what happens if the user uses "copy into folder" but
> doesn't use "move into folder". The only logical result would be to put
> a copy into the give folder and another copy into the default inbox
> (for non-IMAP accounts).
>
> > What about the usability argument?
>
> That's exactly why I don't want two actions.

Yeah, I can see the logic in that, I guess that (having just a single file 
into folder, or whatever it's called action) would be ideal.

But IIRC from an implementation point of view currently when filtering if a 
messages matches a filter rule with a file into folder action and filtering 
doesn't stop there, and then the message matches another filter rule with 
another file into folder rule, then the first file into folder action would 
essentially be ignored and only the last file into folder rule would take 
effect.

So I guess achieving the ideal could be challenging, or at least it would 
require a change to filtering semantics.

> > > Opening and closing the folder in KMFilterActionCopy::process() can
> > > be very slow. It would be better if the folders were kept open
> > > until filtering is finished. IIRC "File into Folder" already works
> > > like this.
> >
> > The behavior could be changed. However I wonder how keeping folders
> > open can work when the filters are processed asynchronously and thus
> > perhaps in parallel. It might be a hard job to synchronize this.
>
> Keeping folders open works already. The filter manager keeps track of
> the open folders.
>
> > > Will mFolder->addMsg work for IMAP folders? I guess this will
> > > require the usage of the ActionScheduler Don is working on.

My work won't make mFolder->addMsg won't work for IMAP folders.

I rely on the KMMoveCommand in the action scheduler. Actually the problem I am 
currently working on is related to the KMMoveCommand, sometimes it is failing 
to cause a folderComplete signal to be emitted for the destination folder. 
(If anyone has an idea why I'm interested, but I intend to look into it 
tonight).

> > > I don't think it makes much sense to further look into this before
> > > Don's changes are in place.
> >
> > My change is nearly trivial against the changes Don is working on. I
> > really don't see how my change could be a high risk for Don's
> > progress. But of course it's the best to wait for his comment.

Well if the current filtering system is modified to support copying into 
folders using the mFolder->addMsg command then those modifications won't work 
with the action scheduler, in the sense those modifications won't work for 
IMAP folders. Which means, as far as i can see, in order to support copying 
with the action scheduler I will have to obsolete the work Andreas is doing 
and create a new implementation.

Actually I think I already have a todo item regarding logging of filtering. 
IIRC Andreas modified kmfiltermgr.* to do that, and I have yet to port those 
changes to the action scheduler.

So yeah, I mean to be honest, filtering changes like these do increase the 
amount of complexity I need to deal with.

Don.
_______________________________________________
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]