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
