----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/107621/#review23117 -----------------------------------------------------------
Ship it! Seems the proper fix to me, to match what the API dox promises. For possible regressions from what I looked at and tried to imagine I could not yet see one, should be safe. ToCGenerator::fetchBookmarkRef(...) using block.position() + block.length() to wrongly calculate the last position is a separate problem :) KoDocumentRdf::findXmlId(...) is broken anyway (though I miss how your patch improves the situation here, follow-up on IRC for this). KoTextWriter::Private::saveParagraph(...) looks safe as well to me. DeleteCommand::doDelete(), hm, not sure where textEditor->selectionEnd() points to and if it is one-off? But separate problem. BTW, as note for a general TODO for us: I have seen the last days that *To and *End have not always the same semantic, sometimes this refers to behind the last included position, sometimes to the last itself. Should cleaning this up be made a Junior Job bug item perhaps? libs/kotext/KoTextRangeManager.cpp <http://git.reviewboard.kde.org/r/107621/#comment17659> These three ifs could be merged as well. libs/kotext/KoTextRangeManager.cpp <http://git.reviewboard.kde.org/r/107621/#comment17660> While you are at it, could you also add a comment about the option to set matchLast to -1 to the API dox? libs/kotext/KoTextRangeManager.cpp <http://git.reviewboard.kde.org/r/107621/#comment17658> These three ifs could be merged into a single one. - Friedrich W. H. Kossebau On Dec. 7, 2012, 1:58 a.m., C. Boemann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/107621/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2012, 1:58 a.m.) > > > Review request for Calligra and Friedrich W. H. Kossebau. > > > Description > ------- > > Make the textRangesChangingWithin method actually check all that it's > supposed to check > > Normally we don't use this method in a too complex way so there should be no > regressions, > but still if bugs come up with saving too few textranges, this will be a > patch to investigate > > Basically we were only checking the matching tag against one of the > parameters while we should be > checking against both. I most case the our use is to have global match > limits, so that is why we > have not noticed the bug. > > > Diffs > ----- > > libs/kotext/KoTextRangeManager.cpp 0940fda > > Diff: http://git.reviewboard.kde.org/r/107621/diff/ > > > Testing > ------- > > None, except the skf test case, and some debug utput to see the method > actually does what I want it to. > > However there may be some regressions (espcialy in saving), because of > changed behavour > > > Thanks, > > C. Boemann > >
_______________________________________________ calligra-devel mailing list calligra-devel@kde.org https://mail.kde.org/mailman/listinfo/calligra-devel
