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

List:       koffice-devel
Subject:    Re: ODF text (was Release schedule after the alpha's)
From:       Thorsten Zachmann <t.zachmann () zagge ! de>
Date:       2007-12-20 4:05:07
Message-ID: 200712200505.07737.t.zachmann () zagge ! de
[Download RAW message or body]

On Wednesday 19 December 2007, Thomas Zander wrote:
> On Wednesday 19. December 2007 02:14:20 Sebastian Sauer wrote:
> > > I have also, as you can see, formulated a set of criteria I see as
> > > showstoppers for a final release. I would also like to consider the
> > > milestones I've placed for each of the three betas showstoppers.
> >
> > I am strickly against marketing KWord as "beta" atm since I strikly deny
> > the imgo right logical; "can be saved and loaded without loss of
> > information" for KWord.
>
> I have to admit; I tried getting into the odf loading a couple of times,
> but failed.
> There are just too many classes and too many cross dependencies and no
> overview doc I could find.

I have to agree that loading/saving of the text is not simple at the moment. I 
also had a look at it some time ago.

> I think having unit tests for loading is an absolute must, but every time I
> try I'm lost in the amount of classes that have to be instantiated to be
> able to actually call something like KoTextLoader::loadList

To have unit tests for loading is actually a very good thing. I think it is 
possible to have a TextShape fill it with some text, do a save / load and see 
if the content/style is still the same.

>
> I'd like to suggest two things
>
> 1) refactor the context objects to have no arguments to their constructors
> unless they are required for the context objects constructor to run.
> Add setFoo() methods on those classes.

I think it is wrong to have an KoTextLoadingContext. The way flake is designed 
it can't work that way. For me it looks like the only true reason the have a 
KoTextLoadingContext is a way to access the KoStyleManager. 

I was just yesterday discussing with David a way to set stuff that needs to be 
available in shapes just after object creation. With that we can make it 
possible to set a KoStyleManager for text shapes by the application which 
would also make sure to have just one for all text shapes. 

Also I think it is a must that the code does also work when no style manager 
is present for applications that don't need/want to support it. 

>
> 2) explain or refactor this confusing construct;
> Constructor of KoTextLoadingContext;
>    KoTextLoadingContext (KoTextLoader *loader, KoDocument *doc,
>    KoOdfStylesReader &stylesReader, KoStore *store)
> notice how you pass a loader there.

as said above I think we should try to live without it. If a 
KoTextLoadingContext is needed it should not be a derived from 
KoOasisLoadingContext. Also it needs to get a reference to the passed 
KoShapeLoadingContext that it can pass on when there are other shapes that 
have to be loaded in the text.

> And then almost all methods on KoTextLoader you get have to pass in an
> instance of KoTextLoadingContext. The one you created *with* the loader as
> a reference.
> That just doesn't make sense to me...
> Or, more accurately, I have no idea how to use this code by just looking at
> the API and docs.

You are right it does not make sense.

> Bottom line; I want to have a way to test this.  In my test i need a way to
> instantiate the correct classes.
> Define an XML that I then let KoXml load, as my testing code.
> Then I want to call specific methods in the KoTextLoader and check if the
> QTextDocument looks correct afterwards.
>
> Who can help me with that? I already added an empty test framework to be
> filled ;)

I can provide help by explaining load/save of odf. I won't have time help 
coding as kpresenter sill needs a lot of work to be done.

Hope that helps. 

Thorsten
_______________________________________________
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]