On Tuesday, 5 March 2013 at 21:04:15 UTC, Vladimir Panteleev
wrote:
On Tuesday, 5 March 2013 at 20:19:06 UTC, Lars T. Kyllingstad
wrote:
A special thanks to Vladimir P. for pointing out an egregious
flaw in the original design.
But wait, there's more!
Aw, man....
(please don't hurt me)
1. Typo: "plattform"
That would be my native language shining through. :)
2. Is there any meaning in the idea of consolidating
spawnProcess/pipeProcess/execute and
spawnShell/pipeShell/shell? How about that collectOutput idea?
In principle, I like that idea. In fact, you'll see that
execute() and shell() are now both implemented using a function
similar to (and inspired by) the collectOutput() method you
suggested. Furthermore, pipeProcess() and pipeShell() both
forward to a pipeProcessImpl() function which takes a spawn
function as a template parameter.
I'm not sure if this is the API we want to expose to the user,
though. Firstly,
auto r = execute("foo");
is a lot easier on the eye than
auto r = pipeProcess("foo", Redirect.stdout |
Redirect.stderrToStdout)
.collectOutput();
Secondly, I only think a collectOutput() method would be
appropriate to use if one of the output streams is redirected
into the other. Consider this:
auto r = pipeProcess("foo").collectOutput();
Now, the output and error streams are redirected into separate
pipes. But what if "foo" starts off by writing 1 MB of data to
its error stream?
Maybe it could be solved by some intelligent behaviour on the
part of collectOutput(), based on the redirect flags, but I think
it is better to encapsulate pipe creation AND reading in one
function, as is currently done with execute() and shell().
pipeProcess(), on the other hand, that is another matter. I
wonder if pipeProcessImpl() would be a good public interface
(with a different name, of course)?
3. Where are we with compatibility with the old module? One
idea I haven't seen mentioned yet is: perhaps we could make the
return value of "shell" have a deprecated "alias this" to the
output string, so that it's implicitly convertible to a string
to preserve compatibility.
If that works in all cases, I think it is a fantastic idea!
There is still the issue of the old shell() throwing when the
process exits with a nonzero status, though.
Maybe I'll just have to bite the bullet and accept a different
name. :( It really seems to be the one thing that is preventing
the two modules from being combined. Suggestions, anyone?
4. Is there any way to deal with pipe clogging (pipe buffer
getting exceeded when manually handling both input and output
of a subprocess)? Can we query the number of bytes we can
immediately read/write without blocking on a File?
I've wondered about that myself. I don't know whether this is a
problem std.process should aim to solve in any way, or if it
should be treated as a general problem with File.
It is a real problem, though. Pipe buffers are surprisingly
small.
5. How about that Environment.opIn_r?
Forgot about it. :) I'll add it.
Lars
