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

[Sebastian Sauer]

On 04/17/2013 02:50 AM, Inge Wallin wrote:
> 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.
>

>    But i can
> not understand how you can say that it is not related to ODF.

I wrote odflib. I must admit I not got any argument of your replies out 
why not to solve. So, maybe its an understanding issue.

Let me reparse:
1) this is not about the actual code, to have it in the repo or have it 
done this way or... whatever.
2) its only a request to not mix 2 unrelated concepts in one library.
3) Its a request to not make maintaining one of the core and most 
difficult pieces we have in Calligra more difficult without good reason.

This is a 1 minute job, has zero negative effects but a positive effect 
on maintainence cost. So, why *exactly* not do?

>>>>    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.

What is the better way?