> On April 6, 2014, 7:48 a.m., David Faure wrote: > > kdeui/itemviews/kcategorizedview.cpp, line 795 > > <https://git.reviewboard.kde.org/r/113969/diff/4/?file=215105#file215105line795> > > > > Coding style comment: some code paths use "break" (which results in > > return QModelIndex() at the end) and some other code paths (e.g. line 776) > > do "return QModelIndex()" directly. Any reason for the inconsistency?
The break at this line is not inconsistent with the `return QModelIndex()` at line 776 (line 764 in the new patch) because that line is inside another for loop. It is indeed not consistent with the old code at line 750 (line 738 in the new patch) which I didn't noticed before. I've changed this in the new patch to be consistent with the old style. - Yichao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/113969/#review55062 ----------------------------------------------------------- On April 22, 2014, 6:13 a.m., Yichao Yu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/113969/ > ----------------------------------------------------------- > > (Updated April 22, 2014, 6:13 a.m.) > > > Review request for kdelibs, David Faure, Rafael Fernández López, and Michael > Pyne. > > > Bugs: 309780 > http://bugs.kde.org/show_bug.cgi?id=309780 > > > Repository: kdelibs > > > Description > ------- > > 1. the offset addjust in `KCategorizedView::indexAt` was a no-op (it operates > on a temporary variable and is not needed). > 2. KCategorizedView::indexAt (effectively) assumes all items has the same > height when doing bsearch and therefore failed to handle some cases when the > text takes multiple lines as shown in the bug report. > > This patch removes the no-op and add special check for items in the same row > on the left (or on the right for RightToLeft layout) in order to determine > which way the bsearch should go. > > > Diffs > ----- > > kdeui/itemviews/kcategorizedview.cpp be811aa > > Diff: https://git.reviewboard.kde.org/r/113969/diff/ > > > Testing > ------- > > Compiles and fixes the problem. > Tested with systemsettings in the following conditions: > 1. single row in each category. > 2. multiple rows in each category. > 3. scrollbar not at the top. > > > Thanks, > > Yichao Yu > >
