[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-27 8:44:13
Message-ID: 200712270944.13622.t.zachmann () zagge ! de
[Download RAW message or body]

Hello,

I have put answers of different mails in this one so that all is in one place.

On Wednesday 26 December 2007, Sebastian Sauer wrote:
> > I can't see right now why the KoTextLoader needs to be passed around. Can
> > you tell me the reason for it?
>
> 1. The KoTextLoader does cache loaded styles what means just like the
> KoStyleManager got passed around, it's needed to pass the loader around.
> The caching is there since it improves loading by something like 50% or so
> and a few months back we (Pinaraf, myself and Thomas) decided to don't add
> that code to the KoStyleManager itself since it's only related and needed
> during loading and will be later added to the stylemanager.
> 2. Since each Shape does only provide those loadOdf - what includes the
> textshape - and since there was no way to pass in even more infos (like the
> current list-level and all the other things that are uncommented / not
> implemented yet), the context got introduced. This is needed since e.g.
> KWord does create own shapes for the header, the footer and the body and we
> still like to e.g. share the styles.
>
> > You are right flake does not allow to pass around data. But also
> > subclassing does not work as then the application creating the context
> > would allways need to create the subclassed version which is e.g. not
> > possible for the copy and paste classes that are in flake as flake does
> > and should not depend on kotext.
>
> The context is created on the fly if not passed as well (just like the
> stylemanager and the loader).
>
> I still agree that the solution may not be the best one, but I still don't
> see a better one.

How about extending the KoShapeLoadingContext to have a QMap<QString, 
QVariant> to store data that should be passed between the different shapes on 
loading. This way kotext could use the KoShapeLoadingContext and would not 
need to have a own context. When a text shape is loaded it checks if it's 
cache is available and if not it initializes it once. When the next text 
shape is loaded it sees that the cache is there and uses it.

> > For kword it is possible to subclass the ShapeLoadingContext and
> > pass it around, but only kword related classes can use it. I use
> > something like that in kopageapp to load the pages which are not handled
> > in flake.
>
> and that's exactly what the loadercontext does except that it allows also
> others to _optional_ use it. Though I wonder a bit now that kopageapp does
> the same but kotext should not :-/

The problem is not that is has it's own context and passes it around. The 
problem is that when it creates its own context it throws away the 
KoShapeLoadingContext which is needed to load shapes that are nested in the 
text shape.

> Right, same problem like before; KoStyleManager need to be passed around
> somehow and if we like to have loading across different text-shapes working
> as well, then we also need to pass the textloader around.

That is why I wrote the mail on how to initializing values in shapes. With 
that we can make sure applications have a the style manager set on text 
shapes if they want to use it. In the other thread we agreed that only 
applications wanting to have a KoStyleManager should provide one. The loading 
should work also when there is no KoStyleManager used for applications that 
prefer to don't have one.

On Wednesday 26 December 2007, Sebastian Sauer wrote:
> > > 2. Use the solution I proposed that will make it possible to set
> > > document wide stuff in a shape after it is created. This will also make
> > > the KoTextLoadingContext unnecessary.
> > >
> > > If however the style manager is not needed during loading I propose to
> > > remove the KoTextLoadingContext as I can't see the use of it.
> >
> > 2) sounds fine as in the most logical way to go imho, but if you remove
> > the KoTextLoadingContext class, please check that files still load since
> > I doubt that 2) makes KoTextLoadingContext unnecessary (and it would be
> > so horrible demotivating to see it such deep broken at this state).
> > Anyway, any idea/hint/code how to remove it while preserving the
> > functionality would be very welcome :)
> > Also if there are any questions please just ask since I know that the
> > code is rather confusing (but unfortunately I've the feeling it's not
> > easy to change that - p.s. the flake-code is imho even more confusing to
> > me :-)

I sure don't want to break anything on the change I propose. I hope to fix 
some things that don't work at the moment by the change. We sure need to keep 
the functionality of the KoTextLoadingContext.

> a small extension;
> and we would still have the alternate to let KoOasisLoadingContext inherit
> QObject and allow that way to attach own stuff... or at least properties...

If is the KoShapeLoadingContext that has to be extended and not the 
KoOasisLoadingContext. Do you think the solution I described above with the 
map of variants would work. It looks the same QObject properties is the same 
stuff.

So what do you think?

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]