The new std.process is ready for review

[Lars T. Kyllingstad]

On Monday, 25 February 2013 at 00:15:21 UTC, Vladimir Panteleev 
wrote:
> On Sunday, 24 February 2013 at 14:43:50 UTC, Lars T. 
> Kyllingstad wrote:
>> [snip]
>
> Hi Lars,
>
> First of all, about environment. I think the old behavior makes 
> more sense.
>
> I think you had a good point about making it behave like an 
> associative array. I would expect using opIndex with an 
> inexisting key to throw. Subtle deviations of behavior for 
> types that generally behave like well-known types can introduce 
> latent bugs.
>
> The danger is even more potent in the case of environment 
> variables, as those are often used for constructing 
> command-lines and such. If attempting to get the value of an 
> inexisting variable now returns null, which is used to build a 
> command line, unexpected things can happen.
>
> For example, let's say that you're writing a program for 
> analyzing malware, which expects $BINDUMP to be set to the path 
> of some analysis tool. So it runs environment["BINDUMP"] ~ 
> args[1] - however, if BINDUMP is unset, the program runs the 
> malware executable itself.
>
> For another example, here's this classic catastrophic bug in 
> shell scripts:
>
> rm -rf $FOO/$BAR
>
> What happens if $FOO and $BAR are unset?
>
> One thing that I think is missing from the environment object 
> is opIn_r. Implementing opIn_r would allow users to more safely 
> explicitly check if a variable is set or not, and is more 
> readable than environment.get("FOO", null).
>
> And of course, there's the issue of people migrating code from 
> the old module version to the new one: if they relied on the 
> old behavior, the code can break in unexpected ways after the 
> migration.
>
> What are your specific reasons for changing environment's 
> behavior?
>
> Speaking of shells, I noticed you hardcode cmd.exe in 
> std.process2. That's another bug, it should look at the COMSPEC 
> variable.
>
> Also, about the shell function, I noticed this recent bug 
> report:
> http://d.puremagic.com/issues/show_bug.cgi?id=9444
>
> Maybe it somehow makes the transition to the new function 
> easier? :)
>
> If not, since you're adamant about not changing the name, can 
> we overload the function (e.g. make the new one return some 
> results in "out" parameters), and deprecate the original 
> overload?
>
> Finally, I'd just like to sum up that we seem to have two 
> decisions on the scales: somehow solving the API 
> incompatibilities, or introducing the new version as an 
> entirely new module. The latter is a mess we really don't want 
> to get into, so it'd need to be justified, and IMHO the 
> incompatibilities don't seem to be as severe and unresolvable 
> to warrant that mess.

Sorry, I have to get to work now, and I don't have time to answer 
your post properly.  I will say this, though:  You make a strong 
case about environment.opIndex(). :)

I'll think some more about it and write a proper reply later.

Lars