Review Request: Extend KStandardDirs::find[All]Exe() to also look for .com, and .bat on Windows

[David Faure]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/3388/#review5239
-----------------------------------------------------------

Ship it!


Looks good, just two minor things (no need for uploading a new patch).


trunk/KDE/kdelibs/kdecore//kernel/kstandarddirs.cpp
<http://reviewboard.kde.org/r/3388/#comment4740>

    This should be a file-static method rather than a class-static one, since it's only used here (one symbol less).



trunk/KDE/kdelibs/kdecore//kernel/kstandarddirs.cpp
<http://reviewboard.kde.org/r/3388/#comment4741>

    split(';')  (there's a QChar overload)


- David


On 2010-04-21 16:00:31, Thomas Friedrichsmeier wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3388/
> -----------------------------------------------------------
> 
> (Updated 2010-04-21 16:00:31)
> 
> 
> Review request for kdelibs, Jaroslaw Staniek and Patrick Spendrin.
> 
> 
> Summary
> -------
> 
> On Windows, KStandardDirs::findExe(), and findAllExe() append ".exe" to the filename to look for. However, executable files on Windows may also end in .com or .bat. Therefore, this patch looks for all three extensions in the order .exe, .com, .bat.
> 
> For example, rkward ships with a wrapper script rkward.bat. kwinstartmenu relies on KStandardDirs::findExe() to locate the executables referenced in .desktop files, and fails to find rkward.bat. This patch fixes that.
> 
> 
> P.S.: Where should windows-specific review requests go to?
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdelibs/kdecore//kernel/kstandarddirs.cpp 1115458 
>   trunk/KDE/kdelibs/kdecore//tests/kstandarddirstest.cpp 1115458 
> 
> Diff: http://reviewboard.kde.org/r/3388/diff
> 
> 
> Testing
> -------
> 
> I get the same pattern of failed test suites with and without this patch.
> 
> Inside kstandarddirstest, all but one tests now pass. The remaining failed part (in both patched and unpatched versions) is KStandarddirsTest::testFindExe() due to picking up the local exe within the build tree (unrelated to the essence of this review request).
> 
> On Linux, kstandarddirstest continues to compile, and all tests continue to pass with the patch.
> 
> 
> Thanks,
> 
> Thomas
> 
>