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


Thanks for looking at this! Looks mostly good, apart from two issues below.


kio/kio/krun.cpp
<http://git.reviewboard.kde.org/r/106500/#comment15234>

    Yes we need this, it's the command to be executed :-)
    
    Example, "kget srcUrl destUrl". I guess KShell::splitArgs could be used, if 
QProcess can only take individual args?
    This would still leave a behavior difference with $HOME and ~ and other 
shell-specific things, but I guess we could ban these, due to the lack of 
portability (to Windows for instance).
    
    Looking at the uses of runCommand in kdelibs, I see that many of these are 
assembling the command line themselves... so it sounds like they should use 
QProcess directly instead, or (to benefit from startup notification and error 
boxes if the executable isn't found, etc.), we need to ensure KRun provides a 
way to run a command like "executable + args", in addition to the current 
"QString cmd". Only then we can start considering to deprecate the "QString 
cmd" one...



kio/kio/krun.cpp
<http://git.reviewboard.kde.org/r/106500/#comment15235>

    That was a bit of an over-zealous search/replace ;-)
    
    We should keep the KProcessRunner class name, only Qt should have classes 
starting with a Q.


- David Faure


On Sept. 18, 2012, 11:12 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106500/
> -----------------------------------------------------------
> 
> (Updated Sept. 18, 2012, 11:12 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Description
> -------
> 
> Port some uses of KProcess to QProcess
> 
> 
> Diffs
> -----
> 
>   kdeui/dialogs/kedittoolbar.cpp 0582934b0bf8cb7160cb48b4c8151c81b35277f0 
>   kinit/klauncher.cpp 855e56041be5a5b76b9a7e9d0597ac7ad485682e 
>   kio/kfile/kfilemetadatareader.cpp 88cadaa2edf1b1de24c0e91576cca368db41f470 
>   kio/kio/krun.h 7bfe66b59f1deffc37d3ceae999fb929e453fd31 
>   kio/kio/krun.cpp 031dbc1dfef685729038b4a59cbeacd34d448ed2 
>   kio/kio/krun_p.h 0ad15c8434599ccabcd649f251aa622d4fb0b0f7 
>   staging/kwidgets/autotests/kglobalsettingstest.cpp 
> 4426fee08427499c777cc7fc94e4b1345c790ac2 
> 
> Diff: http://git.reviewboard.kde.org/r/106500/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

_______________________________________________
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Reply via email to