Review Request: Style inheritance

[C. Boemann]

> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > libs/kotext/styles/KoCharacterStyle.cpp, line 406
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40046#file40046line406>
> >
> >     hmmm... does this mean we are applying the format way more often now?
> >     
> >     Consider;
> >     <p format=1>Some<p format=2>Thing</p></p>
> >     then we are apply at "Thing" do we unnecessary set format=1 twice now on the QTextCharFormat. First when apply the QTextCharFormat to the first paragraph and then again when applying the QTextCharFormat at the second (sub-/child-)paragraph?
> >     
> >     Can this have a performance impact? Should be easy to check with valgrind/vtunes.
> >

well we may be applying more often, but the number of properties is the same (although overlapping properties will be applied again)

Anyway there is not really an alternative to this


> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > libs/kotext/styles/KoParagraphStyle.cpp, line 1865
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40048#file40048line1865>
> >
> >     Wasn't that method just removed 10 lines above? I don't see that it was added to the KoCharacterStyle or anywhere else... hmmm?

hmm indeed, i think it may be a git malfunction


> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > libs/kotext/styles/KoStyleManager.cpp, line 168
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40049#file40049line168>
> >
> >     so, something like
> >     Q_ASSERT(!dynamic_cast<KoParagraphStyle*>(characterStyle) || dynamic_cast<KoParagraphStyle*>(characterStyle) != d->defaultParagraphStyle);
> >     would not assert here?
> >     Means it cannot happen that the default KoParagraphStyle is in the d->charStyles list?

It should indeed not happen, so such an assert would be correct


> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > plugins/textshape/dialogs/StylesModel.cpp, line 224
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40054#file40054line224>
> >
> >     same question as above

same answer


> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > libs/kotext/styles/KoParagraphStyle.cpp, line 1200
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40048#file40048line1200>
> >
> >     KoCharacterStyle::loadOdf(context);
> >     ?
> >

what is the question


> On Nov. 1, 2011, 8 p.m., Sebastian Sauer wrote:
> > libs/kotext/opendocument/KoTextWriter_p.cpp, line 521
> > <http://git.reviewboard.kde.org/r/103017/diff/1/?file=40044#file40044line521>
> >
> >     hmmm... this means that if styleManager->characterStyle(charFormat.intProperty(KoCharacterStyle::StyleId)) returns NULL we are crashing now.

right i'll fix that


- C.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/103017/#review7833
-----------------------------------------------------------


On Nov. 1, 2011, 5:41 p.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103017/
> -----------------------------------------------------------
> 
> (Updated Nov. 1, 2011, 5:41 p.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Fix inheritance in paragraph styles
> introduce inheritance in character styles
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoTextEditor.cpp 91dc334 
>   libs/kotext/opendocument/KoTextSharedLoadingData.cpp c1991bc 
>   libs/kotext/opendocument/KoTextWriter_p.cpp 7b92403 
>   libs/kotext/styles/KoCharacterStyle.h 53e98a1 
>   libs/kotext/styles/KoCharacterStyle.cpp 665e066 
>   libs/kotext/styles/KoParagraphStyle.h 3f9f40c 
>   libs/kotext/styles/KoParagraphStyle.cpp cdf16a4 
>   libs/kotext/styles/KoStyleManager.cpp fbbc4ee 
>   libs/kotext/styles/tests/TestStyles.cpp ea7a434 
>   libs/textlayout/KoTextLayoutArea.cpp 28e4141 
>   plugins/textshape/TextTool.cpp e699319 
>   plugins/textshape/dialogs/StyleManager.cpp 6110044 
>   plugins/textshape/dialogs/StylesModel.cpp 168ef715 
>   tables/Cell.cpp 0a74d9f 
>   tables/Map.cpp 956b3e1 
>   words/part/KWDocument.cpp 1941d7a 
> 
> Diff: http://git.reviewboard.kde.org/r/103017/diff/diff
> 
> 
> Testing
> -------
> 
> cstester, show some improvements and some regressions
> 
> 
> Thanks,
> 
> C. Boemann
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20111101/7f4ef9bb/attachment.htm>