Review Request: Style inheritance

[Jarosław Staniek]

> On Nov. 6, 2011, 2:01 p.m., Jarosław Staniek wrote:
> > libs/kotext/styles/KoTableCellStyle.h, line 159
> > <http://git.reviewboard.kde.org/r/103017/diff/4/?file=40374#file40374line159>
> >
> >     cannot be const?
> 
> C. Boemann wrote:
>     well it's returning an internal member and yes that is bad
>     
>     and yes that is part of the needed api restructure too

ok so ignore this


> On Nov. 6, 2011, 2:01 p.m., Jarosław Staniek wrote:
> > libs/kotext/styles/KoParagraphStyle.h, line 677
> > <http://git.reviewboard.kde.org/r/103017/diff/4/?file=40370#file40370line677>
> >
> >     Extra bool arg is not good API, one day we'll remove all. Would be nice to avoid adding new signatures like this.
> >     
> >     How about this: split to two methods: 
> >     
> >     void loadOdf(const KoXmlElement *element, KoShapeLoadingContext &context)
> >     
> >     void loadOdfWithParents(const KoXmlElement *element, KoShapeLoadingContext &context)
> >     
> >     (and move the original method to internal  helper)
> 
> C. Boemann wrote:
>     i guess, i mean i dont disagreee
>     
>     it's just that we already know that styles need a big api restructure anyway so there isn't much point and this was the road of least trouble
>     
>     i'll give it some thought

Fix I proposed would be easy and can generate good habits.

That said, yes, full refactoring will be big, yes and ideally would require changes to value-based data structures to escape from pointers.


- Jarosław


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


On Nov. 6, 2011, 9:47 a.m., C. Boemann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103017/
> -----------------------------------------------------------
> 
> (Updated Nov. 6, 2011, 9:47 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Description
> -------
> 
> Fix inheritance in paragraph styles
> introduce inheritance in character styles
> 
> 
> Diffs
> -----
> 
>   libs/kotext/KoTextDebug.cpp 45df678 
>   libs/kotext/KoTextEditor.cpp 91dc334 
>   libs/kotext/opendocument/KoTextLoader.cpp fc7b764 
>   libs/kotext/opendocument/KoTextSharedLoadingData.h 320db92 
>   libs/kotext/opendocument/KoTextSharedLoadingData.cpp c1991bc 
>   libs/kotext/opendocument/KoTextWriter_p.cpp e9f1414 
>   libs/kotext/opendocument/tests/TestLoadStyle.cpp 37d39fd 
>   libs/kotext/opendocument/tests/TestLoading.cpp 222fb81 
>   libs/kotext/styles/KoCharacterStyle.h 53e98a1 
>   libs/kotext/styles/KoCharacterStyle.cpp 665e066 
>   libs/kotext/styles/KoListLevelProperties.cpp 4a35398 
>   libs/kotext/styles/KoListStyle.h aadeb0f 
>   libs/kotext/styles/KoParagraphStyle.h 3f9f40c 
>   libs/kotext/styles/KoParagraphStyle.cpp cdf16a4 
>   libs/kotext/styles/KoStyleManager.h ac145f9 
>   libs/kotext/styles/KoStyleManager.cpp fbbc4ee 
>   libs/kotext/styles/KoTableCellStyle.h 9ba0c3a 
>   libs/kotext/styles/KoTableCellStyle.cpp eb96cf9 
>   libs/kotext/styles/KoTableCellStyle_p.h c38556b 
>   libs/kotext/styles/tests/TestOpenDocumentStyle.cpp be46779 
>   libs/kotext/styles/tests/TestStyles.cpp ea7a434 
>   libs/textlayout/KoTextLayoutArea.cpp 28e4141 
>   plugins/chartshape/Axis.cpp d2a82ba 
>   plugins/textshape/TextShape.cpp cbd9593 
>   plugins/textshape/TextTool.cpp e699319 
>   plugins/textshape/dialogs/StyleManager.cpp 6110044 
>   plugins/textshape/dialogs/StylesModel.cpp 168ef715 
>   tables/Cell.cpp 378fa03 
>   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/20111106/532f5dad/attachment.htm>