> On Aug. 21, 2013, 9:28 p.m., Frank Reininghaus wrote: > > I guess what Thomas meant is not that you should use > > QStringList_removeDuplicates directly, but that you should follow a similar > > approach to resolve the following problem with your current patch: > > > > You have two nested loops which iterate over the list "lstServiceTypes". > > Therefore, the patched function has O(N^2) complexity, where N is the > > length of the list. If N ever becomes large (I'm not sure how likely that > > is), then there is a very bad performance problem. > > > > BTW, your patch still has coding style issues. Always put a space behind > > "if", do not put spaces inside "(...)", and use braces even for single-line > > if/else blocks, see > > > > http://techbase.kde.org/Policies/Kdelibs_Coding_Style > > > > for details. Even if the existing code does not follow the style, you > > should format any new or changed lines correctly.
The two nice things about the stringlist function are that it doesn't require a second list (but just collapses the current) and checks against a growing (but pre-allocated) set. I guess the problem here is that KMimeType::is() is not commutative. In this case that does unfortunately not work. Since performance seems not critical anyway, that doesn't matter though. - Thomas ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/111951/#review38305 ----------------------------------------------------------- On Aug. 21, 2013, 10:25 p.m., Mathias Tillman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/111951/ > ----------------------------------------------------------- > > (Updated Aug. 21, 2013, 10:25 p.m.) > > > Review request for kdelibs. > > > Description > ------- > > This fixes a bug introduced by > https://projects.kde.org/projects/kde/kdelibs/repository/revisions/871cccc8a88a600c8f850a020d44bfc5f5858caa > that made it impossible to re-order file type associations both in System > settings and in the open with list. Hence it contains a new way of detecting > duplicate (inherited) mimetype entries, that the original was supposed to fix. > > > This addresses bug 321706. > http://bugs.kde.org/show_bug.cgi?id=321706 > > > Diffs > ----- > > kdecore/services/kservice.cpp 8e81929b91803a3eed586d9fc15cdd78165b6bce > kded/kbuildservicefactory.cpp 7f89a991d088476d8ed94763e6fa65dcc3d0603c > kded/kmimeassociations.h 4a2c71312ade32a9ac779495496bf6ebb78b37a4 > kded/kmimeassociations.cpp b0af7bcc4a39e5cce1fa6abe86cace476313702a > > Diff: http://git.reviewboard.kde.org/r/111951/diff/ > > > Testing > ------- > > I have tested so that file type associations can be ordered correctly, in > addition to making sure that parent mimetypes have precedence when an app > lists two or more mimetypes where one is the parent of the other. > > > Thanks, > > Mathias Tillman > >
