Review Request 123223: Fix bug 344614: kservice splits filename wrong

[Gregor Mi]

> On April 4, 2015, 9:34 a.m., David Faure wrote:
> > 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().

> Well spotted

Thx. Andreas H helped finding it.

> QFileInfo::completeBaseName does much more work

If it is so much additional work that it prevents us from using it, what about making this a static method in QT API without the overhead?

(Until then) one could still use the existing API method which makes the code more readable. It probably will not be the bottleneck considering the surrounding code, what do you think?


- Gregor


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123223/#review78473
-----------------------------------------------------------


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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150406/4ee58917/attachment-0001.html>