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

List:       koffice-devel
Subject:    Re: pagestyles branch
From:       Sebastian Sauer <mail () dipe ! org>
Date:       2008-08-16 15:51:52
Message-ID: 200808161751.52734.mail () dipe ! org
[Download RAW message or body]

On Friday 15 August 2008, Pierre wrote:
> > yet it doesn't have a copy constructor and its even a QObject.
> > I think it would be very helpful to make this class a QSharedData
> > inheriting object and get rid of the signal by just making the view force
> > the relayout. This would solve the problem if it being a QObject and it
> > would also solve the now rather odd passing of the document pointer to
> > the pageManager. This doc pointer to the pagemanger I think is something
> > we should try to avoid. The pattern I set up is that KWDocument is the
> > front end for common things and that it defers to the page manager for
> > less-common API. You can see this by the fact that the document has an
> > (non-pointer) instance of the pageManager.
> >
> > As an added bonus, this makes unit testing the pagemanager a LOT easier.
>
> Can't agree more, I'll look at this.

hmmm... for what exactly is the pagemanager if not to outsource everything 
related to pages from the doc? If that's the case then we may still like to 
be able to access the doc itself from the pagemanager to e.g. manipulate 
frames, relayout (ok, maybe that logic should be moved to the pagemanager 
too?), etc.

Re view; the view's are *optional* what means, we can trust in them being 
there and doing important things like forcing the relayout for us. We should 
really try to keep the model (kwdocument) and the view (kwview) separated 
here cause else we will break any possibility to run kword from within the 
commandline and automate things using e.g. kross or other apps that utilize 
kwdocument but don't provide a kwview.

> > * KWPageManager::addPageSettings seems to leak an instance of
> > KWPageSettings if the name has been added before.
>
> It should never happen, and I don't really see a way to properly deal with
> this.

Isn't there a Q_ASSERT for that reason? iirc I extended the KWOdfLoader to 
handle broken odt's that define the masterpages with the same masterpagename 
multiple times (though they would not pass jing and would be invalid odt's).

> For instance, if one call addPageSettings with the following code :
> KWPageSettings *settings = new KWPageSettings("MyStyle");
> pageManager->addPageSettings(settings);
> pageManager->appendPage(settings);
>
> I think your point is fully valid, but the only "reliable" fix I see would
> be to have the KWPageSettings constructor being private and its creation
> managed by the KWPageManager.

yeah, sounds like the best solution imho :)
_______________________________________________
koffice-devel mailing list
koffice-devel@kde.org
https://mail.kde.org/mailman/listinfo/koffice-devel
[prev in list] [next in list] [prev in thread] [next in thread]