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

List:       kmail-devel
Subject:    Re: Partial merge of kroupware into HEAD
From:       Don Sanders <sanders () kde ! org>
Date:       2002-11-30 3:29:05
[Download RAW message or body]

On Saturday 30 November 2002 02:36, Marc Mutz wrote:
> On Thursday 28 November 2002 15:50, Don Sanders wrote:
> > On Thursday 28 November 2002 23:47, Marc Mutz wrote:
>
> <snip>
>
> > > There are some changes that I think need more work. Off the top
> > > of my head, Index should be a class of it's own right, not part
> > > of the KMFolder hierarchy. Folders would then use the index as
> > > a Strategy with unindexed folders using a NullIndex object.
> >
> > If we ever reach the stage where multiple different index
> > implementations are required then that might make sense, but I
> > don't see any reason for adding that extra complexity before the
> > merge. Or perhaps I misunderstand?
>
> It's not adding complexity, it's reducing complexity. 

I have to disagree, if/when an alternative to .index files is under 
development then this suggestion would make sense. But at this 
present point in time such a generalization is not useful.

> Complexity of
> KMFolder. When a class does too much, you Extract Class. Should be
> quite easy. We don't need the Index class created by a factory
> method, of course. But Index has no place in KMFolder, IMO.

Because search folders don't require an index I moved the Index code 
out of KMFolder, this is already done.

This is a major step forwards towards supporting alternative index 
implementations. What is in make_it_cool works and is a major 
improvement over head. Handling the more general case of alternative 
index implementations would be nice but I see no reason to delay the 
work already completed until this other extra feature of alternative 
index implementations is implemented also.

I really don't think this objection is a valid reason for keeping my 
work out of HEAD.

> > > I see kmindexer
> > > being added. Is that what I describe
> >
> > Sorry, what did you describe?
>
> Extracting index handling into KMail::Index.
>
> > > or is it used as a parser that
> > > builds the index? If the latter, building the index should be
> > > done through a Builder.
> >
> > There's always room for improvement. The indexing code is in the
> > early stage of development and is disabled in the code by
> > default. It can easily be removed if it's not completed by 3.2.
> >
> > Having said that I really don't see how the Builder pattern is
> > relevant here.
>
> Builder abstract parsing from generating the in-memory
> representation. If KMail::Index is the in-memory (or mmapped)
> representation, then it needs not know how to parse. I guess that's
> what Sam tries to achieve with the indexer.

You've completely misunderstood what's going on here.

KMail stores summary information in *.index files. KMMsgIndex has 
nothing to do with that.

KMMsgIndex is a full text indexer, it's comparable to the full text 
index used by Web Search Engines, or Digitial Library Software or the 
Evolution mail client. KMMsgIndex speeds up slow searches (like body 
searches) by creating what's known as an 'inverted file'.

> So the question is: Is indexer part of a builder pattern that
> combines indexer and KMFolder(Index) or is indexer the result of
> extracting index handling from the kmfolder hierarchy?

Neither, KMMsgIndex has nothing to do with .index files.

> > > Has the separate readerwin issue been resolved?
> >
> > I'm very happy with the current solution of KMReaderMainWin being
> > a separate class, especially now that Ingo's suggestions have
> > been implemented and the code duplication eliminated. I see this
> > solution as much preferable to making KMMainWin a generic
> > container class.
>
> Sounds better than last time around ;-)

It's a marginal improvement.

> > > Maybe the people working on make_it_cool can give an overview
> > > of changes that were done there, so we can start merging the
> > > ones that are consensus?
> >
> > You can see a list of user visible changes in kmailNewFeatures in
> > kmreaderwin. Also inplace renaming of folders is now supported,
>
> Danimo told me he's not really happy with it due to accel problems.
> So that's not a part that needs to go to HEAD.

I think it could do with some more work, it's probably better to 
disable it for now and continue working on it in HEAD.

> > and
> > ryan breen's system tray notification patch has been applied.
>
> That's something that everyone's welcome, I guess.
>
> > Architecturally KMFolder has been split into KMFolderIndex that
> > implements .index file support and a basic KMFolder class that
> > does the rest.
>
> See above.
>
> > KMFolderSearch has been added that implements search folder
> > support.
>
> From how you explained the implementation (list or sernums), I
> guess it's OK.
>
> > Zack's imap cleanups that he has already talked about on
> > this list have been applied, and also he has made some custom
> > folder icon cleanups.
>
> OK.
>
> > KMail has been transformed into a KPart and this
> > required KMMainWin being split into KMMainWin and KMMainWidget,
> > and some startup code being moved out of main.cpp into
> > kmstartup.cpp.
>
> That's what we need to port kroupware to, yes.
>
> > Also several top wishes are implemented including duplicate
> > removal,
>
> OK.
>
> > dynamic filters.
>
> I have minor cosmetic things about that one, but the feature in
> itself is great.
>
> > The two outstanding major bugs accidental folder
> > switching and draft messages being lost have been fixed.
>
> Something for 3.1.1?

Potentially.

> > Folders are
> > periodically saved to prevent information loss on abnormal
> > shutdown, the .sorted file has been ported to serial numbers so
> > that it remains valid after message deletion, the kmcommands have
> > been modified so that all commands only need data in the kernel.
>
> Sounds good.
>
> > Some icon changes have been made to cr*.png those shouldn't go
> > into HEAD,
>
> Why were they done then?

To prepare for an independent release of the make_it_cool branch.

> Thank you _very_ much for the summary. As you can see, I agree with
> most of them in principle. That doesn't mean that they should go in
> in one go, though.

Well some of the changes discussed are radical changes in core 
architecture, last time I updated make_it_cool it was stable, so the 
benefit of introducing them holistically is that the current 
stability will be (more easily) preserved.

Don Sanders / sanders@kde.org
KMail  Adopter / kmail.kde.org
KDE PIM  Co-founder / kdepim.org
KAddressbook  Founder / kaddressbook.org
KDE  Contributor / kde.org

_______________________________________________
KMail Developers mailing list
kmail@mail.kde.org
http://mail.kde.org/mailman/listinfo/kmail
[prev in list] [next in list] [prev in thread] [next in thread]