Review Request 122278: Additional changes in section handling

Friedrich W. H. Kossebau kossebau at kde.org
Wed Jan 28 20:22:43 GMT 2015



> On Jan. 27, 2015, 7:34 p.m., Camilla Boemann wrote:
> > libs/kotext/KoSectionManager.cpp, line 120
> > <https://git.reviewboard.kde.org/r/122278/diff/1/?file=345390#file345390line120>
> >
> >     why no long const ?
> 
> Denis Kuplyakov wrote:
>     I don't know why, but QHash<T,Q>::find() isn't const. Thats why I can't make this function const and possibleNewName() too as dependant.
> 
> Denis Kuplyakov wrote:
>     Ohh, I see constFind there. Why they made it such way? Anyway, will fix this.
> 
> Denis Kuplyakov wrote:
>     There is a sectionNames() call, that updates internal data, to prevent using of invalidated one. So we can't put const here.

Could the members which hold that data that is only lazily fetched/updated be tagged with `mutable` perhaps?
Problem with non-const get-methods is that this escalates up in the call hierarchy, i.e. any other method which uses this method now also needs to be non-const. Which is not nice, because it spoils the usefulness of const, which should protect against unintended data modification. Which lazily updated data surely is not, that's e.g. why there is `mutable`.


- Friedrich W. H.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/122278/#review74840
-----------------------------------------------------------


On Jan. 28, 2015, 8:21 p.m., Denis Kuplyakov wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/122278/
> -----------------------------------------------------------
> 
> (Updated Jan. 28, 2015, 8:21 p.m.)
> 
> 
> Review request for Calligra, Camilla Boemann and Inge Wallin.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> This all things that I have done after GSoC, but they seems not be in master by now.
> 
> 1) Added special functions to retrive startings and endings. Changed all accesses to use this functions from KoSectionUtils. Also extracted some section related functionality to KoSectionUtils.
> 
> 2) Fixed wrong indication of section level.
> 
> 3) There were no invalidate send to KoSectionManager after paste to update section level.
> 
> 4) Added KoSection and KoSectionEnd to Metatype system
>    
>    This allowed to reduce number of QVariant casts allwhere
>    sections are used. Also fixed some missing usings of
>    KoSectionUtils functions.
>    
> 5) Deleted unused code somewhere.
> 
> 6) KoSectionManager doesn't store QStandardItemModel now, but can generate it on update. Extracted KoSectionManagerPrivate to cpp file back and changed QScopedPointer to usual pointer for MS VC compiler. Now there is a set of all ever registered sections in KoSectionManager, cuz there are sections that store in memory but cannot be deleted due to possible undo.
> 
> 7) Bug with "Incorrect name" is fixed, if you try to rename section with name that existed before.
> 
> 
> Diffs
> -----
> 
>   plugins/textshape/dialogs/SimpleInsertWidget.cpp b5500ae 
>   words/part/dockers/KWDebugWidget.cpp 2aa53c0 
>   libs/kotext/KoSectionManager.h dc4821f 
>   libs/kotext/KoSectionManager.cpp df4e535 
>   libs/kotext/KoSectionUtils.h 1bd5eb3 
>   libs/kotext/KoSectionUtils.cpp cbcefd5 
>   libs/kotext/KoTextPaste.cpp 08c6cf8 
>   libs/kotext/commands/DeleteCommand.cpp 7c69df7 
>   libs/kotext/commands/NewSectionCommand.cpp 50ba14d 
>   libs/kotext/opendocument/KoTextLoader.cpp fe4713a 
>   libs/kotext/opendocument/KoTextWriter_p.cpp 0e4ea11 
>   libs/kotext/tests/TestKoTextEditor.cpp a4b02c0 
>   libs/textlayout/KoTextLayoutArea_paint.cpp d6fa3b8 
>   plugins/textshape/dialogs/SectionFormatDialog.cpp c825fd2 
>   libs/kotext/KoSection.h 8183958 
>   libs/kotext/KoSection.cpp a651fd4 
>   libs/kotext/KoSectionEnd.h 0005851 
>   libs/kotext/KoSectionEnd.cpp 0d7cf83 
> 
> Diff: https://git.reviewboard.kde.org/r/122278/diff/
> 
> 
> Testing
> -------
> 
> Have played with sections a bit (deleting, inserting, renaming, undo). And DeleteCommand for sections unittest is passing. Seems to be OK.
> 
> 
> Thanks,
> 
> Denis Kuplyakov
> 
>

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


More information about the calligra-devel mailing list