Review Request 109980: Load styles into a new, more generic style manager

[Inge Wallin]

On Tuesday, April 16, 2013 23:59:01 Sebastian Sauer wrote:
> On 04/13/2013 08:37 PM, Inge Wallin wrote:
> > (Note to zagge: sorry for double sending this, the first reply lost the
> > caligra-devel recipient for some reason.)
> > 
> > On Saturday, April 13, 2013 14:14:40 Thorsten Zachmann wrote:
> >> Hello,
> >> 
> >>> Here is step 2 in the plan to create a docx export filter.
> >>> 
> >>> It contains code to load styles into a decently generic style manager.
> >>> The
> >>> main use of this version will be as a much faster alternative to
> >>> KoOdfStylesReader that forces you to reparse the XML of all styles that
> >>> you
> >>> want to access. This code loads the style content into more easily
> >>> accessed
> >>> data structures. The current version only supports the Text, Paragraph,
> >>> and
> >>> Graphic families because that's what we need in the docx filter.
> >>> 
> >>> I also have a half-finished patch of Karbon that makes it load, store
> >>> and
> >>> save graphics styles which it currently discards in all odg files.
> >>> However
> >>> that turned out to be more difficult than expected because the text
> >>> based
> >>> style manager in libs/kotext doesn't allow anybody to save any styles
> >>> outside itself so I'm postponing that into a separate patch. This way we
> >>> can start using these styles when doing the docx export.
> >>> 
> >>> As a side note, I have also included a new innovative XML stream reader
> >>> which I will port the odf traverser to.
> >> 
> >> I'm a bit unhappy that this was put into libs while for now only the docx
> >> export filter is going to use it. Unfortunately the review was not open
> >> very long so I could raise my voice before it was committed. I think
> >> this should be move to the docx filter for now as now it gives a
> >> toatally wrong impression in the libs.
> 
> +1
> +it makes the odf lib bigger with things not related.
> +introduces names that make, at a first look, the impression as its
> related to odflib while its not.

I can understand that you feel that you are not using it right now.  But i can 
not understand how you can say that it is not related to ODF.

Style handling is at the very core of ODF and right now we are simply not 
doing a good job of it.  Named styes get dropped (specifically graphic styles 
but also others), styles are only partly loaded and they are read and cached 
in the wrong way. Most of this is not visible to the user but there is 
actually quite a bit of data loss going on.  Read 
http://community.kde.org/Calligra/Architecture/Styles/Internals for the gritty 
details.

Actually this patch is -- beside being a bit of a rush job because we need to 
get a working docx filter in less than 2 weeks -- the first patch in my 
suggested plan to fix these issues. And this plan was discussed on this mailing 
list if I recall correctly and approved.

> + We are not force to out everything into the same lib... its even very
> useful to proper split things to make maintaining, understanding,
> reusing easier. E.g. why should I need to build this for coffice (which
> neither has Karbon nor export included).

Well, that is the nature of libraries.  You seldom use all of it. If you are 
really really strapped for space then there are other classes in libodf that I 
could suggest for removal.  But the biggest space waster for coffice is probably 
the ties to QWidget in the flake shapes.

The total amount of code in the cpp files are ~1300 lines (~600 in 
libs/odf/styles and ~700 for KoXmlStreamReader). I think splitting that out is 
not worth it because it will have to be merged back very soon anyway.


> >>   Also I'm not sure it will be easily be useable for
> >> 
> >> how we handle styles currently in the applications. If we have it
> >> integrated and well tested in more then one component then I think would
> >> be
> >> the right time to put it into libs.
> > 
> > I see what you mean.  Maybe I was not clear enough in the review but it's
> > not only going to be used in the filters.  If that had been the case I
> > would indeed have put it under filters/.  The docx filter is only the
> > first user.
> > 
> > As I think I wrote in the review request I have a half-finished patch that
> > uses this and that lets Karbon retain the graphic styles that it
> > currently discards.  I think that's a very important fix and it has
> > bugged me for a long time that it doesn't preserve them.  I will finish
> > this and put it up for review so you can see how it can be used in  one
> > of the applications too.
> then make it an own lib shared between Karbon and export filters? :)

This is not the best way.  If we wanted to go this way and take it to its 
logical conclusion we would have to find out for every file in the libraries 
which are used in which applications and build separate libraries for the sets 
of files and applications.  That way lies madness.

> >> Thorsten.
> > 
> > _______________________________________________
> > calligra-devel mailing list
> > calligra-devel at kde.org
> > https://mail.kde.org/mailman/listinfo/calligra-devel
> 
> _______________________________________________
> calligra-devel mailing list
> calligra-devel at kde.org
> https://mail.kde.org/mailman/listinfo/calligra-devel