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

List:       kmail-devel
Subject:    Re: Address completion (Re: Duplicates)
From:       Christian Zangl <cza () sixxac ! com>
Date:       2006-09-14 6:10:59
Message-ID: 200609140810.59961.cza () sixxac ! com
[Download RAW message or body]

Since you put me into the CC I think you mixed me up with this other Christian 
("On Monday 11 September 2006 15:20, Christian wrote").

I've found his address in the archive and added it to the CC so that he can 
reply.

bye, chris

> -----Original Message-----
> From: David Faure <dfaure@klaralvdalens-datakonsult.se>
> Date: Thursday 14 September 2006 00:00
> On Tuesday 12 September 2006 23:55, Allen Winter wrote:
> > On Monday 11 September 2006 15:20, Christian wrote:
> > > Hi,
> > >
> > > can some one please have a look at these bugs. I think they can be
> > > marked as duplicates and fixed with the suggested patch for Bug 107945
> > >
> > > Bug 98691: tab completion last name / nickname
> > > Bug 76739: Partial string search for contacts in address fields
> > > Bug 77342: include nicknames in address lookup
> > > Bug 109798: apply distinct on address in dropdown for autocomplete
> > > Bug 107945: wrong/strange autocomplete in composer addressfield
> >
> > Couple of things:
> >  1. Christian is doing great work
>
> I concur. I am *very* impressed that Christian found a way to implement
> substring completion in kmail's composer, I thought it was impossible to do
> with KCompletion. I stand corrected, obviously.
> I tested the patch and it works fine here, I get a single completion offer
> with a standard form, whether I type the first name, last name, or email.
>
> But... am I right that this patch basically forces popup completion, i.e.
> breaks/disables the other modes of completion? If that's the case, I would
> advocate to either find a way to make them work again, or to disable/remove
> the option in the RMB popup which allows to set the completion mode.
> I personally don't see why anyone really would want automatic completion...
> but I'm sure that there are people using it, maybe because some bug in the
> popup completion made them switch to it, or for whichever other reason :/
> In any case the config options have to match the capabilities.
>
> Coding comments:
> > +    QStringList t  = emailList.grep(item);     //search for current
> > email in list +    if( t.empty() )                            //add email
> > if not there
>
> grep to find if an item is in a list is a slow solution, no need to
> construct a temp stringlist etc. The fastest way in qt3 is
>   if (emailList.find(item) != emailList.end())
>
> > QStringList sl = QStringList( addr );
>
> can be simply   QStringList sl( addr );
>
> > +    for( QStringList::Iterator sit (mailAddr.begin()),
> > sEnd(mailAddr.end()); sit != sEnd; ++sit){
>
> Should be a ConstIterator.
>
> > +  QMap<QString,QString> mailAddrDistinct;
> > +      mailAddrDistinct[(*sit)];  //store mailAddr, QMap will make them
> > unique
>
> I know that qt3 is missing QSet, but maybe this could be a
> QMap<QString,bool> to avoid copying the empty string into every entry and
> to avoid the somewhat surprising notation above, which would be come
> mailAddrDistinct[(*sit)] = true.
>
> > +  pMatches->operator+=(mailAddrDistinct.keys()); //add emailAddr
>
> Unusual too, to write "operator+=" ;)
> The alternative isn't great, I admit, but I think it's a bit easier to
> read: (*pMatches) += mailAddrDistinct.keys();
>
> > +  QStringList keyWords;
> >    for ( it = emails.begin(); it != emails.end(); ++it ) {
> >+    keyWords.clear();
>
> Better declare keyWords inside the for loop. Make variables as local as
> C++, it helps maintainance and future refactorings.
> Same for QStringList mailAddr in postProcessMatches.
>
> In any case - thanks for that work, I am happy to see this completion stuff
> being made sane.
_______________________________________________
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]