> On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/kotext/KoTextBlockData.cpp, line 87 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103020#file103020line87> > > > > Shouldn't markupRanges.value(type) be used instead of > > markupRanges[type]? > > According to Qt's documentation, the [] operator silently inserts a > > item in the map if none exists.
Well in most cases (like) here i need a &T so i can modify it. However value() gives me a const T > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutArea_paint.cpp, line 752 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103023#file103023line752> > > > > Perhaps rephrase for a more generic sense (marking could be for other > > things than spell-checking AFAICS). > > Perhaps: Finally let's paint our own markings (eg. spelling,...) both yes and no, since this is drawig for the specialized case of spellings. other kinds of markings might be drawn in a completely different way > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutArea_paint.cpp, line 754 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103023#file103023line754> > > > > why not painter->save/painter->restore? Because this is faster. save/restore does the same thing but for every setting in the painter. Possibly this is done in a slower way than save/resotre can do it but overall i'm fairly sure this is faster. And a lot of internal qt code makes a similar trade when few things needs to be saved and restored > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutArea_paint.cpp, lines 756-760 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103023#file103023line756> > > > > Shouldn't there be a check if spelling plug-in is present/active? > > > > Perhaps insert a "TODO: make this configurable", so it is easier to > > find if/when we want to make it so. i guess but this is controlled from the plugin > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutArea_paint.cpp, line 783 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103023#file103023line783> > > > > Are we sure marIt->endX is always correct here? I'm sure :) > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > plugins/textediting/spellcheck/SpellCheck.cpp, line 127 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103032#file103032line127> > > > > Still valid? yes still valid - i've not touched this part - but not sure it a relevant idea. it makes it less responsive > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > plugins/textshape/TextTool.cpp, line 1633 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103038#file103038line1633> > > > > Has this anything to do with the spell-checking stuff, or is it a bug > > fix, which should be back-ported also? > > Yes it's a fix and should probably be backported, though i'm not sure it will not have regressions without the rest > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutRootAreaProvider.h, line 58 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103024#file103024line58> > > > > Perhaps add a comment that the default implementation does nothing? > > And why not pure virtual like the others? I've made it pure virtual now, so no comment added > On Dec. 31, 2012, 10 a.m., Pierre Stirnweiss wrote: > > libs/textlayout/KoTextLayoutArea_paint.cpp, lines 776-781 > > <http://git.reviewboard.kde.org/r/108022/diff/1/?file=103023#file103023line776> > > > > I am not sure, but can't the logic be a bit improved here? It seems odd > > to assign x1 in the above line, and then check if the values in the > > MarkupRange are correct, eventually assigning x1 again. I've restructed it a little bit - C. ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108022/#review24301 ----------------------------------------------------------- On Dec. 30, 2012, 1:51 p.m., C. Boemann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/108022/ > ----------------------------------------------------------- > > (Updated Dec. 30, 2012, 1:51 p.m.) > > > Review request for Calligra. > > > Description > ------- > > Change the way we draw markings of speeling errors. Instead of using the qt > way we draw it ourselve > This has the following benefits: > - we don't have to relayout after finding spelling errors > - we can have more than one type of marking (spelling, grammar, etc) > > This commit also changes the way we ask for spellchecking to be performed. > The old way had some > shortcommings that could lead to text not being checked at all > > > Diffs > ----- > > libs/kotext/KoTextBlockData.h 05db499 > libs/kotext/KoTextBlockData.cpp b1d7d5c > libs/kotext/KoTextEditingPlugin.h 7b7b9dd > libs/textlayout/KoTextLayoutArea.h 94c52ec > libs/textlayout/KoTextLayoutArea_paint.cpp 2fc2131 > libs/textlayout/KoTextLayoutRootAreaProvider.h d8c0614 > libs/textlayout/KoTextLayoutRootAreaProvider.cpp 7d27944 > plugins/textediting/autocorrection/Autocorrect.h 2a84506 > plugins/textediting/autocorrection/Autocorrect.cpp e136216 > plugins/textediting/changecase/Changecase.h 8433be1 > plugins/textediting/changecase/Changecase.cpp 936957c > plugins/textediting/spellcheck/CMakeLists.txt a4dedef > plugins/textediting/spellcheck/SpellCheck.h ee2f7bf > plugins/textediting/spellcheck/SpellCheck.cpp 4da7e7d > plugins/textediting/thesaurus/Thesaurus.h c5de5b0 > plugins/textediting/thesaurus/Thesaurus.cpp b10a69d > plugins/textshape/SimpleRootAreaProvider.h 8daf2f8 > plugins/textshape/SimpleRootAreaProvider.cpp 1f3b0fe > plugins/textshape/TextTool.h a8a525d > plugins/textshape/TextTool.cpp 148806c > words/part/KWRootAreaProvider.h 1908b11 > words/part/KWRootAreaProvider.cpp 9e28045 > > Diff: http://git.reviewboard.kde.org/r/108022/diff/ > > > Testing > ------- > > played around with it > > > Thanks, > > C. Boemann > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
