On Sunday, 24 February 2013 at 21:04:45 UTC, Vladimir Panteleev
wrote:
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.
Sure, I can think of another example. But I wouldn't read too
much into this one; it was never meant as a demonstration of the
"correct" way to open a web page. It was just a simple example
of spawnProcess() usage that uses a cross-platform application
everyone's heard of.
After all, you *could* argue this way about almost any kind of
application which wasn't just invented for the sake of the
example. (In the last one, shouldn't we open the user's
preferred word processor, etc?)
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.
Actually, the quotes can just be removed altogether. I believe
this is an old example, BTW, from the module's infancy, when it
was POSIX-only. If anyone has a good idea for sample code which
will be familiar to users of all platforms, please speak up.
3. The documentation for the "gui" config item seems to be
wrong: it prevents the creation of a console, instead of
causing it.
I've fixed it now.
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.
They were indeed supposed to be used in the new std.process, it
just slipped my mind. I don't have time to fix it right now, but
I'll do it at some point.
Personally, I don't think they should be part of the public API.
They are inherently platform-specific, and we've tried to keep
the module as platform-agnostic as possible. Besides, they are
not really usable with any of the other functions, and I am
afraid it will be interpreted that way if we make them public.
How about we put them somewhere in the std.windows package?
(std.windows.util, for example?)
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.
The whole point was to avoid any kind of arcane platform-specific
quoting and escaping rules. If you have spaces inside your
command line arguments, use the functions that take an array, and
spawnProcess() will properly quote them. If you have any other
funky characters in there, don't worry about it, spawnProcess()
will properly escape them.
The way it is now, the rules (if you can call it that) are
exceedingly simple, and they are the same on all platforms. This
has the added benefit of discouraging platform-dependent client
code.
Lars
- Re: The new std.process is ready for review Lars T. Kyllingstad
-
