----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123223/#review78473 -----------------------------------------------------------
Well spotted. I would however do what the commit says and just replace indexOf with lastIndexOf. QFileInfo::completeBaseName does much more work, see QFileSystemEntry::completeBaseName(). About the unittest: the solution is to test this at a higher level, using public API instead. E.g. KService("a.b.desktop").name(). - David Faure On April 3, 2015, 5:35 p.m., Gregor Mi wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/123223/ > ----------------------------------------------------------- > > (Updated April 3, 2015, 5:35 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kservice > > > Description > ------- > > Patch for https://bugs.kde.org/show_bug.cgi?id=344614. > > The difference between the old and new code (which is moved to a separated > method in the .h file) is: name.indexOf is replaced by name.lastIndexOf > > There is one issue: I would like to move the implementation of the new method > KServicePrivate::entryPathToName from the .h to .cpp file but I get at link > time: " .../autotests/kservicetest.cpp:824: undefined reference to > 'KServicePrivate::entryPathToName(QString const&)'" Is it generally possible > to unit test methods delcared *Private classes? > > > Diffs > ----- > > autotests/kservicetest.h 929cd9fefe22308909e44650e8e4bfb2456fd534 > autotests/kservicetest.cpp d46f868185c3bf45138d80d04f4eb0d2840de9ca > src/services/kservice.cpp 8c4e1a180a7628872d6f62486db9aee4b858547f > src/services/kservice_p.h 9a5a31caadc7487f8daf898eff062b2855800313 > > Diff: https://git.reviewboard.kde.org/r/123223/diff/ > > > Testing > ------- > > 1. new unit test succeeds > 2. all other unit tests in ./kservicetest sill succeed > 3. Call `kbuildsycoca5 --noincremental` without and with the patch and try > out dolphin's Space Info menu. In the first case filelight is not found. With > the patch filelight is found. > > > Thanks, > > Gregor Mi > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel