On Tue, 26 Feb 2013 09:36:24 -0500, Jacob Carlborg <doob@me.com> wrote:

> On 2013-02-26 15:22, Steven Schveighoffer wrote:
>
>> It would be nice to just be able to do this:
>>
>> spawnProcess(split(executeThis));
>>
>> I think we need an overload for that, especially if we get rid of the
>> auto-splitting of commands.  It should assert if the array is empty.
>
> How about:
>
> spawnProcess(string[] args ...);
>

Except there are non-string arguments at the end, stdin/stdout/stderr/config

-Steve

On Tue, 26 Feb 2013 09:45:31 -0500, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:

> On Tuesday, 26 February 2013 at 14:26:22 UTC, Steven Schveighoffer wrote:
>> On Tue, 26 Feb 2013 09:15:05 -0500, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:
>>
>>> On Tuesday, 26 February 2013 at 14:02:08 UTC, Steven Schveighoffer wrote:
>>>> If I use $XYZ in a script, and XYZ isn't set, it equates to nothing.  When I use getenv, it returns null.  That is the behavior I would intuitively expect.
>>>
>>> I thought well-written scripts should use "set -u"?
>>
>> I didn't even know about that.  But my point still stands -- if well-written scripts are supposed to use set -u, it should be the default.
>
> Pretty sure it can't be the default due to backwards-compatibility reasons.
>
>> Hm... what about something like Environment.throwOnUnsetVariable = true;
>
> That would break with programs using distinct components that rely on that setting's value...

They would just segfault instead of throwing an exception, no?  I think people would understand those consequences, but they could be spelled out in the docs for that property.

We could also make the default true, since that is what existing code currently expects.

-Steve

On Tue, Feb 26, 2013 at 09:22:11AM -0500, Steven Schveighoffer wrote: [...]
> I just reread the docs, considering Vladimir's point about space-containing no-arg programs.  I agree there is a problem.
> 
> We need to not get rid of the single program version of spawn, we need to simply interpret it as a no-arg program.
> 
> To have this not work:
> 
> spawnProcess("c:/Program Files/xyz/xyz.exe");
> 
> and require this instead:
> 
> spawnProcess("c:/Program Files/xyz/xyz.exe", []);
> 
> is not very intuitive.
> 
> It reminds me of when we had writefln and not writeln, in order to print out a string with % in it, you had to do writefln("%s", "%s");
[...]

I agree. I think the onus should be on the user to call std.array.split (or equivalent) if he wants to have arguments split on whitespace. Like this:

	spawnProcess(split("dmd -O myprogram.d mymodule.d"));

Not much of a difference in usability, but prevents nasty gotchas like cited above. I vote for spawnProcess to never automatically split on whitespace.


T

-- 
Real Programmers use "cat > a.out".

On Tuesday, 26 February 2013 at 14:56:37 UTC, Steven Schveighoffer wrote:
>> That would break with programs using distinct components that rely on that setting's value...
>
> They would just segfault instead of throwing an exception, no?  I think people would understand those consequences, but they could be spelled out in the docs for that property.
>
> We could also make the default true, since that is what existing code currently expects.

I'm not really following... What does a segfault have to do with it?

What I meant is that you may use two components (two libraries, or the main program and one library) where one sets environment.throwOnUnsetVariable. If at least one component assumes that the setting is enabled or disabled, and the other component does not restore the previous setting after changing it, then the first component's behavior will change in an unexpected way.

On Tue, 26 Feb 2013 10:19:04 -0500, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:

> On Tuesday, 26 February 2013 at 14:56:37 UTC, Steven Schveighoffer wrote:
>>> That would break with programs using distinct components that rely on that setting's value...
>>
>> They would just segfault instead of throwing an exception, no?  I think people would understand those consequences, but they could be spelled out in the docs for that property.
>>
>> We could also make the default true, since that is what existing code currently expects.
>
> I'm not really following... What does a segfault have to do with it?
>
> What I meant is that you may use two components (two libraries, or the main program and one library) where one sets environment.throwOnUnsetVariable. If at least one component assumes that the setting is enabled or disabled, and the other component does not restore the previous setting after changing it, then the first component's behavior will change in an unexpected way.

You mean changing as in, instead of throwing an exception, it tries to use a null value and segfaults?  Not a very significant difference.

But we are splitting hairs here.  The first one could potentially change the environment variable that the second uses, thereby affecting the behavior.

-Steve

On Tuesday, 26 February 2013 at 15:26:50 UTC, Steven Schveighoffer wrote:
> You mean changing as in, instead of throwing an exception, it tries to use a null value and segfaults?  Not a very significant difference.

I'm still not following... where would the segfault come from? Unless you dereference .ptr, you can't get a segfault from operating on a null string.

> But we are splitting hairs here.  The first one could potentially change the environment variable that the second uses, thereby affecting the behavior.

That's a completely different matter from changing how code within the same program accesses the environment in general. Both components may be operating on specialized, prefix-named variables that have no chance of interfering with each other, and still break when the behavior of a global object changes.

It would be safer for the component to define a very small wrapper, which changes environment's semantics according to its requirements.

On Tuesday, 26 February 2013 at 15:47:41 UTC, Vladimir Panteleev wrote:
> That's a completely different matter from changing how code within the same program accesses the environment in general. Both components may be operating on specialized, prefix-named variables that have no chance of interfering with each other, and still break when the behavior of a global object changes.

To go back to the filesystem analogy, such a setting would be the equivalent of a global boolean variable in std.file, which made std.file.read return null if a file didn't exist.

On Tuesday, 26 February 2013 at 14:22:08 UTC, Steven Schveighoffer wrote:
> On Sat, 23 Feb 2013 06:31:19 -0500, Lars T. Kyllingstad <public@kyllingen.net> wrote:
>
>> It's been years in the coming, but we finally got it done. :)  The upshot is that the module has actually seen active use over those years, both by yours truly and others, so hopefully the worst wrinkles are already ironed out.
>>
>> Pull request:
>> https://github.com/D-Programming-Language/phobos/pull/1151
>>
>> Code:
>> https://github.com/kyllingstad/phobos/blob/std-process2/std/process2.d
>>
>> Documentation:
>> http://www.kyllingen.net/code/std-process2/phobos-prerelease/std_process2.html
>>
>> I hope we can get it reviewed in time for the next release.  (The wiki page indicates that both std.benchmark and std.uni are currently being reviewed, but I fail to find any "official" review threads on the forum.  Is the wiki just out of date?)
>
> I just reread the docs, considering Vladimir's point about space-containing no-arg programs.  I agree there is a problem.
>
> We need to not get rid of the single program version of spawn, we need to simply interpret it as a no-arg program.
>
> To have this not work:
>
> spawnProcess("c:/Program Files/xyz/xyz.exe");
>
> and require this instead:
>
> spawnProcess("c:/Program Files/xyz/xyz.exe", []);
>
> is not very intuitive.
>
> It reminds me of when we had writefln and not writeln, in order to print out a string with % in it, you had to do writefln("%s", "%s");
>
> Now, I think we have an additional issue in that it's difficult to take a string argument with parameters in it, and pass it in one line:
>
> string executeThis = "prog arg1 arg2";
> auto params = split(executeThis);
> spawnProcess(params[0], params[1..$]);
>
> It would be nice to just be able to do this:
>
> spawnProcess(split(executeThis));
>
> I think we need an overload for that, especially if we get rid of the auto-splitting of commands.  It should assert if the array is empty.

I propose we only have two versions:

spawnProcess(string[] args, File stdin, etc...)
spawnProcess(string[] args, string[string] env, File stdin, etc...)

You'd use it like this:

spawnProcess(["prog"]);
spawnProcess(["prog", "arg1", "arg2"])
etc.

Then it would also work with split().

Lars

On Tue, 26 Feb 2013 10:47:40 -0500, Vladimir Panteleev <vladimir@thecybershadow.net> wrote:

> On Tuesday, 26 February 2013 at 15:26:50 UTC, Steven Schveighoffer wrote:
>> You mean changing as in, instead of throwing an exception, it tries to use a null value and segfaults?  Not a very significant difference.
>
> I'm still not following... where would the segfault come from? Unless you dereference .ptr, you can't get a segfault from operating on a null string.

It's funny, I completely forgot about that!  My brain was still in Objective-C/C++ mode :)

You are right, the difference is important.

>> But we are splitting hairs here.  The first one could potentially change the environment variable that the second uses, thereby affecting the behavior.
>
> That's a completely different matter from changing how code within the same program accesses the environment in general. Both components may be operating on specialized, prefix-named variables that have no chance of interfering with each other, and still break when the behavior of a global object changes.
>
> It would be safer for the component to define a very small wrapper, which changes environment's semantics according to its requirements.

I was trying to come up with an equivalent to set -u.  In other words, you said "good scripts use set -u", you could equivalently say "good D programs use Enviroment.throwOnMissingData = true"

It was just a thought.

Another possibility (naming to be determined):

Environment.nthrow["x"]; // doesn't throw
Environment["x"]; // throws

At this point though, I think the discussion really is bikeshedding.  Environment.get is not that much different than Environment[].  We should keep the current behavior in the interest of backwards compatibility.

-Steve

On Tuesday, 26 February 2013 at 16:37:26 UTC, Steven Schveighoffer wrote:
> At this point though, I think the discussion really is bikeshedding.  Environment.get is not that much different than Environment[].  We should keep the current behavior in the interest of backwards compatibility.

I, too, have come to agree with this.  Let us drop the environment discussion and focus on getting the spawnProcess() family of functions right. :)

Lars