The new std.process is ready for review

[Lars T. Kyllingstad]

On Sunday, 24 February 2013 at 21:04:45 UTC, Vladimir Panteleev 
wrote:
> On Sunday, 24 February 2013 at 17:41:44 UTC, Lars T. 
> Kyllingstad wrote:
>> Ok, a new version with non-blocking wait is up.
>
> 1. Can the Firefox example be replaced with something else? 
> Spawning a specific browser to open a webpage is bad practice, 
> and I've noticed in several programs. It would be nice not to 
> perpetuate it any further. There exists a proper way to open an 
> URL in the default browser, already implemented as browse(url) 
> in the current std.process.

Sure, I can think of another example.  But I wouldn't read too 
much into this one; it was never meant as a demonstration of the 
"correct" way to open a web page.   It was just a simple example 
of spawnProcess() usage that uses a cross-platform application 
everyone's heard of.

After all, you *could* argue this way about almost any kind of 
application which wasn't just invented for the sake of the 
example.  (In the last one, shouldn't we open the user's 
preferred word processor, etc?)


> 2. (Nitpick) The grep example uses a POSIX quoting syntax 
> (single quotes). Would be better to use double quotes, or pass 
> as array to avoid one more needless OS-specific element.

Actually, the quotes can just be removed altogether.  I believe 
this is an old example, BTW, from the module's infancy, when it 
was POSIX-only.  If anyone has a good idea for sample code which 
will be familiar to users of all platforms, please speak up.


> 3. The documentation for the "gui" config item seems to be 
> wrong: it prevents the creation of a console, instead of 
> causing it.

I've fixed it now.


> 4. I see that my command escaping functions have not made it 
> through. I believe the matter has been discussed before, and I 
> thought the consensus was to use them, although it's been a 
> while. The function escapeShellCommand and its callees from the 
> current std.process have the advantages that a) they come with 
> a very thorough unit test, whereas std.process2's Windows 
> escaping code does not have tests at all, and b) they are 
> usable independently, which allows constructing scripts and 
> batch files in D programs.

They were indeed supposed to be used in the new std.process, it 
just slipped my mind.  I don't have time to fix it right now, but 
I'll do it at some point.

Personally, I don't think they should be part of the public API.  
They are inherently platform-specific, and we've tried to keep 
the module as platform-agnostic as possible.  Besides, they are 
not really usable with any of the other functions, and I am 
afraid it will be interpreted that way if we make them public.  
How about we put them somewhere in the std.windows package?  
(std.windows.util, for example?)


> 5. The spawnProcess versions which take a single string command 
> simply call std.string.split on the command. I believe this 
> approach to be fundamentally wrong, as passing an argument in 
> quotes will not work as expected. Furthermore, on Windows 
> (where process arguments are passed as a single string), 
> splitting the string in spawnProcess and then putting it back 
> together in spawnProcessWindows will result in a final command 
> line that is different from the one supplied by the user.

The whole point was to avoid any kind of arcane platform-specific 
quoting and escaping rules.  If you have spaces inside your 
command line arguments, use the functions that take an array, and 
spawnProcess() will properly quote them.  If you have any other 
funky characters in there, don't worry about it, spawnProcess() 
will properly escape them.

The way it is now, the rules (if you can call it that) are 
exceedingly simple, and they are the same on all platforms.  This 
has the added benefit of discouraging platform-dependent client 
code.

Lars