The new std.process is ready for review

[Vladimir Panteleev]

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.

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.

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

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.

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.

6. What are the reasons why this module can't be integrated with 
the existing std.process? I've noticed it mentioned a few times 
but couldn't actually find the reasoning, anyone can post the 
link?

7. How do I test this with a recent version of Phobos? I'm 
getting the following runtime exception with the "ls" example:

std.stdio.StdioException at std\stdio.d(2343): Failed to pass stdin 
stream to child process
----------------
0x0041D504 in char[][] 
core.sys.windows.stacktrace.StackTrace.trace()
0x0041D38F in core.sys.windows.stacktrace.StackTrace 
core.sys.windows.stacktrace.StackTrace.__ctor()
0x004124A8 in 
D3std8process219spawnPАЖПWindowsFNeAyaxAтPvSАД░5┌io4FileАРРАРРEАНр6ConfigZCАНЦ3Pid13prepa┘St┘amFKАР╔kАГ JiJPvZv
0x0040F8AA in @trusted std.process2.Pid 
std.process2.spawnProcess(immutable(char)[], 
const(immutable(char)[][]), std.stdio.File, std.stdio.File, 
std.stdio.File, std.process2.Config)
0x0040A353 in @trusted std.process2.Pid 
std.process2.spawnProcess(immutable(char)[], std.stdio.File, 
std.stdio.File, std.stdio.File, std.process2.Config)
0x004020B9 in _Dmain

Do I need a patched snn.lib or something else?