> On Янв. 27, 2015, 7:34 п.п., 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. > > Friedrich W. H. Kossebau wrote: > 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`. > > Denis Kuplyakov wrote: > Can you post me some link, where I can read about what you mean under > mutable? > > Friedrich W. H. Kossebau wrote: > Any good physical C++ book, hard to link ;) (no idea about what related > ebooks there are) > > Hm, perhaps this here gives you a start: > http://en.cppreference.com/w/cpp/language/cv > > Elvis Stansvik wrote: > http://en.cppreference.com/w/cpp/language/cv - "mutable - applies to > non-static class members of non-reference non-const type and specifies that > the member does not affect the externally visible state of the class. mutable > members of const classes are modifiable (Note: the C++ language grammar > treats mutable as a storage-class-specifier, but it does not affect storage > class)". Wikipedia has a slightly less technical description (second > example): http://en.wikipedia.org/wiki/Immutable_object#C.2B.2B > > Denis Kuplyakov wrote: > I have tried to use mutable with no luck. I doesn't understand how can I > properly apply it in my case: isValidNewName() calls sectionNames() of its > private part, and it calls update() public method. It is absolutely unlogic > to make update() const function. And also two subsequent calls of > isValidNewName() could give you different results, even if KoSectionManager > isn't changed. You can create a section between them without referencing > anywhere KoSectionManager (creating KoSection with QTextCursor, will get > document associated KoSectionManager by pointer, and update() of it will be > called and that will be done regardless of you local entity of > KoSectionManager is const or not (which is pointed)). And KoSectionManager > isn't supposed to be part of some other class, as I think. It is created > dynamically and only links to it are stored. > > Camilla Boemann wrote: > ah yes, you are right it can't be applied to this case. Hmmm why do > sections even have names, and why do they then have to be unique - surely we > are not exposing some odf attribute?
As I remember ODF documents say, that Section element has a Name that should be unique. > On Янв. 27, 2015, 7:34 п.п., Camilla Boemann wrote: > > libs/kotext/commands/DeleteCommand.cpp, line 321 > > <https://git.reviewboard.kde.org/r/122278/diff/1/?file=345394#file345394line321> > > > > yikes > > Denis Kuplyakov wrote: > QVariant before was used like static_cast<KoSection*>(v.value<void*>()). > And this structure only for local use, doesn't look dangerous for me. > > Camilla Boemann wrote: > well why do we have both KoSection and KoSectionEnd and not just say > KoSectionDelimiter that handles both. Just wondering - not saying it has to > be like that This is entirely different entities: KoSection has many things like own style, name and so on (and I can't say that delimiter could have style). And KoSectionEnd is a simple helper class that links to corresponding KoSection. - Denis ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122278/#review74840 ----------------------------------------------------------- On Янв. 28, 2015, 8:21 п.п., Denis Kuplyakov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/122278/ > ----------------------------------------------------------- > > (Updated Янв. 28, 2015, 8:21 п.п.) > > > 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 > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
