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

Rolf Eike Beer kde at opensource.sf-tec.de
Tue Apr 6 17:52:44 BST 2010


Thomas Friedrichsmeier wrote:
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/3388/
> -----------------------------------------------------------
> 
> (Updated 2010-04-06 15:53:21.359159)
> 
> 
> Review request for kdelibs, Jaroslaw Staniek and Patrick Spendrin.
> 
> 
> Changes
> -------
> 
> Thanks for that info. The updated version of the patch makes use of the
> PATHEXT environment variable as suggested (falling back to .com;.exe;.bat
> if that is not available).

-use qgetenv() instead of getenv() which returns a QByteArray instead of a 
char* and is more portable
-QStringList::contains() can be passed a Qt::CaseSensitivity as second 
argument so you don't need toLower()
-".cmd" is still missing which is bad as this is more powerful than the old 
batch files

What's the policy about single-statement if's? Put the statement on the next 
line?

I would put this code in a function of the private class and call that from 
both places. This would reduce the code duplication.

Greetings,

Eike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100406/a9287bd3/attachment.sig>


More information about the kde-core-devel mailing list