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

List:       kmail-devel
Subject:    Re: [PATCH] filter changes - part I
From:       Marc Mutz <Marc () Mutz ! com>
Date:       2001-04-18 10:18:09
[Download RAW message or body]

On Tuesday 17 April 2001 13:29, Michael Häckel wrote:
> On Tuesday, 17. April 2001 09:41, Marc Mutz wrote:
> > Hi!
> >
> > I've finished the first part of the KMail filter extension and
> > cleanup of the accociated dialog. The patch was made from the
> > modified 2.1.1 against the 2.1.1 release, but I've checked that it
> > applies cleanly against current HEAD cvs.
>
> It indeed applied, however I had some trouble compiling it.
>
> Following the first error message. I personally didn't find out, why
> this command in generated and why kmfilterdlg.moc.cpp is neccessary
> at all.

I don't know why it wants to execute that command (but coolo gave a 
hint), but the kmfilterdlg.moc.cpp is generated by the moc from 
kmfilterdlg.moc. See $QTDIR/html/moc.html, Method A.

> Well after adding the missing -I parameters manually to the
> g++ command it did pass that problem.
>
> g++ -g -ansi -D_XOPEN_SOURCE=500 -D_BSD_SOURCE -Wbad-function-cast
<snip>
>  kmfilterdlg.moc.cpp   -o kmfilterdlg.moc

This should read '-o kmfilterdlg.moc.o'.

<snip>
> Then in kmkernel.cpp an include was missing.
>

If you look at the patch, I never actually touched kmkernel.cpp. Hm, it 
would be helpful to know which include it was missing. 
OK, I thought about it and it was probably kmsearchpattern.h, no? And 
then there was probably an include of kmfilterrules.h, which should 
have been kmsearchpattern.h. Also, the kmfilterrules.* files should 
have been removed. Apologies, this patch is indeed screwed. :-(



> I think things would be much easier, if you would upgrade to a CVS
> version.
>

I'd have to compile kdelibs beforehand, but I need this machine 
running. If someone knows how to develop in the HEAD branch, while 
still using the the 2.1.1 branch for everything else, please point me 
to the docs.

> > What this patch does:
> >
> > 1. Make a nice KMSearchPatternEdit in the KMFilterDlg
> >    (cf. the screenshot I sent to this list a while ago)
>
> Is there any reason, why the combo boxes for the actions are now that
> fat? IMHO they looked better before.
>

I don't know if you know how ugly (sorry to the original author) 
KMFilterDlg is/was. The width of the combo boxes was calulated from the 
width of one of the rules widgets, ignoring their own sizeHint(). 
Because I didn't touch the filter action part yet, I simpy hard-coded 
that width to what it was on my box.

Also, their positions are calculated inside an overloaded 
resizeEvent(), although they were somehow arranged in a grid layout 
already. 

Because of this, I'm currently re-writing the whole dialog top-down 
from scratch, inheriting from KDialogBase and hiding the complexity in 
three main widgets: A KMFilterListBox, which holds the list box with 
the filters, as well as the button to 

>
> > 2. Lift the Two-rules limit. You can now have up to
> >    eight rules by default. If you need more, you only
> >    need to increase FILTER_MAX_RULES in kmsearchpattern.h
> >    (up to a maximum of 26, see below)
>
> In genaral this seems to work, however when clicking on the "Fewer"
> button the window not not shrink again and the layout of the dialog
> looks bad.
>
> Layout management is not done properly in that dialog.

I know. This is what  bothers me, too. I didn't find a way to ask the 
dialog to shrink again. I put a few adjustSize()s in the more and fewer 
slots, but it seems that although the main widget's layout grows the 
height of the dialog on 'more', it doesn't shrink it on 'fewer'... 
Anyone got a hint form me?

> If I resize it, only the width of the list on the left becomes bigger
> but not the height and all other fields still are as small as before.
> Especially the fields for the text that should be contained in the
> header is nearly always too small.
>

Ah, I see. Will look into it.


> Always when I select a different filter rule the size of the dialog
> changes back to the original size, which I consider not nice.
>

The effect of one of the adjustSize()s, this ine in 
slotFilterSelected().

> I don't know what functionality the ESC key is supposed to have, but
> I think it should have the same as the Cancel button and ignore the
> changes made and not only hide the dialog.
>

This will come automatically when the dialog inherits KDialogBase.

> > 3. The filter config format has been slightly modified: It now has
> > a new key 'rules', that gives the number of rules in that filter.
> > If this key is not present, the KMSearchPattern::readConfig assumes
> > a legacy config and converts the operators 'ignore' and 'unless'
> > accordingly. The third rule is saved as
> >    (fieldC/funcC/contentsC) and so on.
>
> At least my filter for "X-Mailing-List contains
> kde-devel@master.kde.org" stopped working after that upgrade, however
> after modifying it in the dialog it worked again. I don't know
> exactely what the problem was.
>

Can you send me the corresponding section of your (old) kmailrc?

> > 4. Incorporate Don's additions to kmfilter{dlg,mgr}.{h,cpp}. Don,
> >    could you have a look. This is the single point that I didn't
> >    test, but then I've only made the changes I've done to
> >    KMFilterDlg::slotBtnNew to createFilter().
>
> There will be probably some merging required, but that doesn't look
> difficult.
>

Well, I think now that that patch is in CVS, this patch will not apply 
ceanly anymore. Workaround: revert the rapidfilter patch in those four 
files and apply this one. It already incorporates the changes the 
rapidfilter patch was supposed to do in those files.

> > I have tested these modifications quite heavily. But of course, I
> > couldn't think of all the things that people will do with their
> > filters, so please consider putting these changes into CVS for
> > anyone to test.
>
> I think the problems mentioned above are already enough for now and
> don't see a real advantage if this patch being in CVS in its current
> state. That would only result in a high amount of bug reports about
> known problems. If you care for those things first I might put it
> into CVS before you start to work on the actions. Also at least the
> compilation problems need fixing first.
>

Because I already have stared working on the actions and the filter 
dialog as a whole, I'm waiting until that's done before I send another 
patch. If anyone cares to tell me how I can develop for a HEAD app 
while using a 2.1.1 desktop, I'd jump asap to using CVS HEAD as the 
basis of my work, removing the prime source of errors.

> > There's a known problem, though: If you use filters while the
> > filter dialog shows a filter, that filter may contain invalid data.
> > This is because the widgets operate directly on the filters.
> >
> > I will work around this problem by making KMFilterDlg a subclass of
> > KDialogBase, which holds a deep copy of the filter list and
> > provides an 'Apply' button to transfer the changes in the copy back
> > to KMFilterMgr.
>
> Doesn't the filter dialog already have deep copies as long as the
> user doesn't press the Ok button?
<snip>

No. KMFilterMgr inherits the reference-based QList<KMFilter> and does 
not implement QCollection::newItem(), which could provide deep copies. 
The QList::at() method, used in slotNewFilter() and called functions 
only returns a reference to the item.

Bye,
Marc

-- 
Marc Mutz <Marc@Mutz.com>
http://marc.mutz.com/
http://www.mathematik.uni-bielefeld.de/~mmutz/
http://EncryptionHOWTO.sourceforge.net/
_______________________________________________
Kmail Developers mailing list
Kmail@master.kde.org
http://master.kde.org/mailman/listinfo/kmail

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