KProcess overhaul [PATCH]

[Oswald Buddenhagen]

On Tue, Apr 18, 2006 at 10:11:27PM +0200, Ralf Habacker wrote:
> >>Agreed, but KProcess need an interface for executing a shell. It is okay 
> >>as it is ?
> >>
> >>    
> >i don't have objections.
> >well, in fact i hoped to go more into the direction of set<SomeMode>
> >functions and one async exec() with very few options, but unfortunately
> >qt went exactly the opposite direction, so for consistency we should do
> >the same. oh, well.
> >
> >  
> Please note that my code is currently more a proposal based on recent 
> QProcess. It has the advantage that is is QT conform, but requires some 
> rewriting of KDE code. I have played with an alternative approach using 
> recent KProcess api using QProcess internal, which would work too after 
> some KProcess api changes I would suggest.
> 
that wasn't the point of the rant. just forget it.

> >>>but there's no reason for
> >>>shellOptions anyway, as it is required to be consistent among shells.
> >>> 
> >>>      
> >>QProcess::start()'s or QProcess:execute()'s first parameter must be an 
> >>executable without any parameters, otherwise it couldn't be located in 
> >>the path variable.
> >>This require to save parameters elsewhere in this case shellOptions. Is 
> >>this okay for you ?
> >>
> >>    
> >on unix it is always shell -c command, on windows shell /c command - so
> >why do you want to save it in a variable?
> >  
> to avoid #ifdefs in the related start functions otherwise it would be
> 
> void KProcess::startShell( const QString & program, const QStringList & 
> arguments )
> {
> #ifdef Q_OS_WIN
>    return start(KProcessPrivate::shell, QStringList() << "/c" << 
> program << arguments);
> #else
>    return start(KProcessPrivate::shell, QStringList() << "-c" << 
> program << arguments);
> #endif
> 
> and the same for  KProcess::executeShell()
> 
> Now is is
> 
> setupShell()
> {
> #ifdef Q_OS_WIN
> set shell variable
> #else
>   set unix shell for unix derivate
> #endif
> }
> 
> void KProcess::startShell( const QString & program, const QStringList & 
> arguments )
> {
>    return start(KProcessPrivate::shell, QStringList() << 
> KProcessPrivate::shellOptions << program << arguments);
> }
> 
> I like more to setup relevants options on one place and to use it 
> without conditions later , but this could also be implemented in another 
> way. :-)
> 
you can #define C_ARG in an #ifdef, if you like. or you could split the
line and have only the relevant part #ifdef'd.

> >>>2) you botched the command composition. it is 
> >>>"shell -c <all-arguments-concatenated>", not "shell -c <arg> <arg> ...".
> >>>on windows that does not matter, as everything is more or less simply
> >>>concatenated into one string anyway, but on unix it does.
> >>> 
> >>>      
> >>I don't know how to implement this on unix.
> >>
> >>    
> >you could simly read the old code.
> >honestly, you are scaring me by playing with this stuff. get yourself *a
> >bit* into the topic first, for *all* relevant platforms.
> >
> >  
> Hmmh, KProcess::start() and KProcess::execute() a QStringList as 
> arguments, so it would be easy to add this support in these methods.
> 
> void KProcess::startShell( const QString & program, const QStringList & 
> arguments )
> {
>    args = quoteArguments(arguments);
>    return start(KProcessPrivate::shell, QStringList() << "/c" << 
> program << args );
> }
> 
> 
> Additional on windows quoting is already implemented in 
> QProcess::execute() and QProcess::start() (see see 
> http://doc.trolltech.com/4.1/qprocess.html)
> 
yes, windows sucks. :)

> Do you expect a full working solutions for all platforms in this state ?
>
well, no. but you should be perfectly clear about the QProcess calls the code
is supposed to produce - and in this case the code is quite trivial once
you know what you want.

> The executeShell() and startShell() methods are such a proposal, maybe 
> it would be better to add this to KShell, which could be the Shell 
> Starter class as KShellProcess is recent.
> 
no, KShell is not about execution, it is about parsing and constructing
shell syntax.

the bottom line is that execution through a shell is hard to get
portable at all, and once done, the benefits are relatively minor -
sounds like the reason why qt 4 still does not have it. not that i'm
opposed to the idea ...

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.